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

[Merged by Bors] - Asset re-loading while it's being deleted #2011

Closed
wants to merge 5 commits into from

Conversation

mockersf
Copy link
Member

@mockersf mockersf commented Apr 25, 2021

fixes #824
fixes #1956

  • marked asset loading methods as must_use
  • fixed asset re-loading while asset is still loading to work as comment is describing code
  • introduced a 1 frame delay between unused asset marking and actual asset removal

@mockersf mockersf added A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior labels Apr 25, 2021
Co-Authored-By: Nathan Ward <43621845+NathanSWard@users.noreply.github.com>
@NathanSWard
Copy link
Contributor

I believe this PR could be marked as ready-for-cart, unless someone else seeing any issues with the implementation.

@cart
Copy link
Member

cart commented May 4, 2021

Looks good to me / behaves as expected in my tests:

Despawning a SpriteBundle entity with a texture material:

  • frame X: despawn
  • frame X + 1: free material (which relies on texture)
  • frame X + 2: nothing
  • frame X + 3: free texture

@cart
Copy link
Member

cart commented May 4, 2021

This does imply that deeply nested dependency chains will now take longer to free up resources. Ex: something with 30 nested dependencies will be freed over the course of 30 deps * 2 frames / 60 fps = 1 second. But I think thats fine

@cart
Copy link
Member

cart commented May 4, 2021

bors r+

bors bot pushed a commit that referenced this pull request May 4, 2021
fixes #824
fixes #1956 

* marked asset loading methods as `must_use`
* fixed asset re-loading while asset is still loading to work as comment is describing code
* introduced a 1 frame delay between unused asset marking and actual asset removal
@bors bors bot changed the title Asset re-loading while it's being deleted [Merged by Bors] - Asset re-loading while it's being deleted May 4, 2021
@bors bors bot closed this May 4, 2021
@mockersf
Copy link
Member Author

mockersf commented May 4, 2021

30 deps * 2 frames / 60 fps = 1 second.

If that becomes an issue, the number of frames spent can be divided by 2 by marking unused assets in a separate system at the end of the frame, after all handles have been freed

ostwilkens pushed a commit to ostwilkens/bevy that referenced this pull request Jul 27, 2021
fixes bevyengine#824
fixes bevyengine#1956 

* marked asset loading methods as `must_use`
* fixed asset re-loading while asset is still loading to work as comment is describing code
* introduced a 1 frame delay between unused asset marking and actual asset removal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Assets Load files from disk to use for things like images, models, and sounds C-Bug An unexpected or incorrect behavior
Projects
None yet
3 participants