lucaspoffo / renet

Server/Client network library for multiplayer games with authentication and connection management made with Rust
Apache License 2.0
622 stars 66 forks source link

Messages are broadcast to newly connected client before `ServerEvent::ClientConnected` is sent #106

Closed pmikolajek closed 7 months ago

pmikolajek commented 10 months ago

This is an issue when using bevy_renet.

Assuming a standard plugin setup:

app.add_plugins(RenetServerPlugin);
app.add_plugins(NetcodeServerPlugin);

renet will already broadcast messages to a newly connected client for a single tick before sending ServerEvent::ClientConnected bevy event. This means the game doesn't have the opportunity to react to the client joining first.

The flow is as follows:

### Sometime during Tick 0 ###

- there are already connected clients
- new client connects

### Tick 1 ###

PreUpdate:
- RenetServerPlugin: fn update_system()
- NetcodeServerPlugin: fn update_system()   // new client is added to the list of clients
                                            // ServerEvent::ClientConnected event is queued in RenetServer.events
Update:
- app: game logic queues new messages

PostUpdate:
- transport: fn send_packets()  // messages are sent to everyone including new client

### Tick 2 ###

PreUpdate:
- RenetServerPlugin: fn update_system()     // sends queued ServerEvent::ClientConnected event to bevy
- NetcodeServerPlugin: fn update_system()

Update:
- app: game logic queues new messages   // also reacts to ClientConnected, queues client setup messages

PostUpdate:
- transport: fn send_packets()  // new client receives setup messages, but after regular messages from Tick 1

The issue comes from the fact that in NetcodeServerPlugin, the update method is set to run after the one from RenetServerPlugin:

# transport.rs
impl Plugin for NetcodeServerPlugin {
    fn build(&self, app: &mut App) {
        app.add_event::<NetcodeTransportError>();

        app.add_systems(
            PreUpdate,
            Self::update_system
                .run_if(resource_exists::<NetcodeServerTransport>())
                .run_if(resource_exists::<RenetServer>())
                .after(RenetServerPlugin::update_system),
        );

// ...

So during the first tick, RenetServerPlugin does not have any events to send.

Expected behaviour

After a new client connects, ServerEvent::ClientConnected should be sent before send_packets() is called. This would allow the game to react to the new arrival before sending them any messages.

lucaspoffo commented 7 months ago

This should be fixed by c8485040e32de483a6c7f398a22cdedf72088c0d

Now we have the system emit_server_events_system just to emit the events. The transport layer now updates before this new system and disconnect/connect events shouldn't have a one frame delay.

If you have a custom transport you should update it before the new system.