Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document ManualEventReader #3384

Closed

Conversation

alice-i-cecile
Copy link
Member

Objective

  • ManualEventReader is a bit confusing and complex

Solution

  • Document it.
  • Also, rename State to DoubleBufferState for better clarity and CTRL+F usability.

@github-actions github-actions bot added the S-Needs-Triage This issue needs to be labelled label Dec 19, 2021
@alice-i-cecile alice-i-cecile added A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation and removed S-Needs-Triage This issue needs to be labelled labels Dec 19, 2021
Comment on lines +185 to +187
/// To get around this, you can store a [ManualEventReader] in a [Local] to help track events seen
/// when working directly with `ResMut<Events<T>>`,
/// allowing you to read and write events from within the same system.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this, I find it confusing. Why something named ManualEventReader would work better than using EventReader?. Maybe a code sample in doc could help to show how ManualEventReader works with ResMut<Events<T>> to provide the same functionality as a EventReader and a EventWriter?

@alice-i-cecile
Copy link
Member Author

FYI, I'm a bit nervous about these docs: #2765 basically deprecates this usage pattern. If that is merged before this PR, the docs should be documented to reflect that (and I honestly wouldn't hate hiding ManualEventReader entirely).

@Weibye
Copy link
Contributor

Weibye commented May 28, 2022

FYI, I'm a bit nervous about these docs: #2765 basically deprecates this usage pattern. If that is merged before this PR, the docs should be documented to reflect that (and I honestly wouldn't hate hiding ManualEventReader entirely).

With #2765 merged, what is the status of this PR?

@alice-i-cecile
Copy link
Member Author

Good call. I'm going to close this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Docs An addition or correction to our documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants