Panic if you try to remove RenetClient or RenetServer resources after registering events

[paul-hansen]

Description

If you try to remove RenetClient or RenetServer resources after registering client or/and? server events.

Example of the panic when removing RenetServer:

thread 'Compute Task Pool (9)' panicked at 'Resource requested by bevy_replicon::network_event::client_event::receiving_system<leafwing_input_manager::action_state::ActionDiff<stellar_squeezebox:
:player::PlayerAction, stellar_squeezebox::network::NetworkOwner>> does not exist: renet::server::RenetServer', C:\Users\Paul\.cargo\registry\src\github.com-1ecc6299db9ec823\bevy_ecs-0.10.1\src\system\system_param.rs:555:17

What I Expected

I expected this to not panic and instead drop any resources needed to close the connection. This would allow setting up a new connection, for example when the user want's to stop playing one game they joined and join another or host their own.

Steps to replicate

git clone https://github.com/paul-hansen/bevy-jam-3.git
git checkout cb451e27
cargo run -- --listen 127.0.0.1
cargo run -- --connect 127.0.0.1

On either the client or the server press escape and "y" to accept. It will panic here trying to run the event systems saying the client or server resource does not exist.

I have a working patched version of bevy_replicon. If you want to test with that version, do the same but checkout 4217a6d5 instead. (Our game's main branch is using this patch too) Instead of a panic you should see a dialog that allows you to join or create a new game.

[Shatur]

Thanks for the report! When you remove RenetServer / RenetServer the plugin should change states to ServerState::NoServer / ClientState::NoConnection: https://github.com/lifescapegame/bevy_replicon/blob/fa3d2bec26d1d94876a65f670d98b7c6bad0d79c/src/client.rs#L26-L29 https://github.com/lifescapegame/bevy_replicon/blob/fa3d2bec26d1d94876a65f670d98b7c6bad0d79c/src/server.rs#L58-L63 But these systems triggered with a one frame delay and its causes the crash. It's my mistake after migration to 0.10. We need to put these systems before the event transitions. May I ask you to try to do it in you PR?

[paul-hansen]

Nice, yeah I was thinking there's probably some way to get ServerState to reflect when the resources exist more tightly. I changed it to put those systems in CoreSet::PreUpdate and use run_if(state_exists_and_equals(...)) instead of .in_set(OnUpdate(some_state)) for a couple of them (Since afaik we can't control when the OnUpdate set runs), seems to be working. Let me know if there's a better system set than CoreSet::PreUpdate to put those in or anything.