EmiOnGit / warbler_grass

A bevy plugin for creating 3d grass in your game
Apache License 2.0
120 stars 11 forks source link

Use try_insert to avoid this OR give us instructions about when we are allowed to delete entities / use system sets for this plugin #81

Closed ethereumdegen closed 1 week ago

ethereumdegen commented 1 month ago

Describe the bug

thread 'main' panicked at /home/andy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/bevy_ecs-0.13.2/src/system/commands/mod.rs:1075:13: error[B0003]: Could not insert a bundle (of type bevy_asset::handle::Handle<warbler_grass::dithering::DitheredBuffer>) for entity 20v1 because it doesn't exist in this World. note: run with RUST_BACKTRACE=1 environment variable to display a backtrace Encountered a panic when applying buffers for system warbler_grass::dithering::add_dither_to_density! Encountered a panic in system bevy_app::main_schedule::Main::run_main!

To Reproduce

Steps to reproduce the behavior: Delete a warbler grass entity in the Update Loop (race cond)

Expected behavior

No panic :)

EmiOnGit commented 1 month ago

Great catch and definitely not expected behavior. Thanks for reporting I'll look into it tomorrow

ethereumdegen commented 1 month ago

i fixed the issue for now by running my entity rebuild / removal code in PreUpdate but ideally i fix this by using SystemSets and such. Or by using try_insert for components

ethereumdegen commented 1 month ago

in my experience, checking for existence of entity is not sufficient to fix the bug . i can provide a repro

ethereumdegen commented 1 month ago

here... clone this branch and run the basic example

it crashes for me every time bc of the race condition in the dithering code

https://github.com/ethereumdegen/bevy_foliage_paint/tree/crash-repro

ethereumdegen commented 1 month ago

my hacky fix is to run my code that deletes and rebuilds the grass entities in PreUpdate. But ideally i could do something else like .before( WarblerGrassSystemSet ) or something idk . Or maybe use try_insert like i was saying .

EmiOnGit commented 1 month ago

Mh, I also changed the implementation to use try_insert already. (Referenced you there) I inserted the first check because of another problem:

The last step would run for ever since the entity will always be put back into storage

EmiOnGit commented 1 month ago

If I run your code with my changes the dithering problem is resolved. Instead it complains that your replace_grass_mesh tries to insert to a not existing entity. (Which should be a problem on your system ordering since you also delete the entity I suppose?)

ethereumdegen commented 1 month ago

Okay perfect ill check it out thanks . Oh is there a branch or On Tue, May 21, 2024 at 11:04 AM Emi @.***> wrote:

If I run your code with my changes the dithering problem is resolved. Instead it complains that your replace_grass_mesh tries to insert to a not existing entity. (Which should be a problem on your system ordering since you also delete the entity I suppose?)

— Reply to this email directly, view it on GitHub https://github.com/EmiOnGit/warbler_grass/issues/81#issuecomment-2122844069, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPVWLZROPE3B5FID5HHHHDZDNO6FAVCNFSM6AAAAABH54CBWCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRSHA2DIMBWHE . You are receiving this because you authored the thread.Message ID: @.***>

EmiOnGit commented 1 month ago

Great. You can use the branch is here: https://github.com/EmiOnGit/warbler_grass/tree/dithering_race_condition Btw your plan to integrate warbler_grass into your crate looks great and seem to be a good fit since we seem to use a similar structure. Tell me if other problems occur. I also opened #86 which might be important for usability on your side. Well, feel free to report other problems if you encounter them.

EmiOnGit commented 1 month ago

Hey @ethereumdegen, instead of the branch I linked before I would ask you to test out https://github.com/EmiOnGit/warbler_grass/tree/dither_speedup instead. Does it work on your end? It should also make you drawing a lot more smoother and stable.

ethereumdegen commented 1 month ago

Okay so i tried the dither_speedup branch and set my systems to run in Update again. It does work nicely on some bootups. But 50% of the time it crashes with :

thread 'Compute Task Pool (0)' panicked at /home/andy/.cargo/registry/src/index.crates.io-6f17d22bba15001f/async-task-4.7.1/src/task.rs:452:45:
Task polled after completion
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Encountered a panic in system `warbler_grass::dithering::check_dither_compute_tasks`!
Encountered a panic in system `bevy_app::main_schedule::Main::run_main`!
2024-05-22T18:20:59.941744Z  WARN bevy_ecs::system::commands::command_queue: CommandQueue has un-applied commands being dropped.

The branch dithering_race_condition branch does seem to remove the crash bugs and is great. However no matter what i do, dithering speedup branch always crashes intermittently at startup . But if it boots up well it stays up forever. odd.

EmiOnGit commented 1 month ago

Found the bug in dithering_speedup. If you pull the changes it should work without the crash now. If it still crashes I would appreciate if you could tell me what code I can run to also get the error :)

EmiOnGit commented 1 month ago

Hey @ethereumdegen did you have the time to test out the #88 pr and can give feedback if it works as expected on your end?