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

Implement Load Progress Tracking #164

Merged
merged 2 commits into from
Jul 29, 2022

Conversation

zicklag
Copy link
Member

@zicklag zicklag commented Jul 27, 2022

Loading screen still needs to be implemented.

This PR implements a solution for tracking the load progress of game assets.

This works towards #161, but it doesn't implement the UI yet, just the load progress tracking.

It allows us to get the load progress for both GameMeta and LevelMeta and adds logic to wait until game and level assets are loaded before starting the game/level. This time period should be replaced with a loading screen in a subsequent PR.

Why This is So Complicated

Tracking the loading progress for the game is not trivial due to the fact that we have several different kinds of asset and handles referenced throughout the GameMeta struct at many different levels of struct nesting.

My initial thought was to add a simple function where I simply manually return all the HandleIds in the various different parts of the GameMeta struct in a vector. This instantly became very difficult because of how spread out between types the handles are. There wasn't a good way to know if I had gotten all the handles, and if we added more handles later, we would have to remember to update the list which would be horrible to manage and would almost surely become out of date.

Instead, in this PR I added a HasLoadProgress trait that we use a custom derive macro to derive on GameMeta and all of the structs that it contains. This ensures that all of the handles contained in the GameMeta struct will be properly evaluated when checking load progress and it doesn't require difficult manual listing of handles.

The cost is that we write a macro and add a macro crate to our codebase, but this took me less than a day, and it is a relatively simple macro. I think it turned out rather elegant and I don't know of any other reasonable alternative at this point, but let me know if you have other thoughts!

@zicklag zicklag requested review from 64kramsystem and odecay July 27, 2022 21:24
@zicklag
Copy link
Member Author

zicklag commented Jul 27, 2022

After testing on web there are two caveats that mean we still get some initial frame glitching while the page is loading:

  1. On the main menu, the menu frame appears off to the side in a weird spot for one frame. This has to do with the way that egui centers the frame, and isn't really related to our code otherwise. Maybe we should just do a fade in from the loading screen once we have it to allow time for the frame to center. Anyway, not a huge deal for now.
  2. Because of the way bevy_parallax_background is setup, we don't have access to the handles it uses for the background, and can't tell if it's loaded yet. 🤔 I might be able to fix that by loading handles into the level asset. I'll check that out.

@zicklag zicklag marked this pull request as draft July 27, 2022 23:12
@zicklag
Copy link
Member Author

zicklag commented Jul 27, 2022

OK I fixed the parallax background issue by loading the background image handles into the LevelMeta struct so that we can check on their load progress too.

As far as the egui menu issue, I think we tackle that later as it's really a minor point for now, though definitely something to look into later.

@zicklag zicklag force-pushed the load-progress-tracking branch from 62584a9 to 91904ae Compare July 27, 2022 23:27
Copy link
Member

@64kramsystem 64kramsystem left a comment

Choose a reason for hiding this comment

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

Nice! 👍

@zicklag zicklag force-pushed the load-progress-tracking branch from 91904ae to 30b4a33 Compare July 28, 2022 17:08
@zicklag zicklag marked this pull request as ready for review July 28, 2022 17:10
@zicklag zicklag force-pushed the load-progress-tracking branch from 30b4a33 to 06e496e Compare July 28, 2022 17:14
@zicklag
Copy link
Member Author

zicklag commented Jul 28, 2022

This should be ready, but I'd like your buy off at least on the idea of adding a macro crate @odecay before we merge.

@zicklag zicklag closed this Jul 28, 2022
@zicklag zicklag force-pushed the load-progress-tracking branch from 06e496e to 876eebd Compare July 28, 2022 19:24
@odecay
Copy link
Collaborator

odecay commented Jul 28, 2022

Ah what happened here? I haven't had the chance to look it over and now see its closed.

@zicklag
Copy link
Member Author

zicklag commented Jul 28, 2022

I'm in the middle of re-basing it, but I'm not sure why it automatically closed?

Give me a second.

@zicklag zicklag reopened this Jul 28, 2022
@zicklag
Copy link
Member Author

zicklag commented Jul 28, 2022

There pushed the rebase.

@odecay
Copy link
Collaborator

odecay commented Jul 29, 2022

I'm not conceptually opposed to the idea of having macro crate. I've not written macros before but I looked over the macro code and it doesn't seem too bad.

@zicklag
Copy link
Member Author

zicklag commented Jul 29, 2022

OK, then I'll merge this and as always we can change/improve later!

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 29, 2022

Build succeeded:

@bors bors bot merged commit d36eb6c into fishfolk:master Jul 29, 2022
@zicklag zicklag deleted the load-progress-tracking branch July 29, 2022 18:36
zicklag added a commit to zicklag/punchy that referenced this pull request Jul 29, 2022
PR fishfolk#164 broke hot reloading by accidentally triggering asset update
events every time it would check the game load progress.

This moves game load progress checking to a system condition instead
with a read-only borrow to the game asset to prevent the issue.
zicklag added a commit to zicklag/punchy that referenced this pull request Jul 29, 2022
PR fishfolk#164 broke hot reloading by accidentally triggering asset update
events every time it would check the game load progress.

This moves game load progress checking to a system condition instead
with a read-only borrow to the game asset to prevent the issue.
zicklag added a commit to zicklag/punchy that referenced this pull request Jul 29, 2022
PR fishfolk#164 broke hot reloading by accidentally triggering asset update
events every time it would check the game load progress.

This moves game load progress checking to a system condition instead
with a read-only borrow to the game asset to prevent the issue.
bors bot added a commit that referenced this pull request Jul 29, 2022
173: Fix Bug With Hot Reload and New Loading Progress r=64kramsystem a=zicklag

PR #164 broke hot reloading by accidentally triggering asset update
events every time it would check the game load progress.

This moves game load progress checking to a system condition instead
with a read-only borrow to the game asset to prevent the issue.

Co-authored-by: Zicklag <zicklag@katharostech.com>
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.

3 participants