gschup / bevy_ggrs

Bevy plugin for the GGRS P2P rollback networking library.
Other
294 stars 41 forks source link

Convenient way to end a session #93

Open ndev-rs opened 9 months ago

ndev-rs commented 9 months ago

Is your feature request related to a problem? Please describe. When a player joins or leaves a server, we must remake the P2P session with the appropriate amount of people (https://github.com/gschup/ggrs/issues/23). In bevy_ggrs v0.13, this worked easily by just removing the Session resource before creating a new one:

commands.remove_resource::<Session<Config>>();

In v0.14, we must also reset the GgrsTime resource. Otherwise the app will panic once synchronization is completed for the 2nd time in an app's lifetime:

commands.insert_resource(Time::new_with(GgrsTime::default()));

I also noticed that the new resources LocalInputs and PlayerInputs are not removed or reset, but this doesn't appear to cause any issues for me at the moment.

Describe the solution you'd like A convenient way to end a session. Something like:

commands.end_session::<Config>();

Describe alternatives you've considered It's just a few lines of code for now, so this is only for convenience. Though it certainly caused me a headache to diagnose :)

johanhelsing commented 9 months ago

Thanks for reporting, I'd like a convenient way to do this as well.

I think it's best if session starting and closing mirrors each other. So perhaps best to change how we start from inserting a resource to commands as well. Perhaps:

let session = session_builder
    .start_p2p_session(socket)
    .expect("failed to start session, check config");

commands.start_session(session);

Or perhaps take a session_builder and socket instead?

commands.start_p2p_session(session_builder, socket);
ndev-rs commented 9 months ago

I think it's best if session starting and closing mirrors each other. So perhaps best to change how we start from inserting a resource to commands as well. Perhaps:

let session = session_builder
    .start_p2p_session(socket)
    .expect("failed to start session, check config");

commands.start_session(session);

That would be a nice abstraction. Would it be possible to have the socket returned to us when we end the session? Something like:

let socket = commands.end_p2p_session()
    .expect("failed to return socket");
johanhelsing commented 9 months ago

Not with commands, no. It could perhaps be put into a resource where we could collect it... Not the nicest api, though

ndev-rs commented 9 months ago

Not with commands, no. It could perhaps be put into a resource where we could collect it... Not the nicest api, though

Yeah, that sounds pretty ugly. I like your idea of having the api work entirely thru commands, but perhaps there could be a separate way to take the socket which would make the session inactive until we clean it up via commands. Though that's probably a separate Issue.

let socket = session.take_socket();
commands.end_p2p_session();
johanhelsing commented 9 months ago

Btw, my workaround for this is to create a newtype around an Arc<RwLock>, so i don't actually give the socket to ggrs