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

Fixed the stageless_progress example. Added a little more clarity to the examples: stageless_progress and progress_tracking #73

Merged
merged 4 commits into from
Jul 29, 2022

Conversation

arewerage
Copy link
Contributor

I will ask you to read the comments to the commits, so as not to duplicate them here.
Thanks.

This example did not work as we would like. `ProgressPlugin`
did not affect the loading status of assets in any way.
As well as minor changes to better understand the essence of what is happening.
Copy link
Owner

@NiklasEi NiklasEi left a comment

Choose a reason for hiding this comment

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

iyes_progress not continuing the state in the stageless example was a workaround for #54. I realize now how that broke the whole idea of the example, thank you for pointing that out!

Letting iyes_progress continue the state is correct, but will break without the manually tracked task. Could you add the following to the top comment on the stageless example please:

/// NOTE: If you do not track any `iyes_progress` tasks manually, you need to let the `LoadingState`
/// handle the state transition! See [#54](https://github.com/NiklasEi/bevy_asset_loader/issues/54) for more info.

hopefully that will prevent more users from running into that bug. Ideally we would properly fix it, but I still think my last comment on the issue is the right call.

bevy_asset_loader/examples/progress_tracking.rs Outdated Show resolved Hide resolved
bevy_asset_loader/examples/progress_tracking.rs Outdated Show resolved Hide resolved
bevy_asset_loader/examples/progress_tracking.rs Outdated Show resolved Hide resolved
@arewerage
Copy link
Contributor Author

I will consider all your wishes in the near future

@NiklasEi
Copy link
Owner

Since I am preparing for the Bevy 0.8 release and would like to include this, I quickly fixed it up myself. Thank you for the PR.

@NiklasEi NiklasEi merged commit 7926034 into NiklasEi:main Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants