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

Fix in rendering resources node for despawning #247

Merged
merged 1 commit into from
Aug 22, 2020
Merged

Fix in rendering resources node for despawning #247

merged 1 commit into from
Aug 22, 2020

Conversation

BorisBoutillier
Copy link
Contributor

@BorisBoutillier BorisBoutillier commented Aug 19, 2020

This should fix #135 .

This fix has the drawback to reset the stored indices each frame, so the get_or_assign_index function will always assign.
But the underlying issue is in the incoherency between the changed_item_count which evolves with new/dead/drawn/invisible rendering components, while the indices were created once and for all.

Impact on spawner.rs example seems in the 0.3%.

I have not added the changes to fix the rendering issue with Draw.is_visible=false, as I have already done a PR for that (#230)

@DGriffin91
Copy link
Contributor

Just tested this with my project that was previously crashing when using despawn and this does fix the issue. Thanks!

@Cupnfish
Copy link
Contributor

It works great in my code. It works great

@karroffel karroffel added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen labels Aug 20, 2020
@ropewalker
Copy link

ropewalker commented Aug 20, 2020

Didn't fix the #134 for me: my game still crashes when I am trying to create both TextComponents and SpriteSheetComponents.

@BorisBoutillier
Copy link
Contributor Author

This PR currently only fixes panic during rendering related to a reduction in the number of render components ( through despawning or is_visible=false) followed by addition of new renderable components.
This does not fix the panic happening when mixing TextComponents with SpriteSheetComponents.
So fixes #135 , but does not fix #134.

@ashneverdawn
Copy link
Contributor

Hopefully this will be merged soon. Really important fix in my opinion 😄

@cart
Copy link
Member

cart commented Aug 22, 2020

This works for me too. We'll need to sort out a better long term solution, but this is 100% "worth it" at this point. Thanks!

@cart cart merged commit 4eb437a into bevyengine:master Aug 22, 2020
BimDav pushed a commit to BimDav/bevy that referenced this pull request Aug 26, 2020
…nces as get_or_assign_index will now always assign (bevyengine#247)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Despawning causes panics
7 participants