Skip to content

Commit

Permalink
fix: Fix Zero'd DenseMoveDir not representing no input. (#380)
Browse files Browse the repository at this point in the history
When GGRS sends predicted input on first frame (no prev input to use),
it [zeroes
PlayerInput](https://github.com/gschup/ggrs/blob/0af1a044b96465bd10398947b7fb5e0a34a75f70/src/frame_info.rs#L56)
and uses that.

Due to how we quantize move direction, zeroed bits did not represent no
movement input, it instead was `(-1.0, -1.0)`. This sometimes caused
desync on beginning of online match.

Previously used 6 bit representation over range, was changed to quantize
[0, 1.0] with 5 bits, and include a sign bit to map from [0, 1.0] ->
[-1.0, 1.0]. Because range is symmetric and we quantize only half of it
with one less bit, should still be same amount of precision. Now all
zero bits represents 0 which is compatible with ggrs.

Additionally added debug log (not enabled by default) in networking to
dump net inputs.
  • Loading branch information
MaxCWhitehead committed Apr 8, 2024
1 parent 6b8d743 commit cbe537f
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
7 changes: 6 additions & 1 deletion framework_crates/bones_framework/src/networking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{fmt::Debug, marker::PhantomData, sync::Arc};
use ggrs::{NetworkStats, P2PSession, PlayerHandle};
use instant::Duration;
use once_cell::sync::Lazy;
use tracing::{debug, error, info, warn};
use tracing::{debug, error, info, trace, warn};

use crate::prelude::*;

Expand Down Expand Up @@ -469,6 +469,11 @@ where
for (player_idx, (input, status)) in
network_inputs.into_iter().enumerate()
{
trace!(
"Net player({player_idx}) local: {}, status: {status:?}, input: {:?}",
self.local_player_idx == player_idx,
input
);
player_inputs.network_update(
player_idx,
&input,
Expand Down
52 changes: 44 additions & 8 deletions framework_crates/bones_framework/src/networking/proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,24 +11,35 @@ use crate::prelude::*;
pub struct DenseMoveDirection(pub Vec2);

/// This is the specific [`Quantized`] type that we use to represent movement directions in
/// [`DenseMoveDirection`].
type MoveDirQuant = Quantized<IntRange<u16, 0b111111, -1, 1>>;
/// [`DenseMoveDirection`]. This encodes magnitude of direction, but sign is encoded separately.
type MoveDirQuant = Quantized<IntRange<u16, 0b11111, 0, 1>>;

impl From<u16> for DenseMoveDirection {
fn from(bits: u16) -> Self {
// maximum movement value representable, we use 6 bits to represent each movement direction.
let max = 0b111111;
// Most significant is sign, and other 5 encode float value between 0 and
let bit_length = 6;
let quantized = 0b011111;
let sign = 0b100000;
// The first six bits represent the x movement
let x_move_bits = bits & max;
let x_move_bits = bits & quantized;
let x_move_sign = if bits & sign == 0 { 1.0 } else { -1.0 };
// The second six bits represents the y movement
let y_move_bits = (bits >> 6) & max;
let y_move_bits = (bits >> bit_length) & quantized;
let y_move_sign = if (bits >> bit_length) & sign == 0 {
1.0
} else {
-1.0
};

// Round near-zero values to zero
let mut x = MoveDirQuant::from_raw(x_move_bits).to_f32();
x *= x_move_sign;
if x.abs() < 0.02 {
x = 0.0;
}
let mut y = MoveDirQuant::from_raw(y_move_bits).to_f32();
y *= y_move_sign;
if y.abs() < 0.02 {
y = 0.0;
}
Expand All @@ -39,10 +50,20 @@ impl From<u16> for DenseMoveDirection {

impl From<DenseMoveDirection> for u16 {
fn from(dir: DenseMoveDirection) -> Self {
let x_bits = MoveDirQuant::from_f32(dir.x).raw();
let y_bits = MoveDirQuant::from_f32(dir.y).raw();
let x_bits = MoveDirQuant::from_f32(dir.x.abs()).raw();
let y_bits = MoveDirQuant::from_f32(dir.y.abs()).raw();
let x_sign_bit = if dir.x.is_sign_positive() {
0
} else {
0b100000
};
let y_sign_bit = if dir.y.is_sign_positive() {
0
} else {
0b100000
};

x_bits | (y_bits << 6)
(x_bits | x_sign_bit) | ((y_bits | y_sign_bit) << 6)
}
}

Expand All @@ -59,3 +80,18 @@ impl From<DenseMoveDirection> for u32 {
bits_16 as u32
}
}

#[cfg(test)]
mod tests {
use crate::prelude::proto::DenseMoveDirection;

#[test]
/// GGRS currently uses zero'd player input as prediction on first frame,
/// so ensure our move direction representation is no input when built from
/// 0 bits.
pub fn zeroed_dense_move_dir() {
let bits: u16 = 0;
let dense_move_dir = DenseMoveDirection::from(bits);
assert_eq!(dense_move_dir.0, glam::Vec2::ZERO);
}
}

0 comments on commit cbe537f

Please sign in to comment.