naia-lib / naia

a cross-platform (including Wasm!) networking library built in Rust. Intended to make multiplayer game development dead-simple & lightning-fast
Apache License 2.0
873 stars 59 forks source link

Bug when using TickBuffered channel for inputs #128

Closed cBournhonesque closed 1 year ago

cBournhonesque commented 1 year ago

I tried changing my InputChannel to use a TickBuffered channel instead of OrderedReliable and my app crashed with the error:

thread 'main' panicked at 'this method should always receive increasing or equal ticks!', /Users/cbournhonesque/dev/rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/client/src/tick/channel_tick_buffer_sender.rs:265:17
 11:        0x10d1ffc72 - naia_client::tick::channel_tick_buffer_sender::OutgoingMessages<P>::push::hce94abf1e93845d9
                               at /rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/client/src/tick/channel_tick_buffer_sender.rs:265:17
  12:        0x10d2a2442 - naia_client::tick::channel_tick_buffer_sender::ChannelTickBufferSender<P>::send_message::h16dc6dced73fe068
                               at /rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/client/src/tick/channel_tick_buffer_sender.rs:64:9
  13:        0x10d2a2442 - naia_client::tick::tick_buffer_sender::TickBufferSender<P,C>::send_message::hf95655fc54e35ee9
                               at /rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/client/src/tick/tick_buffer_sender.rs:41:13
  14:        0x10d2b785e - naia_client::client::Client<P,E,C>::send_message::he8a47413d7566bf3
                               at /rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/client/src/client.rs:203:21
  15:        0x10d25f35b - naia_bevy_client::client::Client<P,C>::send_message::hb89b91e9fc335375
                               at /rust/.cargo/git/checkouts/naia-9140df01a53cfd7b/9ed2839/adapters/bevy/client/src/client.rs:88:9
  16:        0x10d25f35b - snakegame_client::systems::input::client_send_movement::hf7a268e9cdf9ce9d
                               at /rust/snakegame/client/src/systems/input.rs:36:13

My input system (on client) is:

fn client_send_movement(
    movement_query: Query<&ActionState<Movement>, With<ControlledPlayer>>,
    mut client: Client<Protocol, Channels>,
) {
    for action_state in movement_query.iter() {
        if let Some(movement) = just_pressed_direction(action_state) {
            debug!("Sending movement command: {:?}", movement);
            client.send_message(Channels::InputMovement, &InputMovementMessage::new(movement));
        }
    }
}

Do i need to make other changes? For example use a Queue + CommandHistory like in the bevy example? Also, using the OrderedReliable felt definitely more responsive than the InputBuffered channel (under good network conditions)

@connorcarpenter you can reproduce it in my repo, branch cb/naia-input-bug. The command to run the client or the server is RUST_BACKTRACE=full RUN_MODE="powerline" cargo run --features "dynamic" --target-dir ../target-dynamic (might take a while to compile though!)

connorcarpenter commented 1 year ago

Thanks for the bug and branch to repro :+1:

I noticed that in your example, you are consuming movement events and sending them to the Server in a parallelized system, that may be running as quickly as events come in. When the Client is on a Tick boundary, this system could trigger several times, but then finish quicker than a system that started before it? That's what I was seeing anyway: a Message from an earlier Tick would get sent via Client.send_message(), which is unexpected behavior for a TickBufferChannel.

In https://github.com/naia-lib/naia/pull/130 I turned that panic message into a more helpful warning instead, and now simply don't proceed with the sending the Message if it's from a past Tick.

The TickBufferChannel can only transmit 1 Message every Tick, so the ideal is to send the Message in the Tick stage set up for that (like in https://github.com/naia-lib/naia/blob/5766d0bac655ca019ea9b46aff3941599a65434b/demos/bevy/client/src/systems/tick.rs#L32)