From 4e4417f2f4c98d2e0e6f5809b678150f9ac8bcd9 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sat, 7 Sep 2024 14:25:12 -0700 Subject: [PATCH 01/29] fix: Add net tracing for rollback frame and add current frame to input trace --- framework_crates/bones_framework/src/networking.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index 8f52b20191..07a6f69aeb 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -845,9 +845,10 @@ where .unwrap(); } + let current_frame = self.session.current_frame(); + #[cfg(feature = "net-debug")] { - let current_frame = self.session.current_frame(); let confirmed_frame = self.session.confirmed_frame(); NETWORK_DEBUG_CHANNEL @@ -871,7 +872,7 @@ where ggrs::GgrsRequest::SaveGameState { cell, frame } => { cell.save(frame, Some(world.clone()), None) } - ggrs::GgrsRequest::LoadGameState { cell, .. } => { + ggrs::GgrsRequest::LoadGameState { cell, frame } => { // Swap out sessions to preserve them after world save. // Sessions clone makes empty copy, so saved snapshots do not include sessions. // Sessions are borrowed from Game for execution of this session, @@ -886,6 +887,8 @@ where &mut sessions, &mut world.resource_mut::(), ); + + trace!("Loading (rollback) frame: {frame}"); } ggrs::GgrsRequest::AdvanceFrame { inputs: network_inputs, @@ -949,8 +952,8 @@ where network_inputs.into_iter().enumerate() { trace!( - "Net player({player_idx}) local: {}, status: {status:?}, input: {:?}", - self.local_player_idx as usize == player_idx, + "Net player({player_idx}) local: {}, status: {status:?}, frame: {current_frame} input: {:?}", + self.local_player_idx == player_idx as u32, input ); player_inputs.network_update( From 99bf5e9f2aba9a39e973ce10525eb4ca2ca28d2b Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:14:58 -0700 Subject: [PATCH 02/29] feat: DesyncHash trait and std impls --- Cargo.lock | 1 + framework_crates/bones_utils/Cargo.toml | 2 + .../bones_utils/src/desync_hash.rs | 143 ++++++++++++++++++ framework_crates/bones_utils/src/lib.rs | 3 +- 4 files changed, 148 insertions(+), 1 deletion(-) create mode 100644 framework_crates/bones_utils/src/desync_hash.rs diff --git a/Cargo.lock b/Cargo.lock index dad1b44b79..e8d94d77a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1612,6 +1612,7 @@ dependencies = [ "bones_utils_macros", "fxhash", "getrandom", + "glam 0.24.2", "hashbrown 0.14.5", "instant", "serde", diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index 69547da46e..669518ce2f 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -12,6 +12,7 @@ keywords.workspace = true [features] default = ["ulid"] +glam = ["dep:glam"] serde = ["dep:serde", "hashbrown/serde"] ulid = ["dep:ulid", "instant", "turborand"] @@ -25,6 +26,7 @@ instant = { version = "0.1", features = ["wasm-bindgen"], optional = true } serde = { version = "1.0", optional = true } turborand = { version = "0.10", optional = true } ulid = { version = "1.0", optional = true } +glam = { version = "0.24", optional = true } # Make sure that the getrandom package, used in `ulid` works on web # when compiling for WASM. diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs new file mode 100644 index 0000000000..13ada56e95 --- /dev/null +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -0,0 +1,143 @@ +//! [`DesyncHash`] trait and impls, for detecting net desync. +// +// In order to use [`DesyncHash`] with [`glam`] types, the "glam" feature flag must be used. + +use std::time::Duration; + +use ustr::Ustr; + +/// [`DesyncHash`] is used to hash type and compare over network to detect desyncs. +/// +/// In order to opt in a [`HasSchema`] Component or Resource to be included in hash of World in networked session, +/// `#[net]` or `#[derive_type_data(SchemaDesyncHas)]` must also be included. +pub trait DesyncHash { + /// Update hasher from type's values + fn hash(&self, hasher: &mut dyn std::hash::Hasher); +} + +impl DesyncHash for Duration { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + self.as_nanos().hash(hasher); + } +} + +impl DesyncHash for () { + fn hash(&self, _hasher: &mut dyn std::hash::Hasher) {} +} + +impl DesyncHash for bool { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + hasher.write_u8(*self as u8) + } +} + +impl DesyncHash for Vec { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for value in self { + value.hash(hasher); + } + } +} +macro_rules! desync_hash_impl_int { + ($ty:ident) => { + impl DesyncHash for $ty { + ::paste::paste! { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + hasher.[](*self); + } + } + } + }; +} + +macro_rules! desync_hash_impl_float { + ($ty:ident) => { + impl DesyncHash for $ty { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + if self.is_nan() { + // Ensure all NaN representations hash to the same value + hasher.write(&Self::to_ne_bytes(Self::NAN)); + } else if *self == 0.0 { + // Ensure both zeroes hash to the same value + hasher.write(&Self::to_ne_bytes(0.0)); + } else { + hasher.write(&Self::to_ne_bytes(*self)); + } + } + } + }; +} + +macro_rules! desync_hash_impl_as_bytes { + ($ty:ident) => { + impl DesyncHash for $ty { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + hasher.write(self.as_bytes()); + } + } + }; +} + +desync_hash_impl_float!(f32); +desync_hash_impl_float!(f64); + +desync_hash_impl_int!(i8); +desync_hash_impl_int!(i16); +desync_hash_impl_int!(i32); +desync_hash_impl_int!(i64); +desync_hash_impl_int!(i128); +desync_hash_impl_int!(isize); +desync_hash_impl_int!(u8); +desync_hash_impl_int!(u16); +desync_hash_impl_int!(u32); +desync_hash_impl_int!(u64); +desync_hash_impl_int!(u128); +desync_hash_impl_int!(usize); + +desync_hash_impl_as_bytes!(String); +desync_hash_impl_as_bytes!(str); +desync_hash_impl_as_bytes!(Ustr); + +#[cfg(feature = "glam")] +mod impl_glam { + use glam::*; + + use super::DesyncHash; + + macro_rules! desync_hash_impl_glam_vecs { + ($id:ident) => { + paste::paste! { + desync_hash_impl_glam!( [< $id 2 >], x, y); + desync_hash_impl_glam!( [< $id 3 >], x, y, z); + desync_hash_impl_glam!( [< $id 4 >], x, y, z, w); + } + }; + } + + macro_rules! desync_hash_impl_glam { + ($t:ty, $($field:ident),+) => { + impl DesyncHash for $t { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + // $(self.$field.hash(hasher);)* + // $(hasher.(self.$field);)* + $(DesyncHash::hash(&self.$field, hasher);)* + } + } + }; + } + + desync_hash_impl_glam_vecs!(BVec); + desync_hash_impl_glam_vecs!(UVec); + desync_hash_impl_glam_vecs!(IVec); + desync_hash_impl_glam_vecs!(Vec); + desync_hash_impl_glam_vecs!(DVec); + + impl DesyncHash for Quat { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + self.x.hash(hasher); + self.y.hash(hasher); + self.z.hash(hasher); + self.w.hash(hasher); + } + } +} diff --git a/framework_crates/bones_utils/src/lib.rs b/framework_crates/bones_utils/src/lib.rs index fef20d5dfa..5d1a70fb62 100644 --- a/framework_crates/bones_utils/src/lib.rs +++ b/framework_crates/bones_utils/src/lib.rs @@ -8,6 +8,7 @@ mod collections; mod default; +mod desync_hash; #[cfg(feature = "ulid")] mod labeled_id; mod names; @@ -21,7 +22,7 @@ macro_rules! pub_use { () => { #[cfg(feature = "turborand")] pub use crate::random::*; - pub use crate::{collections::*, default::*, names::*}; + pub use crate::{collections::*, default::*, desync_hash::*, names::*}; #[cfg(feature = "ulid")] pub use crate::{labeled_id::*, ulid::*}; pub use bones_utils_macros::*; From 0df9991a908520d92db5ba01d25f646beee6de8c Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:15:32 -0700 Subject: [PATCH 03/29] feat: DesyncHash derive macro --- framework_crates/bones_utils/Cargo.toml | 1 + .../bones_utils/macros/Cargo.toml | 1 + .../bones_utils/macros/src/lib.rs | 141 ++++++++++++++++++ 3 files changed, 143 insertions(+) diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index 669518ce2f..9022ed170f 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -27,6 +27,7 @@ serde = { version = "1.0", optional = true } turborand = { version = "0.10", optional = true } ulid = { version = "1.0", optional = true } glam = { version = "0.24", optional = true } +paste = { version = "1.0" } # Make sure that the getrandom package, used in `ulid` works on web # when compiling for WASM. diff --git a/framework_crates/bones_utils/macros/Cargo.toml b/framework_crates/bones_utils/macros/Cargo.toml index 0ddf2f36a1..42b2ac8423 100644 --- a/framework_crates/bones_utils/macros/Cargo.toml +++ b/framework_crates/bones_utils/macros/Cargo.toml @@ -16,3 +16,4 @@ proc-macro = true [dependencies] quote = "1.0" venial = "0.5" +proc-macro2 = "1" diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index 0f34c78b27..9b7ac7f01b 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -1,5 +1,7 @@ use proc_macro::TokenStream; +use proc_macro2::TokenStream as TokenStream2; use quote::{format_ident, quote, quote_spanned, spanned::Spanned}; +use venial::StructFields; /// Helper macro to bail out of the macro with a compile error. macro_rules! throw { @@ -168,3 +170,142 @@ pub fn derive_deref_mut(input: TokenStream) -> TokenStream { throw!(input, "Cannot derive DerefMut on anything but structs."); } } + +#[proc_macro_derive(DesyncHash, attributes(desync_hash_module))] +pub fn derive_desync_hash(input: TokenStream) -> TokenStream { + let input = venial::parse_declaration(input.into()).unwrap(); + let name = input.name().expect("Type must have a name"); + + // Get the schema module, reading optionally from the `schema_module` attribute, so that we can + // set the module to `crate` when we want to use it within the `bones_schema` crate itself. + let desync_hash_module = input + .attributes() + .iter() + .find_map(|attr| { + (attr.path.len() == 1 && attr.path[0].to_string() == "desync_hash_module").then(|| { + attr.value + .get_value_tokens() + .iter() + .cloned() + .collect::() + }) + }) + .unwrap_or_else(|| quote!(bones_utils)); + + // Helper to get hash invocations of struct fields + let hash_struct_fields = |fields: &StructFields| match fields { + venial::StructFields::Tuple(tuple) => tuple + .fields + .iter() + .enumerate() + .map(|(idx, (field, _))| { + let ty = &field.ty; + quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#idx, hasher);} + }) + .collect::>(), + venial::StructFields::Named(named) => named + .fields + .iter() + .map(|(field, _)| { + let name = &field.name; + let ty = &field.ty; + + quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#name, hasher);} + }) + .collect::>(), + venial::StructFields::Unit => vec![], + }; + + // Get fields of enum variant + let enum_variant_fields = |fields: &StructFields| match fields { + venial::StructFields::Tuple(tuple) => { + // Build identifiers for tuple as as a,b,c... + + if tuple.fields.len() > 26 { + panic!("DesyncHash derive macro does not support variants of tuples with more than 26 fields."); + } + let identifiers: Vec<_> = (0..tuple.fields.len()) + .map(|i| format_ident!("{}", (b'a' + i as u8) as char)) + .collect(); + + // format as (a, b, c, ...) + let tuple_fields = quote! { + (#(#identifiers),*) + }; + + // generate invocations for each field in tuple using generated identifier + let invocations = identifiers + .iter() + .map(|ident| { + quote! {#desync_hash_module::DesyncHash::hash(#ident, hasher);} + }) + .collect::>(); + (tuple_fields, invocations) + } + venial::StructFields::Named(named) => { + let field_idents: Vec<_> = named.fields.iter().map(|f| &f.0.name).collect(); + + // format list of fields as '{ fieldA, fieldB, ... }' + let named_fields = quote! { + {#(#field_idents),*} + }; + + let invocations = field_idents + .iter() + .map(|ident| { + quote! {#desync_hash_module::DesyncHash::hash(#ident, hasher);} + }) + .collect::>(); + (named_fields, invocations) + } + venial::StructFields::Unit => (quote! {}, vec![]), + }; + + let field_hash_invocations: Vec<_> = match &input { + venial::Declaration::Struct(s) => hash_struct_fields(&s.fields), + venial::Declaration::Enum(e) => { + let mut variants = Vec::new(); + + let enum_name = &e.name; + for (idx, v) in e.variants.items().enumerate() { + let variant_name = &v.name; + let (variant_fields_string, invocations) = enum_variant_fields(&v.contents); + variants.push(quote! { + #enum_name::#variant_name #variant_fields_string => { + // Hash index of variant to ensure that two variants are unique + hasher.write_usize(#idx); + #( + #invocations + )* + }, + }); + } + + vec![quote! { + match self { + #(#variants)* + } + }] + } + venial::Declaration::Union(_) => { + panic!("DesyncHash derive macro impl does not support Unions"); + } + _ => vec![], + }; + + let combined_hash_invocations = field_hash_invocations.iter().fold(quote! {}, |acc, q| { + quote! { + #acc + #q + } + }); + + quote! { + impl #desync_hash_module::DesyncHash for #name { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + #combined_hash_invocations + } + } + } + .into() +} From 988bef5a46de7d4d030d8ed59804293346e2697c Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:20:37 -0700 Subject: [PATCH 04/29] feat: ScheamDesyncHash typedata to opt optionally implement DesyncHash for schema --- .../bones_schema/src/desync_hash.rs | 93 +++++++++++++++++++ framework_crates/bones_schema/src/lib.rs | 2 + 2 files changed, 95 insertions(+) create mode 100644 framework_crates/bones_schema/src/desync_hash.rs diff --git a/framework_crates/bones_schema/src/desync_hash.rs b/framework_crates/bones_schema/src/desync_hash.rs new file mode 100644 index 0000000000..f72ec37ed1 --- /dev/null +++ b/framework_crates/bones_schema/src/desync_hash.rs @@ -0,0 +1,93 @@ +//! Implementation of [`DesyncHash`] for [`SchemaRef`]. + +use std::{any::type_name, hash::Hasher}; + +use bones_utils::DesyncHash; + +use crate::{prelude::*, ptr::SchemaRef, FromType, HasSchema, Schema, SchemaData}; + +/// Used in [`Schema`] [`TypeDatas`] to optionally implement desync hash. +pub struct SchemaDesyncHash { + /// Desync hash fn pointer + pub desync_hash_fn: for<'a> fn(SchemaRef<'a>, hasher: &mut dyn Hasher), +} + +unsafe impl HasSchema for SchemaDesyncHash { + fn schema() -> &'static crate::Schema { + use std::{alloc::Layout, any::TypeId, sync::OnceLock}; + static S: OnceLock<&'static Schema> = OnceLock::new(); + let layout = Layout::new::(); + S.get_or_init(|| { + SCHEMA_REGISTRY.register(SchemaData { + name: type_name::().into(), + full_name: format!("{}::{}", module_path!(), type_name::()).into(), + kind: SchemaKind::Primitive(Primitive::Opaque { + size: layout.size(), + align: layout.align(), + }), + type_id: Some(TypeId::of::()), + clone_fn: None, + drop_fn: None, + default_fn: None, + hash_fn: None, + eq_fn: None, + type_data: Default::default(), + }) + }) + } +} + +impl FromType for SchemaDesyncHash { + fn from_type() -> Self { + SchemaDesyncHash { + desync_hash_fn: |reference, hasher| { + T::schema() + .ensure_match(reference.schema()) + .expect("Schema type does not match schema ref."); + + unsafe { + DesyncHash::hash(&*reference.as_ptr().cast::(), hasher); + } + }, + } + } +} + +impl<'a> DesyncHash for SchemaRef<'a> { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + if let Some(schema_desync_hash) = self.schema().type_data.get::() { + (schema_desync_hash.desync_hash_fn)(*self, hasher); + } + } +} + +impl<'a> DesyncHash for SchemaRefMut<'a> { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + if let Some(schema_desync_hash) = self.schema().type_data.get::() { + (schema_desync_hash.desync_hash_fn)(self.as_ref(), hasher); + } + } +} + +impl DesyncHash for SchemaId { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + hasher.write_u32(self.id()); + } +} + +impl DesyncHash for SVec { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for value in self { + value.hash(hasher); + } + } +} + +impl DesyncHash for SMap { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for (key, value) in self.iter() { + key.hash(hasher); + value.hash(hasher); + } + } +} diff --git a/framework_crates/bones_schema/src/lib.rs b/framework_crates/bones_schema/src/lib.rs index 5798335f35..4d27e657d7 100644 --- a/framework_crates/bones_schema/src/lib.rs +++ b/framework_crates/bones_schema/src/lib.rs @@ -20,6 +20,7 @@ pub mod prelude { pub use crate::ser_de::*; pub use crate::{ alloc::{SMap, SVec, SchemaMap, SchemaVec}, + desync_hash::*, ptr::*, registry::*, schema::*, @@ -33,6 +34,7 @@ mod schema; pub use schema::*; pub mod alloc; +pub mod desync_hash; pub mod ptr; pub mod raw_fns; pub mod registry; From 16a006ffa5a545f33146bda39dc77e3f2ba3d63a Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:22:10 -0700 Subject: [PATCH 05/29] feat: #[net] sugar macro for #[derive_type_data(SchemaDesyncHash)]. --- .../bones_schema/macros/src/lib.rs | 23 ++++++++++++++++++ .../bones_utils/macros/src/lib.rs | 24 +++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/framework_crates/bones_schema/macros/src/lib.rs b/framework_crates/bones_schema/macros/src/lib.rs index 95dd82f7f8..d42da6498f 100644 --- a/framework_crates/bones_schema/macros/src/lib.rs +++ b/framework_crates/bones_schema/macros/src/lib.rs @@ -115,6 +115,11 @@ pub fn derive_has_schema(input: TokenStream) -> TokenStream { // Get the type datas that have been added and derived let derive_type_data_flags = get_flags_for_attr(&input, "derive_type_data"); + + let has_td_schema_desync_hash = derive_type_data_flags + .iter() + .any(|flag| flag.as_str() == "SchemaDesyncHash"); + let type_datas = { let add_derive_type_datas = derive_type_data_flags.into_iter().map(|ty| { let ty = format_ident!("{ty}"); @@ -122,6 +127,23 @@ pub fn derive_has_schema(input: TokenStream) -> TokenStream { tds.insert(<#ty as #schema_mod::FromType<#name>>::from_type()).unwrap(); } }); + + // Do we have #[net] attribute? + let has_net_attribute = input + .attributes() + .iter() + .any(|attr| attr.path.iter().any(|p| p.to_string().contains("net"))); + + // Only insert SchemaDesyncHash w/ #[net] sugar when #[derive_type_data(SchemaDesyncHash)] not present + // (Avoid double insert) + let add_desync_hash_type_data = if has_net_attribute && !has_td_schema_desync_hash { + quote! { + tds.insert(<#schema_mod::desync_hash::SchemaDesyncHash as #schema_mod::FromType<#name>>::from_type()).unwrap(); + } + } else { + quote! {} + }; + let add_type_datas = input .attributes() .iter() @@ -132,6 +154,7 @@ pub fn derive_has_schema(input: TokenStream) -> TokenStream { quote! { { let tds = #schema_mod::alloc::TypeDatas::default(); + #add_desync_hash_type_data #(#add_derive_type_datas),* #( tds.insert(#add_type_datas).unwrap(); diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index 9b7ac7f01b..6a02d905c9 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -20,6 +20,30 @@ fn is_simple_named_attr(attr: &venial::Attribute, name: &str) -> bool { && attr.get_value_tokens().is_empty() } +/// Attribute adding extra functionality for networking. +/// +/// For example, provides sugar for `#[derive_type_data(SchemaDesyncHash)]`, which +/// opts in [`SchemaRef`] to support [`DesyncHash`], so it maybe be included in hash for desync detection. +#[proc_macro_attribute] +pub fn net(_attr: TokenStream, item: TokenStream) -> TokenStream { + // Error if #[net] is used with #[schema(no_clone)]. + let input = venial::parse_declaration(item.clone().into()).unwrap(); + + if let Some(schema_attr) = input + .attributes() + .iter() + .find(|attr| attr.path.len() == 1 && attr.path[0].to_string().as_str() == "schema") + { + if let venial::AttributeValue::Group(_, value) = &schema_attr.value { + if value.iter().any(|f| f.to_string().as_str() == "no_clone") { + panic!("#[net] was used with #[schema(no_clone)]. A Schema that cannot be cloned will never be deterministic in network play."); + } + }; + } + + item +} + /// Derive macro for deriving [`Deref`] on structs with one field. #[proc_macro_derive(Deref, attributes(deref))] pub fn derive_deref(input: TokenStream) -> TokenStream { From f3f6eca5181ab87580b83adb31f452a0cb58595b Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:22:57 -0700 Subject: [PATCH 06/29] feat: Basic tests for desync hash --- framework_crates/bones_utils/Cargo.toml | 4 + framework_crates/bones_utils/tests/tests.rs | 122 ++++++++++++++++++++ 2 files changed, 126 insertions(+) create mode 100644 framework_crates/bones_utils/tests/tests.rs diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index 9022ed170f..6a26c50cfa 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -33,3 +33,7 @@ paste = { version = "1.0" } # when compiling for WASM. [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom = { version = "0.2", features = ["js"] } + +[dev-dependencies] +bones_schema = { version = "0.3", path = "../bones_schema" } +bones_utils = { version = "0.3", path = ".", features = ["glam"] } diff --git a/framework_crates/bones_utils/tests/tests.rs b/framework_crates/bones_utils/tests/tests.rs new file mode 100644 index 0000000000..ef62e564ca --- /dev/null +++ b/framework_crates/bones_utils/tests/tests.rs @@ -0,0 +1,122 @@ +use std::hash::Hasher; + +use fxhash::FxHasher; +use glam::Vec3; + +use bones_schema::prelude::*; +use bones_utils::{net, DesyncHash}; + +#[derive(HasSchema, DesyncHash, Debug, Clone, Default)] +#[desync_hash_module(crate)] +#[net] +struct StructA { + a: f32, + b: String, +} + +#[derive(HasSchema, DesyncHash, Debug, Clone, Default)] +#[desync_hash_module(crate)] +struct StructB { + a: f32, + b: String, +} + +/// Test DesyncHash proc macro on Enum variants +#[derive(HasSchema, DesyncHash, Debug, Clone, Default)] +#[repr(C, u8)] +#[desync_hash_module(crate)] +#[allow(dead_code)] +enum EnumA { + #[default] + A, + B, + C(), + D(f32, u8), + E { + a: f64, + b: u16, + }, + F = 52, +} + +fn hash_value(value: &T) -> u64 { + let mut hasher = FxHasher::default(); + DesyncHash::hash(value, &mut hasher); + hasher.finish() +} + +#[test] +fn desync_hash_enum() { + let a = EnumA::A; + let b = EnumA::B; + + // ensure enum variants do not hash to same value + assert_ne!(hash_value(&a), hash_value(&b)); + + // verify mutating field of tuple variant gives different hash + let d1 = EnumA::D(16.0, 3); + let d2 = EnumA::D(16.0, 2); + assert_ne!(hash_value(&d1), hash_value(&d2)); + + // verify mutating field of named struct variant gives different hash + let e1 = EnumA::E { a: 1.0, b: 2 }; + let e2 = EnumA::E { a: 1.0, b: 1 }; + assert_ne!(hash_value(&e1), hash_value(&e2)); +} + +#[test] +fn desync_hash_struct() { + let a = StructA { + a: 1.0, + b: "foo".to_string(), + }; + let b = StructA { + a: 1.0, + b: "bar".to_string(), + }; + + assert_ne!(hash_value(&a), hash_value(&b)); +} + +#[test] +fn desync_hash_glam() { + let a = Vec3::new(1.0, 2.0, 3.0); + let b = Vec3::new(1.0, 1.0, 1.0); + + assert_ne!(hash_value(&a), hash_value(&b)); +} + +#[test] +fn desync_hash_schemaref() { + // Test that these hash to different values, StructA + // has SchemaDesyncHash typedata. + let a = StructA { + a: 1.0, + b: "foo".to_string(), + }; + let b = StructA { + a: 1.0, + b: "bar".to_string(), + }; + let a_hash = hash_value(&a.as_schema_ref()); + let b_hash = hash_value(&b.as_schema_ref()); + assert_ne!(a_hash, b_hash); + + // StructB does not have SchemaDesyncHash typedata, + // its SchemaRef does not have impl for DesyncHash, + // even if data is different, should just get 0. + let a = StructB { + a: 1.0, + b: "foo".to_string(), + }; + let b = StructB { + a: 1.0, + b: "bar".to_string(), + }; + let a_hash = hash_value(&a.as_schema_ref()); + let b_hash = hash_value(&b.as_schema_ref()); + // Test that these hash to differnet values, StructA + // has SchemaDesyncHash typedata. + assert_eq!(a_hash, b_hash); + assert_eq!(a_hash, 0); +} From 8de5ab3b546a15c054904c2b294f549a31777ba3 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:26:41 -0700 Subject: [PATCH 07/29] feat: Impl DesyncHash for World, Resources, ComponentStores, etc --- framework_crates/bones_ecs/Cargo.toml | 2 +- framework_crates/bones_ecs/src/components.rs | 19 +++++++++++ .../bones_ecs/src/components/untyped.rs | 8 +++++ framework_crates/bones_ecs/src/resources.rs | 32 +++++++++++++++++++ framework_crates/bones_ecs/src/world.rs | 7 ++++ framework_crates/bones_schema/src/registry.rs | 7 ++++ 6 files changed, 74 insertions(+), 1 deletion(-) diff --git a/framework_crates/bones_ecs/Cargo.toml b/framework_crates/bones_ecs/Cargo.toml index d1720e8d34..605828b892 100644 --- a/framework_crates/bones_ecs/Cargo.toml +++ b/framework_crates/bones_ecs/Cargo.toml @@ -14,7 +14,7 @@ keywords.workspace = true default = ["derive", "keysize16"] miri = ["derive", "keysize10"] derive = ["dep:bones_ecs_macros"] -glam = ["dep:glam", "dep:paste", "bones_schema/glam"] +glam = ["dep:glam", "dep:paste", "bones_schema/glam", "bones_utils/glam"] serde = ["dep:serde"] keysize10 = [] diff --git a/framework_crates/bones_ecs/src/components.rs b/framework_crates/bones_ecs/src/components.rs index 6a014ee4c9..ec86c7b67d 100644 --- a/framework_crates/bones_ecs/src/components.rs +++ b/framework_crates/bones_ecs/src/components.rs @@ -47,6 +47,25 @@ impl Clone for ComponentStores { } } +impl DesyncHash for ComponentStores { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for (_, component_store) in self.components.read_only_view().iter() { + // Verify Schema for component store implement desync hash. If no hash_fn, while the + // components will not impact hash, we should probably not hash the schema_id in this case. + let component_store = component_store.as_ref().borrow(); + if component_store + .schema() + .type_data + .get::() + .is_some() + { + component_store.schema().full_name.hash(hasher); + component_store.hash(hasher); + } + } + } +} + impl ComponentStores { /// Get the components of a certain type pub fn get_cell(&self) -> AtomicComponentStore { diff --git a/framework_crates/bones_ecs/src/components/untyped.rs b/framework_crates/bones_ecs/src/components/untyped.rs index 0ae0a3cdc0..039edac423 100644 --- a/framework_crates/bones_ecs/src/components/untyped.rs +++ b/framework_crates/bones_ecs/src/components/untyped.rs @@ -73,6 +73,14 @@ impl Drop for UntypedComponentStore { } } +impl DesyncHash for UntypedComponentStore { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for component in self.iter() { + DesyncHash::hash(&component, hasher); + } + } +} + impl UntypedComponentStore { /// Create a arbitrary [`UntypedComponentStore`]. /// diff --git a/framework_crates/bones_ecs/src/resources.rs b/framework_crates/bones_ecs/src/resources.rs index 883d9fcc1d..e3e1397d99 100644 --- a/framework_crates/bones_ecs/src/resources.rs +++ b/framework_crates/bones_ecs/src/resources.rs @@ -24,6 +24,14 @@ impl std::fmt::Debug for UntypedResource { } } +impl DesyncHash for UntypedResource { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + if let Some(schema_box) = self.cell.borrow().as_ref() { + DesyncHash::hash(&schema_box.as_ref(), hasher); + } + } +} + impl UntypedResource { /// Initialize a new, empty [`UntypedResource`]. pub fn empty(schema: &'static Schema) -> Self { @@ -136,6 +144,24 @@ impl Clone for UntypedResources { } } +impl DesyncHash for UntypedResources { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for (schema_id, resource_cell) in self.resources.read_only_view().iter() { + let is_shared = self.shared_resources.contains_key(schema_id); + + if !is_shared { + // Verify Schema for resource implement desync hash. If no hash fn, + // we should avoid hashing the schema too. + let schema = resource_cell.schema(); + if schema.type_data.get::().is_some() { + schema.full_name.hash(hasher); + resource_cell.hash(hasher); + } + } + } + } +} + /// Error thrown when a resource cell cannot be inserted because it already exists. #[derive(Debug, Clone, Copy)] pub struct CellAlreadyPresentError; @@ -210,6 +236,12 @@ pub struct Resources { untyped: UntypedResources, } +impl DesyncHash for Resources { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + self.untyped.hash(hasher); + } +} + impl Resources { /// Create an empty [`Resources`]. pub fn new() -> Self { diff --git a/framework_crates/bones_ecs/src/world.rs b/framework_crates/bones_ecs/src/world.rs index 9e6ffbd655..85c4b2709c 100644 --- a/framework_crates/bones_ecs/src/world.rs +++ b/framework_crates/bones_ecs/src/world.rs @@ -36,6 +36,13 @@ impl Default for World { } } +impl DesyncHash for World { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + self.components.hash(hasher); + self.resources.hash(hasher); + } +} + impl World { /// Create a new [`World`]. pub fn new() -> Self { diff --git a/framework_crates/bones_schema/src/registry.rs b/framework_crates/bones_schema/src/registry.rs index 76b6dcd7e3..0f3273c5a9 100644 --- a/framework_crates/bones_schema/src/registry.rs +++ b/framework_crates/bones_schema/src/registry.rs @@ -16,6 +16,13 @@ pub struct SchemaId { id: u32, } +impl SchemaId { + /// Get schema id + pub fn id(&self) -> u32 { + self.id + } +} + // Note: The schema type is here in the registry module to prevent modification of registered // schemas by other modules. The idea is that once a schema is registered, it is unchangable and // "certified" so to speak. From 990925bd5836adabf9523b69dcd671c17f2ff9e7 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:27:18 -0700 Subject: [PATCH 08/29] feat: Expose ggrs desync setting, allow overriding of hash function. Hook up desync detection. --- .../bones_framework/src/networking.rs | 54 +++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index 07a6f69aeb..c423382927 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -8,10 +8,11 @@ use crate::networking::online::OnlineMatchmakerResponse; pub use crate::networking::random::RngGenerator; use crate::prelude::*; use bones_matchmaker_proto::{MATCH_ALPN, PLAY_ALPN}; -use ggrs::P2PSession; +use fxhash::FxHasher; +use ggrs::{DesyncDetection, P2PSession}; use instant::Duration; use once_cell::sync::Lazy; -use std::{fmt::Debug, marker::PhantomData, sync::Arc}; +use std::{fmt::Debug, hash::Hasher, marker::PhantomData, sync::Arc}; use tracing::{debug, error, info, trace, warn}; #[cfg(feature = "net-debug")] @@ -550,6 +551,17 @@ pub struct GgrsSessionRunner<'a, InputTypes: NetworkInputConfig<'a>> { /// The random seed used for this session pub random_seed: u64, + + /// Interval in frames of how often to hash state and check for desync with other clients. + /// i.e if set to 10, will check every 10th frame. Desync detection disabled if None. + pub detect_desyncs: Option, + + /// Override of hash function used to hash world for desync detection. + /// By default, [`World`]'s [`DesyncHash`] impl is used. + /// + /// This may be useful if you want to hash only a subset of components or resources + /// during testing. + pub world_hash_func: Option u64>, } /// The info required to create a [`GgrsSessionRunner`]. @@ -572,8 +584,17 @@ pub struct GgrsSessionRunnerInfo { /// /// `None` will use Bone's default. pub local_input_delay: Option, + /// The random seed used for this session pub random_seed: u64, + + /// Interval in frames of how often to hash state and check for desync with other clients. + /// i.e if set to 10, will check every 10th frame. Desync detection disabled if None. + pub detect_desyncs: Option, + + /// Override of hash function used to hash world for desync detection. + /// By default, [`World`]'s [`DesyncHash`] impl is used. + pub world_hash_func: Option u64>, } impl GgrsSessionRunnerInfo { @@ -583,6 +604,8 @@ impl GgrsSessionRunnerInfo { max_prediction_window: Option, local_input_delay: Option, random_seed: u64, + world_hash_func: Option u64>, + detect_desyncs: Option, ) -> Self { let player_idx = socket.player_idx(); let player_count = socket.player_count(); @@ -593,6 +616,8 @@ impl GgrsSessionRunnerInfo { max_prediction_window, local_input_delay, random_seed, + detect_desyncs, + world_hash_func, } } } @@ -662,11 +687,17 @@ where .try_send(NetworkDebugMessage::SetMaxPrediction(max_prediction)) .unwrap(); + let desync_detection = match info.detect_desyncs { + Some(interval) => DesyncDetection::On { interval }, + None => DesyncDetection::Off, + }; + let mut builder = ggrs::SessionBuilder::new() .with_num_players(info.player_count as usize) .with_input_delay(local_input_delay) .with_fps(network_fps) .unwrap() + .with_desync_detection_mode(desync_detection) .with_max_prediction_window(max_prediction) .unwrap(); @@ -700,6 +731,8 @@ where local_input_delay, local_input_disabled: false, random_seed: info.random_seed, + detect_desyncs: info.detect_desyncs, + world_hash_func: info.world_hash_func, } } } @@ -870,7 +903,20 @@ where for request in requests { match request { ggrs::GgrsRequest::SaveGameState { cell, frame } => { - cell.save(frame, Some(world.clone()), None) + // If desync detection enabled, hash world. + let checksum = if self.detect_desyncs.is_some() { + if let Some(hash_func) = self.world_hash_func { + Some(hash_func(world) as u128) + } else { + let mut hasher = FxHasher::default(); + world.hash(&mut hasher); + Some(hasher.finish() as u128) + } + } else { + None + }; + + cell.save(frame, Some(world.clone()), checksum); } ggrs::GgrsRequest::LoadGameState { cell, frame } => { // Swap out sessions to preserve them after world save. @@ -1027,6 +1073,8 @@ where max_prediction_window: Some(self.session.max_prediction()), local_input_delay: Some(self.local_input_delay), random_seed: self.random_seed, + detect_desyncs: self.detect_desyncs, + world_hash_func: self.world_hash_func, }; *self = GgrsSessionRunner::new(Some(self.original_fps as f32), runner_info); } From 3c7fc2e089ca05087a6a1601e017bb124891b7a8 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:27:44 -0700 Subject: [PATCH 09/29] feat: Enable desync detect on transform --- framework_crates/bones_framework/src/render/transform.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/framework_crates/bones_framework/src/render/transform.rs b/framework_crates/bones_framework/src/render/transform.rs index c42c17a196..a8e6c7d094 100644 --- a/framework_crates/bones_framework/src/render/transform.rs +++ b/framework_crates/bones_framework/src/render/transform.rs @@ -5,7 +5,8 @@ use crate::prelude::*; /// The main transform component. /// /// Currently we don't have a hierarchy, and this is therefore a global transform. -#[derive(Clone, Copy, Debug, HasSchema)] +#[derive(Clone, Copy, Debug, HasSchema, DesyncHash, Serialize)] +#[net] #[repr(C)] pub struct Transform { /// The position of the entity in the world. From a311425a49887c7edda87413dbda56d9baa13ed7 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 11 Sep 2024 20:55:08 -0700 Subject: [PATCH 10/29] chore: Fixup docs --- framework_crates/bones_schema/src/desync_hash.rs | 5 ++++- framework_crates/bones_utils/src/desync_hash.rs | 6 +++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/framework_crates/bones_schema/src/desync_hash.rs b/framework_crates/bones_schema/src/desync_hash.rs index f72ec37ed1..0ad9ae90fc 100644 --- a/framework_crates/bones_schema/src/desync_hash.rs +++ b/framework_crates/bones_schema/src/desync_hash.rs @@ -1,4 +1,7 @@ //! Implementation of [`DesyncHash`] for [`SchemaRef`]. +//! +//! SchemaRef's DesyncHash impl calls a function pointer that is optional. It can be opted in +//! by adding `#[net]`. use std::{any::type_name, hash::Hasher}; @@ -6,7 +9,7 @@ use bones_utils::DesyncHash; use crate::{prelude::*, ptr::SchemaRef, FromType, HasSchema, Schema, SchemaData}; -/// Used in [`Schema`] [`TypeDatas`] to optionally implement desync hash. +/// Used in [`Schema`] `TypeDatas` to optionally implement desync hash. pub struct SchemaDesyncHash { /// Desync hash fn pointer pub desync_hash_fn: for<'a> fn(SchemaRef<'a>, hasher: &mut dyn Hasher), diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 13ada56e95..2d532338b7 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -1,6 +1,6 @@ //! [`DesyncHash`] trait and impls, for detecting net desync. -// -// In order to use [`DesyncHash`] with [`glam`] types, the "glam" feature flag must be used. +//! +//! In order to use [`DesyncHash`] with [`glam`] types, the "glam" feature flag must be used. use std::time::Duration; @@ -8,7 +8,7 @@ use ustr::Ustr; /// [`DesyncHash`] is used to hash type and compare over network to detect desyncs. /// -/// In order to opt in a [`HasSchema`] Component or Resource to be included in hash of World in networked session, +/// In order to opt in a `HasSchema` Component or Resource to be included in hash of World in networked session, /// `#[net]` or `#[derive_type_data(SchemaDesyncHas)]` must also be included. pub trait DesyncHash { /// Update hasher from type's values From ab632ca7c760aece48880a619e9850b3a5f5be06 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sat, 14 Sep 2024 16:14:04 -0700 Subject: [PATCH 11/29] feat: wip desync tree --- framework_crates/bones_ecs/src/components.rs | 22 ++++ .../bones_ecs/src/components/untyped.rs | 23 ++++ framework_crates/bones_ecs/src/resources.rs | 85 ++++++++++++-- framework_crates/bones_ecs/src/world.rs | 18 +++ .../bones_utils/src/desync_hash.rs | 105 ++++++++++++++++++ 5 files changed, 243 insertions(+), 10 deletions(-) diff --git a/framework_crates/bones_ecs/src/components.rs b/framework_crates/bones_ecs/src/components.rs index ec86c7b67d..138f0b8ef7 100644 --- a/framework_crates/bones_ecs/src/components.rs +++ b/framework_crates/bones_ecs/src/components.rs @@ -66,6 +66,28 @@ impl DesyncHash for ComponentStores { } } +impl BuildDesyncNode for ComponentStores { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + let mut hasher = H::default(); + + let child_nodes = self + .components + .read_only_view() + .iter() + .map(|(_, component_store)| { + let component_store = component_store.as_ref().borrow(); + let child_node = component_store.desync_tree_node::(); + // Update parent node hash from data + DesyncHash::hash(&child_node.get_hash(), &mut hasher); + + child_node + }) + .collect(); + + DefaultDesyncTreeNode::new(hasher.finish(), Some("Components".into()), child_nodes) + } +} + impl ComponentStores { /// Get the components of a certain type pub fn get_cell(&self) -> AtomicComponentStore { diff --git a/framework_crates/bones_ecs/src/components/untyped.rs b/framework_crates/bones_ecs/src/components/untyped.rs index 039edac423..011c3289cf 100644 --- a/framework_crates/bones_ecs/src/components/untyped.rs +++ b/framework_crates/bones_ecs/src/components/untyped.rs @@ -81,6 +81,29 @@ impl DesyncHash for UntypedComponentStore { } } +impl BuildDesyncNode for UntypedComponentStore { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + let mut hasher = H::default(); + let child_nodes = self + .iter() + .map(|component| -> DefaultDesyncTreeNode { + let hash = component.compute_hash::(); + + // Update parent node hash from data + DesyncHash::hash(&component, &mut hasher); + + DefaultDesyncTreeNode::new(hash, None, vec![]) + }) + .collect(); + + DefaultDesyncTreeNode::new( + hasher.finish(), + Some(self.schema().full_name.to_string()), + child_nodes, + ) + } +} + impl UntypedComponentStore { /// Create a arbitrary [`UntypedComponentStore`]. /// diff --git a/framework_crates/bones_ecs/src/resources.rs b/framework_crates/bones_ecs/src/resources.rs index e3e1397d99..ba9b39fb1b 100644 --- a/framework_crates/bones_ecs/src/resources.rs +++ b/framework_crates/bones_ecs/src/resources.rs @@ -2,6 +2,7 @@ use std::{fmt::Debug, marker::PhantomData, sync::Arc}; +use fxhash::FxHasher; use once_map::OnceMap; use crate::prelude::*; @@ -27,11 +28,26 @@ impl std::fmt::Debug for UntypedResource { impl DesyncHash for UntypedResource { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { if let Some(schema_box) = self.cell.borrow().as_ref() { + DesyncHash::hash(&schema_box.schema().full_name, hasher); DesyncHash::hash(&schema_box.as_ref(), hasher); } } } +impl BuildDesyncNode for UntypedResource { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + let name = Some(self.schema().full_name.to_string()); + + if let Some(schema_box) = self.cell.borrow().as_ref() { + let hash = schema_box.as_ref().compute_hash::(); + return DefaultDesyncTreeNode::new(hash, name, vec![]); + } + + // TODO should we only optionally provide node? + DefaultDesyncTreeNode::new(0, name, vec![]) + } +} + impl UntypedResource { /// Initialize a new, empty [`UntypedResource`]. pub fn empty(schema: &'static Schema) -> Self { @@ -146,19 +162,62 @@ impl Clone for UntypedResources { impl DesyncHash for UntypedResources { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { - for (schema_id, resource_cell) in self.resources.read_only_view().iter() { - let is_shared = self.shared_resources.contains_key(schema_id); + let mut child_hashes: Vec = self + .resources + .read_only_view() + .iter() + .filter_map(|(schema_id, resource_cell)| { + let is_shared = self.shared_resources.contains_key(schema_id); + + if !is_shared { + // Only build child node if hashable + let schema = resource_cell.schema(); + if schema.type_data.get::().is_some() { + return Some(resource_cell.compute_hash::()); + } + } + None + }) + .collect(); - if !is_shared { - // Verify Schema for resource implement desync hash. If no hash fn, - // we should avoid hashing the schema too. - let schema = resource_cell.schema(); - if schema.type_data.get::().is_some() { - schema.full_name.hash(hasher); - resource_cell.hash(hasher); + child_hashes.sort(); + + for hash in child_hashes { + // Update parent hash + hash.hash(hasher); + } + } +} + +impl BuildDesyncNode for UntypedResources { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + let mut hasher = H::default(); + let mut child_nodes: Vec = self + .resources + .read_only_view() + .iter() + .filter_map(|(schema_id, resource_cell)| { + let is_shared = self.shared_resources.contains_key(schema_id); + + if !is_shared { + // Only build child node if hashable + let schema = resource_cell.schema(); + if schema.type_data.get::().is_some() { + return Some(resource_cell.desync_tree_node::()); + } } - } + None + }) + .collect(); + + child_nodes.sort(); + + for node in child_nodes.iter() { + // Update parent hash + DesyncHash::hash(&node.get_hash(), &mut hasher); } + + DefaultDesyncTreeNode::new(hasher.finish(), Some("Resources".into()), child_nodes) } } @@ -242,6 +301,12 @@ impl DesyncHash for Resources { } } +impl BuildDesyncNode for Resources { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + self.untyped.desync_tree_node::() + } +} + impl Resources { /// Create an empty [`Resources`]. pub fn new() -> Self { diff --git a/framework_crates/bones_ecs/src/world.rs b/framework_crates/bones_ecs/src/world.rs index 85c4b2709c..8518ab370d 100644 --- a/framework_crates/bones_ecs/src/world.rs +++ b/framework_crates/bones_ecs/src/world.rs @@ -43,6 +43,24 @@ impl DesyncHash for World { } } +impl BuildDesyncNode for World { + fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + let mut hasher = H::default(); + + let mut child_nodes: Vec = vec![]; + + let components_node = self.components.desync_tree_node::(); + components_node.get_hash().hash(&mut hasher); + child_nodes.push(components_node); + + let resources_node = self.resources.desync_tree_node::(); + resources_node.get_hash().hash(&mut hasher); + child_nodes.push(resources_node); + + DefaultDesyncTreeNode::new(hasher.finish(), Some("World".into()), child_nodes) + } +} + impl World { /// Create a new [`World`]. pub fn new() -> Self { diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 2d532338b7..939dee6ff0 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -15,6 +15,111 @@ pub trait DesyncHash { fn hash(&self, hasher: &mut dyn std::hash::Hasher); } +/// Extension of [`DesyncHash`] that is automatically implemented for `T: DesyncHash`. +/// Adds helper to compute standalone hash instead of updating a hasher. +pub trait DesyncHashImpl { + /// Compute hash of type with provided hasher. + fn compute_hash(&self) -> u64; +} + +impl DesyncHashImpl for T { + fn compute_hash(&self) -> u64 { + let mut hasher = H::default(); + self.hash(&mut hasher); + hasher.finish() + } +} + +/// Tree of desync hashes +pub trait DesyncTree: Clone { + type Node; + + fn get_hash(&self) -> V; + + fn name(&self) -> &Option; + + fn from_root(root: Self::Node) -> Self; +} + +/// [`DesyncTree`] node trait, built from children and hash. A node is effectively a sub-tree, +/// as we build the tree bottom-up. +pub trait DesyncTreeNode: Clone + PartialEq + Eq { + fn new(hash: u64, name: Option, children: Vec) -> Self; + + fn get_hash(&self) -> V; +} + +/// Implement to allow type to create a [`DesyncTreeNode`] containing hash built from children. +pub trait BuildDesyncNode +where + N: DesyncTreeNode, +{ + fn desync_tree_node(&self) -> N; +} + +/// Default impl for [`DesyncTreeNode`]. +#[derive(Clone, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct DefaultDesyncTreeNode { + name: Option, + hash: u64, + children: Vec, +} + +impl PartialOrd for DefaultDesyncTreeNode { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl Ord for DefaultDesyncTreeNode { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.hash.cmp(&other.hash) + } +} + +impl DesyncTreeNode for DefaultDesyncTreeNode { + fn new(hash: u64, name: Option, children: Vec) -> Self { + Self { + name, + hash, + children, + } + } + + fn get_hash(&self) -> u64 { + self.hash + } +} + +/// Tree of desync hashes, allows storing hash of world and children such as components and resources. +#[derive(Clone)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct DefaultDesyncTree { + root: DefaultDesyncTreeNode, +} + +impl From for DefaultDesyncTree { + fn from(value: DefaultDesyncTreeNode) -> Self { + Self::from_root(value) + } +} + +impl DesyncTree for DefaultDesyncTree { + type Node = DefaultDesyncTreeNode; + + fn get_hash(&self) -> u64 { + self.root.get_hash() + } + + fn name(&self) -> &Option { + &self.root.name + } + + fn from_root(root: Self::Node) -> Self { + Self { root } + } +} impl DesyncHash for Duration { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { self.as_nanos().hash(hasher); From c61248ffe9140376baaedc33cb47249936a16ce7 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sat, 14 Sep 2024 23:20:09 -0700 Subject: [PATCH 12/29] feat: wip dump hash tree on detected desync --- framework_crates/bones_asset/src/lib.rs | 13 ++++ framework_crates/bones_ecs/src/components.rs | 69 +++++++++++++------ .../bones_ecs/src/components/untyped.rs | 1 + framework_crates/bones_framework/Cargo.toml | 4 +- .../bones_framework/src/networking.rs | 46 +++++++++++++ .../bones_framework/src/networking/desync.rs | 66 ++++++++++++++++++ 6 files changed, 177 insertions(+), 22 deletions(-) create mode 100644 framework_crates/bones_framework/src/networking/desync.rs diff --git a/framework_crates/bones_asset/src/lib.rs b/framework_crates/bones_asset/src/lib.rs index 3c8c94e7b2..92aa7b1f6f 100644 --- a/framework_crates/bones_asset/src/lib.rs +++ b/framework_crates/bones_asset/src/lib.rs @@ -5,6 +5,7 @@ #![cfg_attr(doc, allow(unknown_lints))] #![deny(rustdoc::all)] +use bones_utils::DesyncHash; use serde::{de::DeserializeSeed, Deserializer}; /// Helper to export the same types in the crate root and in the prelude. @@ -282,6 +283,18 @@ impl From> for Maybe { } } +impl DesyncHash for Maybe { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + match self { + Maybe::Unset => 0.hash(hasher), + Maybe::Set(value) => { + 1.hash(hasher); + value.hash(hasher) + } + } + } +} + fn maybe_loader( ctx: &mut MetaAssetLoadCtx, ptr: SchemaRefMut<'_>, diff --git a/framework_crates/bones_ecs/src/components.rs b/framework_crates/bones_ecs/src/components.rs index 138f0b8ef7..c39a137981 100644 --- a/framework_crates/bones_ecs/src/components.rs +++ b/framework_crates/bones_ecs/src/components.rs @@ -1,7 +1,8 @@ //! ECS component storage. +use fxhash::FxHasher; use once_map::OnceMap; -use std::sync::Arc; +use std::{any::Any, sync::Arc}; use crate::prelude::*; @@ -49,19 +50,33 @@ impl Clone for ComponentStores { impl DesyncHash for ComponentStores { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { - for (_, component_store) in self.components.read_only_view().iter() { - // Verify Schema for component store implement desync hash. If no hash_fn, while the - // components will not impact hash, we should probably not hash the schema_id in this case. - let component_store = component_store.as_ref().borrow(); - if component_store - .schema() - .type_data - .get::() - .is_some() - { - component_store.schema().full_name.hash(hasher); - component_store.hash(hasher); - } + // Compute child hashes and sort + let mut hashes = self + .components + .read_only_view() + .iter() + .filter_map(|(_, component_store)| { + // Verify Schema for component store implement desync hash. If no hash_fn, we don't + // want to add hash. + let component_store = component_store.as_ref().borrow(); + if component_store + .schema() + .type_data + .get::() + .is_some() + { + // We need to compute hashes first + return Some(component_store.compute_hash::()); + } + + None + }) + .collect::>(); + hashes.sort(); + + // Udpate parent hasher from sorted hashes + for hash in hashes.iter() { + hash.hash(hasher); } } } @@ -70,19 +85,31 @@ impl BuildDesyncNode for ComponentStores { fn desync_tree_node(&self) -> DefaultDesyncTreeNode { let mut hasher = H::default(); - let child_nodes = self + let mut child_nodes = self .components .read_only_view() .iter() - .map(|(_, component_store)| { + .filter_map(|(_, component_store)| { let component_store = component_store.as_ref().borrow(); - let child_node = component_store.desync_tree_node::(); - // Update parent node hash from data - DesyncHash::hash(&child_node.get_hash(), &mut hasher); + if component_store + .schema() + .type_data + .get::() + .is_some() + { + let child_node = component_store.desync_tree_node::(); - child_node + return Some(child_node); + } + None }) - .collect(); + .collect::>(); + child_nodes.sort(); + + for node in child_nodes.iter() { + // Update parent node hash from data + DesyncHash::hash(&node.get_hash(), &mut hasher); + } DefaultDesyncTreeNode::new(hasher.finish(), Some("Components".into()), child_nodes) } diff --git a/framework_crates/bones_ecs/src/components/untyped.rs b/framework_crates/bones_ecs/src/components/untyped.rs index 011c3289cf..3ecd0b5528 100644 --- a/framework_crates/bones_ecs/src/components/untyped.rs +++ b/framework_crates/bones_ecs/src/components/untyped.rs @@ -75,6 +75,7 @@ impl Drop for UntypedComponentStore { impl DesyncHash for UntypedComponentStore { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + self.schema().full_name.hash(hasher); for component in self.iter() { DesyncHash::hash(&component, hasher); } diff --git a/framework_crates/bones_framework/Cargo.toml b/framework_crates/bones_framework/Cargo.toml index 1ba48f2e53..498b037ec9 100644 --- a/framework_crates/bones_framework/Cargo.toml +++ b/framework_crates/bones_framework/Cargo.toml @@ -30,7 +30,9 @@ scripting = ["dep:bones_scripting"] ## Enable networking debug window + frame prediction history. net-debug = ["ui"] -#! ### Audio formats +desync-debug = [] + +#! ### Audio formats## Enables advanced desync debugging, generates hash tree of world for frames #! These features enable different audio formats ## Enable OGG audio loader. diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index c423382927..6ba8d4f5c0 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -8,6 +8,7 @@ use crate::networking::online::OnlineMatchmakerResponse; pub use crate::networking::random::RngGenerator; use crate::prelude::*; use bones_matchmaker_proto::{MATCH_ALPN, PLAY_ALPN}; +use desync::DesyncDebugHistoryBuffer; use fxhash::FxHasher; use ggrs::{DesyncDetection, P2PSession}; use instant::Duration; @@ -23,6 +24,7 @@ use { use crate::input::PlayerControls as PlayerControlsTrait; +pub mod desync; pub mod input; pub mod lan; pub mod online; @@ -556,6 +558,10 @@ pub struct GgrsSessionRunner<'a, InputTypes: NetworkInputConfig<'a>> { /// i.e if set to 10, will check every 10th frame. Desync detection disabled if None. pub detect_desyncs: Option, + /// History buffer for desync debug data to fetch it upon detected desyncs. + /// [`DefaultDesyncTree`] will be generated and saved here if feature `desync-debug` is enabled. + pub desync_debug_history: Option>, + /// Override of hash function used to hash world for desync detection. /// By default, [`World`]'s [`DesyncHash`] impl is used. /// @@ -716,6 +722,14 @@ where let session = builder.start_p2p_session(info.socket.clone()).unwrap(); + #[cfg(feature = "desync-debug")] + let desync_debug_history = info + .detect_desyncs + .map(DesyncDebugHistoryBuffer::::new); + + #[cfg(not(feature = "desync-debug"))] + let desync_debug_history = None; + Self { last_player_input: InputTypes::Dense::default(), session, @@ -732,6 +746,7 @@ where local_input_disabled: false, random_seed: info.random_seed, detect_desyncs: info.detect_desyncs, + desync_debug_history, world_hash_func: info.world_hash_func, } } @@ -855,6 +870,19 @@ where addr, } => { error!(%frame, %local_checksum, %remote_checksum, player=%addr, "Network de-sync detected"); + + #[cfg(feature = "desync-debug")] + { + if let Some(desync_debug_history) = &self.desync_debug_history { + if let Some(desync_hash_tree) = + desync_debug_history.get_frame_data(frame as u32) + { + let string = serde_yaml::to_string(desync_hash_tree) + .expect("Failed to serialize desync hash tree"); + error!("Desync hash tree: frame: {frame}\n{}", string); + } + } + } } } } @@ -903,8 +931,26 @@ where for request in requests { match request { ggrs::GgrsRequest::SaveGameState { cell, frame } => { + // TODO: Do we only need to compute hash for desync interval frames? + // GGRS should only use hashes from fixed interval. + // If desync detection enabled, hash world. let checksum = if self.detect_desyncs.is_some() { + #[cfg(feature = "desync-debug")] + { + if let Some(desync_debug_history) = + &mut self.desync_debug_history + { + if desync_debug_history + .is_desync_detect_frame(frame as u32) + { + desync_debug_history.record( + frame as u32, + world.desync_tree_node::().into(), + ); + } + } + } if let Some(hash_func) = self.world_hash_func { Some(hash_func(world) as u128) } else { diff --git a/framework_crates/bones_framework/src/networking/desync.rs b/framework_crates/bones_framework/src/networking/desync.rs new file mode 100644 index 0000000000..8fd5abbee3 --- /dev/null +++ b/framework_crates/bones_framework/src/networking/desync.rs @@ -0,0 +1,66 @@ +use std::collections::VecDeque; + +use bones_lib::prelude::default; + +/// Max frames of data in desync history buffer - this is set to match `ggrs::MAX_CHECKSUM_HISTORY_SIZE`, +/// but is private so cannot be used directly. +const MAX_DESYNC_HISTORY_BUFFER: usize = 32; + +/// Store history of desync detection data, such as a [`DesyncTree`]. When ggrs finds a desync in past, +/// we can retrieve this data for debugging. Ggrs has a fixed limit of pending desync frames it tests, +/// so we match it by keeping the last [`MAX_DESYNC_HISTORY_BUFFER`] of frame data at the desync detect interval. +/// +/// Desync data provided in `record` will only be saved if frame coincides with desync detect interval, otherwise +/// ggrs will never test this frame, and we do not need to buffer it. +pub struct DesyncDebugHistoryBuffer { + buffer: VecDeque<(u32, T)>, + + /// Desync detection interval, should match ggrs session config. + desync_detect_interval: u32, +} + +impl DesyncDebugHistoryBuffer { + /// Create buffer, use same desync detect interval configured on ggrs session. + pub fn new(desync_detect_interval: u32) -> Self { + Self { + desync_detect_interval, + buffer: default(), + } + } + + /// Check if this frame coincides with desync detection interval. + /// If not, we will not perform desync checks on it, and do not need to record history for frame. + pub fn is_desync_detect_frame(&self, frame: u32) -> bool { + // GGRS sends desync detections every X frames where X is interval, and first frame is interval. + frame % self.desync_detect_interval == 0 + } + + /// Get desync data for frame if it is available. + pub fn get_frame_data(&self, frame: u32) -> Option<&T> { + // Don't bother looking for data if not a desync detect frame + if !self.is_desync_detect_frame(frame) { + return None; + } + + self.buffer.iter().find_map(|d| { + if d.0 == frame { + return Some(&d.1); + } + None + }) + } + + /// Possibly record frame and desync data. It is only recorded on frames matching + /// desync detect interval, as ggrs will not check for desyns otherwise and we don't + /// need to save it. + pub fn record(&mut self, frame: u32, desync_data: T) { + // Only record if on a frame that will be desync detected. + if self.is_desync_detect_frame(frame) { + while self.buffer.len() >= MAX_DESYNC_HISTORY_BUFFER { + self.buffer.pop_front(); + } + + self.buffer.push_back((frame, desync_data)); + } + } +} From f4c1f437f519f987ce2f26af264e70eed7730d00 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 15 Sep 2024 10:14:15 -0700 Subject: [PATCH 13/29] chore: Fix bumping bones to 0.4 --- framework_crates/bones_utils/Cargo.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index 6a26c50cfa..b240148649 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -35,5 +35,5 @@ paste = { version = "1.0" } getrandom = { version = "0.2", features = ["js"] } [dev-dependencies] -bones_schema = { version = "0.3", path = "../bones_schema" } -bones_utils = { version = "0.3", path = ".", features = ["glam"] } +bones_schema = { version = "0.4", path = "../bones_schema" } +bones_utils = { version = "0.4", path = ".", features = ["glam"] } From 8e0c0e18cac4361ac76a4c23dc361a23f542246f Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 15 Sep 2024 11:45:01 -0700 Subject: [PATCH 14/29] chore: fix feature comment --- framework_crates/bones_framework/Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/framework_crates/bones_framework/Cargo.toml b/framework_crates/bones_framework/Cargo.toml index 498b037ec9..a558b94275 100644 --- a/framework_crates/bones_framework/Cargo.toml +++ b/framework_crates/bones_framework/Cargo.toml @@ -30,9 +30,10 @@ scripting = ["dep:bones_scripting"] ## Enable networking debug window + frame prediction history. net-debug = ["ui"] +## Enables advanced desync debugging, generates hash tree of world for frames desync-debug = [] -#! ### Audio formats## Enables advanced desync debugging, generates hash tree of world for frames +#! ### Audio formats #! These features enable different audio formats ## Enable OGG audio loader. From 91be293647c45d7f0a59527c9c3271c361d3f7c9 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 15 Sep 2024 15:38:34 -0700 Subject: [PATCH 15/29] fix: impl DesyncHash for arrays --- framework_crates/bones_utils/src/desync_hash.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 939dee6ff0..9504e387e3 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -136,6 +136,14 @@ impl DesyncHash for bool { } } +impl DesyncHash for [T; N] { + fn hash(&self, hasher: &mut dyn std::hash::Hasher) { + for value in self { + DesyncHash::hash(value, hasher); + } + } +} + impl DesyncHash for Vec { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { for value in self { From f6db4b36ba7010fc81f62edd0a36aea981d1ffaf Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Fri, 18 Oct 2024 22:25:01 -0700 Subject: [PATCH 16/29] chore: update lockfile --- Cargo.lock | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index e8d94d77a5..33ccaa0f60 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1609,12 +1609,15 @@ dependencies = [ name = "bones_utils" version = "0.4.0" dependencies = [ + "bones_schema", + "bones_utils", "bones_utils_macros", "fxhash", "getrandom", "glam 0.24.2", "hashbrown 0.14.5", "instant", + "paste", "serde", "turborand", "ulid", @@ -1624,6 +1627,7 @@ dependencies = [ name = "bones_utils_macros" version = "0.4.0" dependencies = [ + "proc-macro2", "quote", "venial", ] From 19de3adb1c76e43ebe48d140cf963927d154d17f Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 15 Sep 2024 15:41:35 -0700 Subject: [PATCH 17/29] feat: Allow tree to include unhashable nodes, make hash optional on node --- framework_crates/bones_ecs/src/components.rs | 39 +++++++---- .../bones_ecs/src/components/untyped.rs | 35 ++++++---- framework_crates/bones_ecs/src/resources.rs | 38 +++++++---- framework_crates/bones_ecs/src/world.rs | 19 ++++-- .../bones_framework/src/networking.rs | 65 +++++++++---------- .../bones_framework/src/networking/desync.rs | 28 +++++++- .../bones_utils/src/desync_hash.rs | 18 ++--- 7 files changed, 158 insertions(+), 84 deletions(-) diff --git a/framework_crates/bones_ecs/src/components.rs b/framework_crates/bones_ecs/src/components.rs index c39a137981..caf6b7ef97 100644 --- a/framework_crates/bones_ecs/src/components.rs +++ b/framework_crates/bones_ecs/src/components.rs @@ -82,22 +82,29 @@ impl DesyncHash for ComponentStores { } impl BuildDesyncNode for ComponentStores { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { - let mut hasher = H::default(); - + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode { + let mut any_hashable = false; let mut child_nodes = self .components .read_only_view() .iter() .filter_map(|(_, component_store)| { let component_store = component_store.as_ref().borrow(); - if component_store + let is_hashable = component_store .schema() .type_data .get::() - .is_some() - { - let child_node = component_store.desync_tree_node::(); + .is_some(); + + if is_hashable { + any_hashable = true; + } + + if include_unhashable || is_hashable { + let child_node = component_store.desync_tree_node::(include_unhashable); return Some(child_node); } @@ -106,12 +113,20 @@ impl BuildDesyncNode for ComponentStores { .collect::>(); child_nodes.sort(); - for node in child_nodes.iter() { - // Update parent node hash from data - DesyncHash::hash(&node.get_hash(), &mut hasher); - } + let hash = if any_hashable { + let mut hasher = H::default(); + for node in child_nodes.iter() { + // Update parent node hash from data + if let Some(hash) = node.get_hash() { + DesyncHash::hash(&hash, &mut hasher); + } + } + Some(hasher.finish()) + } else { + None + }; - DefaultDesyncTreeNode::new(hasher.finish(), Some("Components".into()), child_nodes) + DefaultDesyncTreeNode::new(hash, Some("Components".into()), child_nodes) } } diff --git a/framework_crates/bones_ecs/src/components/untyped.rs b/framework_crates/bones_ecs/src/components/untyped.rs index 3ecd0b5528..ab3d58c917 100644 --- a/framework_crates/bones_ecs/src/components/untyped.rs +++ b/framework_crates/bones_ecs/src/components/untyped.rs @@ -83,25 +83,38 @@ impl DesyncHash for UntypedComponentStore { } impl BuildDesyncNode for UntypedComponentStore { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + fn desync_tree_node( + &self, + _include_unhashable: bool, + ) -> DefaultDesyncTreeNode { let mut hasher = H::default(); - let child_nodes = self + let child_nodes: Vec = self .iter() .map(|component| -> DefaultDesyncTreeNode { - let hash = component.compute_hash::(); - - // Update parent node hash from data - DesyncHash::hash(&component, &mut hasher); + let hash = if component + .schema() + .type_data + .get::() + .is_some() + { + // Update parent node hash from data + DesyncHash::hash(&component, &mut hasher); + Some(component.compute_hash::()) + } else { + None + }; DefaultDesyncTreeNode::new(hash, None, vec![]) }) .collect(); - DefaultDesyncTreeNode::new( - hasher.finish(), - Some(self.schema().full_name.to_string()), - child_nodes, - ) + let hash = if !child_nodes.is_empty() { + Some(hasher.finish()) + } else { + None + }; + + DefaultDesyncTreeNode::new(hash, Some(self.schema().full_name.to_string()), child_nodes) } } diff --git a/framework_crates/bones_ecs/src/resources.rs b/framework_crates/bones_ecs/src/resources.rs index ba9b39fb1b..607710bb46 100644 --- a/framework_crates/bones_ecs/src/resources.rs +++ b/framework_crates/bones_ecs/src/resources.rs @@ -35,16 +35,24 @@ impl DesyncHash for UntypedResource { } impl BuildDesyncNode for UntypedResource { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + fn desync_tree_node( + &self, + _include_unhashable: bool, + ) -> DefaultDesyncTreeNode { let name = Some(self.schema().full_name.to_string()); + let hashable = self.schema().type_data.get::().is_some(); + if let Some(schema_box) = self.cell.borrow().as_ref() { - let hash = schema_box.as_ref().compute_hash::(); + let hash = if hashable { + Some(schema_box.as_ref().compute_hash::()) + } else { + None + }; return DefaultDesyncTreeNode::new(hash, name, vec![]); } - // TODO should we only optionally provide node? - DefaultDesyncTreeNode::new(0, name, vec![]) + DefaultDesyncTreeNode::new(None, name, vec![]) } } @@ -190,7 +198,10 @@ impl DesyncHash for UntypedResources { } impl BuildDesyncNode for UntypedResources { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode { let mut hasher = H::default(); let mut child_nodes: Vec = self .resources @@ -202,8 +213,8 @@ impl BuildDesyncNode for UntypedResources { if !is_shared { // Only build child node if hashable let schema = resource_cell.schema(); - if schema.type_data.get::().is_some() { - return Some(resource_cell.desync_tree_node::()); + if include_unhashable || schema.type_data.get::().is_some() { + return Some(resource_cell.desync_tree_node::(include_unhashable)); } } None @@ -214,10 +225,12 @@ impl BuildDesyncNode for UntypedResources { for node in child_nodes.iter() { // Update parent hash - DesyncHash::hash(&node.get_hash(), &mut hasher); + if let Some(hash) = node.get_hash() { + DesyncHash::hash(&hash, &mut hasher); + } } - DefaultDesyncTreeNode::new(hasher.finish(), Some("Resources".into()), child_nodes) + DefaultDesyncTreeNode::new(Some(hasher.finish()), Some("Resources".into()), child_nodes) } } @@ -302,8 +315,11 @@ impl DesyncHash for Resources { } impl BuildDesyncNode for Resources { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { - self.untyped.desync_tree_node::() + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode { + self.untyped.desync_tree_node::(include_unhashable) } } diff --git a/framework_crates/bones_ecs/src/world.rs b/framework_crates/bones_ecs/src/world.rs index 8518ab370d..eba09cb7ee 100644 --- a/framework_crates/bones_ecs/src/world.rs +++ b/framework_crates/bones_ecs/src/world.rs @@ -44,20 +44,27 @@ impl DesyncHash for World { } impl BuildDesyncNode for World { - fn desync_tree_node(&self) -> DefaultDesyncTreeNode { + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode { let mut hasher = H::default(); let mut child_nodes: Vec = vec![]; - let components_node = self.components.desync_tree_node::(); - components_node.get_hash().hash(&mut hasher); + let components_node = self.components.desync_tree_node::(include_unhashable); + if let Some(hash) = components_node.get_hash() { + hash.hash(&mut hasher); + } child_nodes.push(components_node); - let resources_node = self.resources.desync_tree_node::(); - resources_node.get_hash().hash(&mut hasher); + let resources_node = self.resources.desync_tree_node::(include_unhashable); + if let Some(hash) = resources_node.get_hash() { + hash.hash(&mut hasher); + } child_nodes.push(resources_node); - DefaultDesyncTreeNode::new(hasher.finish(), Some("World".into()), child_nodes) + DefaultDesyncTreeNode::new(Some(hasher.finish()), Some("World".into()), child_nodes) } } diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index 6ba8d4f5c0..b4fc55c6af 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -8,7 +8,7 @@ use crate::networking::online::OnlineMatchmakerResponse; pub use crate::networking::random::RngGenerator; use crate::prelude::*; use bones_matchmaker_proto::{MATCH_ALPN, PLAY_ALPN}; -use desync::DesyncDebugHistoryBuffer; +use desync::{DesyncDebugHistoryBuffer, DetectDesyncs}; use fxhash::FxHasher; use ggrs::{DesyncDetection, P2PSession}; use instant::Duration; @@ -65,7 +65,7 @@ impl From for NetworkInputStatus { /// Module prelude. pub mod prelude { pub use super::{ - input, lan, online, proto, random, DisconnectedPlayers, RngGenerator, SyncingInfo, RUNTIME, + desync::DetectDesyncs, input, lan, online, proto, random, DisconnectedPlayers, RngGenerator, SyncingInfo, RUNTIME, }; #[cfg(feature = "net-debug")] @@ -554,20 +554,12 @@ pub struct GgrsSessionRunner<'a, InputTypes: NetworkInputConfig<'a>> { /// The random seed used for this session pub random_seed: u64, - /// Interval in frames of how often to hash state and check for desync with other clients. - /// i.e if set to 10, will check every 10th frame. Desync detection disabled if None. - pub detect_desyncs: Option, + /// When provided, desync detection is enabled. Contains settings for desync detection. + detect_desyncs: Option, /// History buffer for desync debug data to fetch it upon detected desyncs. /// [`DefaultDesyncTree`] will be generated and saved here if feature `desync-debug` is enabled. pub desync_debug_history: Option>, - - /// Override of hash function used to hash world for desync detection. - /// By default, [`World`]'s [`DesyncHash`] impl is used. - /// - /// This may be useful if you want to hash only a subset of components or resources - /// during testing. - pub world_hash_func: Option u64>, } /// The info required to create a [`GgrsSessionRunner`]. @@ -593,14 +585,10 @@ pub struct GgrsSessionRunnerInfo { /// The random seed used for this session pub random_seed: u64, + + /// When provided, desync detection is enabled. Contains settings for desync detection. + pub detect_desyncs: Option, - /// Interval in frames of how often to hash state and check for desync with other clients. - /// i.e if set to 10, will check every 10th frame. Desync detection disabled if None. - pub detect_desyncs: Option, - - /// Override of hash function used to hash world for desync detection. - /// By default, [`World`]'s [`DesyncHash`] impl is used. - pub world_hash_func: Option u64>, } impl GgrsSessionRunnerInfo { @@ -610,8 +598,7 @@ impl GgrsSessionRunnerInfo { max_prediction_window: Option, local_input_delay: Option, random_seed: u64, - world_hash_func: Option u64>, - detect_desyncs: Option, + detect_desyncs: Option, ) -> Self { let player_idx = socket.player_idx(); let player_count = socket.player_count(); @@ -623,7 +610,6 @@ impl GgrsSessionRunnerInfo { local_input_delay, random_seed, detect_desyncs, - world_hash_func, } } } @@ -693,8 +679,10 @@ where .try_send(NetworkDebugMessage::SetMaxPrediction(max_prediction)) .unwrap(); - let desync_detection = match info.detect_desyncs { - Some(interval) => DesyncDetection::On { interval }, + let desync_detection = match info.detect_desyncs.as_ref() { + Some(config) => DesyncDetection::On { + interval: config.detection_interval, + }, None => DesyncDetection::Off, }; @@ -723,9 +711,13 @@ where let session = builder.start_p2p_session(info.socket.clone()).unwrap(); #[cfg(feature = "desync-debug")] - let desync_debug_history = info - .detect_desyncs - .map(DesyncDebugHistoryBuffer::::new); + let desync_debug_history = if let Some(detect_desync) = info.detect_desyncs.as_ref() { + Some(DesyncDebugHistoryBuffer::::new( + detect_desync.detection_interval, + )) + } else { + None + }; #[cfg(not(feature = "desync-debug"))] let desync_debug_history = None; @@ -747,7 +739,6 @@ where random_seed: info.random_seed, detect_desyncs: info.detect_desyncs, desync_debug_history, - world_hash_func: info.world_hash_func, } } } @@ -935,7 +926,9 @@ where // GGRS should only use hashes from fixed interval. // If desync detection enabled, hash world. - let checksum = if self.detect_desyncs.is_some() { + let checksum = if let Some(detect_desyncs) = + self.detect_desyncs.as_ref() + { #[cfg(feature = "desync-debug")] { if let Some(desync_debug_history) = @@ -944,14 +937,17 @@ where if desync_debug_history .is_desync_detect_frame(frame as u32) { - desync_debug_history.record( - frame as u32, - world.desync_tree_node::().into(), + let tree = DefaultDesyncTree::from( + world.desync_tree_node::( + detect_desyncs.include_unhashable_nodes, + ), ); + desync_debug_history.record(frame as u32, tree); } } } - if let Some(hash_func) = self.world_hash_func { + + if let Some(hash_func) = detect_desyncs.world_hash_func { Some(hash_func(world) as u128) } else { let mut hasher = FxHasher::default(); @@ -1119,8 +1115,7 @@ where max_prediction_window: Some(self.session.max_prediction()), local_input_delay: Some(self.local_input_delay), random_seed: self.random_seed, - detect_desyncs: self.detect_desyncs, - world_hash_func: self.world_hash_func, + detect_desyncs: self.detect_desyncs.clone(), }; *self = GgrsSessionRunner::new(Some(self.original_fps as f32), runner_info); } diff --git a/framework_crates/bones_framework/src/networking/desync.rs b/framework_crates/bones_framework/src/networking/desync.rs index 8fd5abbee3..b5f8156a6b 100644 --- a/framework_crates/bones_framework/src/networking/desync.rs +++ b/framework_crates/bones_framework/src/networking/desync.rs @@ -1,11 +1,37 @@ +//! use std::collections::VecDeque; -use bones_lib::prelude::default; +use bones_lib::{ecs::World, prelude::default}; /// Max frames of data in desync history buffer - this is set to match `ggrs::MAX_CHECKSUM_HISTORY_SIZE`, /// but is private so cannot be used directly. const MAX_DESYNC_HISTORY_BUFFER: usize = 32; +#[derive(Clone)] +pub struct DetectDesyncs { + /// Interval in frames of how often to hash state and check for desync with other clients. + /// i.e if set to 10, will check every 10th frame. + pub detection_interval: u32, + + /// Override of hash function used to hash world for desync detection. + /// By default, [`World`]'s [`DesyncHash`] impl is used. + pub world_hash_func: Option u64>, + + /// When using feature `desync-debug`, a [`DesyncTree`] will be built. Resources and Components + /// that do not support hashing can be optionally included in tree to help highlight candidates + /// to be opted into desync-detection. + pub include_unhashable_nodes: bool, +} + +impl Default for DetectDesyncs { + fn default() -> Self { + Self { + detection_interval: 60, + world_hash_func: None, + include_unhashable_nodes: false, + } + } +} /// Store history of desync detection data, such as a [`DesyncTree`]. When ggrs finds a desync in past, /// we can retrieve this data for debugging. Ggrs has a fixed limit of pending desync frames it tests, /// so we match it by keeping the last [`MAX_DESYNC_HISTORY_BUFFER`] of frame data at the desync detect interval. diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 9504e387e3..5cdfee3c27 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -34,7 +34,7 @@ impl DesyncHashImpl for T { pub trait DesyncTree: Clone { type Node; - fn get_hash(&self) -> V; + fn get_hash(&self) -> Option; fn name(&self) -> &Option; @@ -44,9 +44,9 @@ pub trait DesyncTree: Clone { /// [`DesyncTree`] node trait, built from children and hash. A node is effectively a sub-tree, /// as we build the tree bottom-up. pub trait DesyncTreeNode: Clone + PartialEq + Eq { - fn new(hash: u64, name: Option, children: Vec) -> Self; + fn new(hash: Option, name: Option, children: Vec) -> Self; - fn get_hash(&self) -> V; + fn get_hash(&self) -> Option; } /// Implement to allow type to create a [`DesyncTreeNode`] containing hash built from children. @@ -54,7 +54,9 @@ pub trait BuildDesyncNode where N: DesyncTreeNode, { - fn desync_tree_node(&self) -> N; + /// `include_unhashable` sets whether components or resources be included as non-contributing nodes + /// in tree, to see what could be opted-in. + fn desync_tree_node(&self, include_unhashable: bool) -> N; } /// Default impl for [`DesyncTreeNode`]. @@ -62,7 +64,7 @@ where #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct DefaultDesyncTreeNode { name: Option, - hash: u64, + hash: Option, children: Vec, } @@ -79,7 +81,7 @@ impl Ord for DefaultDesyncTreeNode { } impl DesyncTreeNode for DefaultDesyncTreeNode { - fn new(hash: u64, name: Option, children: Vec) -> Self { + fn new(hash: Option, name: Option, children: Vec) -> Self { Self { name, hash, @@ -87,7 +89,7 @@ impl DesyncTreeNode for DefaultDesyncTreeNode { } } - fn get_hash(&self) -> u64 { + fn get_hash(&self) -> Option { self.hash } } @@ -108,7 +110,7 @@ impl From for DefaultDesyncTree { impl DesyncTree for DefaultDesyncTree { type Node = DefaultDesyncTreeNode; - fn get_hash(&self) -> u64 { + fn get_hash(&self) -> Option { self.root.get_hash() } From 5177eb72bf3ba5eb30bd86a616bca2bdb539d1b2 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Thu, 19 Sep 2024 18:32:21 -0700 Subject: [PATCH 18/29] feat: Add #[desync_exclude] attribute to exclude fields from desync hash. --- .../bones_utils/macros/src/lib.rs | 62 +++++++++++++++--- .../bones_utils/src/desync_hash.rs | 2 + framework_crates/bones_utils/tests/tests.rs | 64 +++++++++++++++++++ 3 files changed, 119 insertions(+), 9 deletions(-) diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index 6a02d905c9..9be483a83b 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -195,7 +195,7 @@ pub fn derive_deref_mut(input: TokenStream) -> TokenStream { } } -#[proc_macro_derive(DesyncHash, attributes(desync_hash_module))] +#[proc_macro_derive(DesyncHash, attributes(desync_hash_module, desync_exclude))] pub fn derive_desync_hash(input: TokenStream) -> TokenStream { let input = venial::parse_declaration(input.into()).unwrap(); let name = input.name().expect("Type must have a name"); @@ -217,7 +217,8 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { .unwrap_or_else(|| quote!(bones_utils)); // Helper to get hash invocations of struct fields - let hash_struct_fields = |fields: &StructFields| match fields { + let hash_struct_fields = |fields: &StructFields| { + match fields { venial::StructFields::Tuple(tuple) => tuple .fields .iter() @@ -230,11 +231,15 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { venial::StructFields::Named(named) => named .fields .iter() - .map(|(field, _)| { + .filter_map(|(field, _)| { let name = &field.name; let ty = &field.ty; - - quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#name, hasher);} + if !field.attributes.iter().any(|attr| { + attr.path[0].to_string() == "desync_exclude" + }) { + return Some(quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#name, hasher);}) + } + None }) .collect::>(), venial::StructFields::Unit => vec![], @@ -267,11 +272,37 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { (tuple_fields, invocations) } venial::StructFields::Named(named) => { - let field_idents: Vec<_> = named.fields.iter().map(|f| &f.0.name).collect(); + let mut any_fields_excluded = false; + + let field_idents: Vec<_> = named + .fields + .iter() + .filter_map(|f| { + if f.0 + .attributes + .iter() + .any(|attr| attr.path[0].to_string() == "desync_exclude") + { + any_fields_excluded = true; + return None; + } + Some(&f.0.name) + }) + .collect(); - // format list of fields as '{ fieldA, fieldB, ... }' - let named_fields = quote! { + // format list of fields as '{ fieldA, fieldB, }' + let named_fields = if !any_fields_excluded { + quote! { {#(#field_idents),*} + } + } else if !field_idents.is_empty() { + // If any fields excluded, include '..' to avoid compilation error + quote! { + {#(#field_idents),* , ..} + } + } else { + // All fields of variant were excluded + quote! { {..} } }; let invocations = field_idents @@ -293,7 +324,20 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { let enum_name = &e.name; for (idx, v) in e.variants.items().enumerate() { let variant_name = &v.name; - let (variant_fields_string, invocations) = enum_variant_fields(&v.contents); + let variant_excluded = v + .attributes + .iter() + .any(|attr| attr.path[0].to_string() == "desync_exclude"); + + // Excluded variant skips all invocations for hashing fields, + // however index of variant is still hashed, making it unique from + // other variants + let (variant_fields_string, invocations) = if !variant_excluded { + enum_variant_fields(&v.contents) + } else { + (quote! { {..} }, vec![]) + }; + variants.push(quote! { #enum_name::#variant_name #variant_fields_string => { // Hash index of variant to ensure that two variants are unique diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 5cdfee3c27..f3a8d6e14b 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -10,6 +10,8 @@ use ustr::Ustr; /// /// In order to opt in a `HasSchema` Component or Resource to be included in hash of World in networked session, /// `#[net]` or `#[derive_type_data(SchemaDesyncHas)]` must also be included. +/// +/// Fields may be excluded from hash by using attribute: `#[desync_exclude]` pub trait DesyncHash { /// Update hasher from type's values fn hash(&self, hasher: &mut dyn std::hash::Hasher); diff --git a/framework_crates/bones_utils/tests/tests.rs b/framework_crates/bones_utils/tests/tests.rs index ef62e564ca..9c40e5eeca 100644 --- a/framework_crates/bones_utils/tests/tests.rs +++ b/framework_crates/bones_utils/tests/tests.rs @@ -21,6 +21,14 @@ struct StructB { b: String, } +#[derive(HasSchema, DesyncHash, Debug, Clone, Default)] +#[desync_hash_module(crate)] +struct StructC { + a: f32, + #[desync_exclude] + b: String, +} + /// Test DesyncHash proc macro on Enum variants #[derive(HasSchema, DesyncHash, Debug, Clone, Default)] #[repr(C, u8)] @@ -39,6 +47,24 @@ enum EnumA { F = 52, } +#[derive(HasSchema, DesyncHash, Debug, Clone, Default)] +#[repr(C, u8)] +#[desync_hash_module(crate)] +#[allow(dead_code)] +enum EnumB { + A { + #[desync_exclude] + a: f64, + + b: u16, + }, + #[default] + #[desync_exclude] + B, + #[desync_exclude] + C { a: f32 }, +} + fn hash_value(value: &T) -> u64 { let mut hasher = FxHasher::default(); DesyncHash::hash(value, &mut hasher); @@ -78,6 +104,44 @@ fn desync_hash_struct() { assert_ne!(hash_value(&a), hash_value(&b)); } +#[test] +fn desync_hash_exclude_struct_field() { + let a = StructC { + a: 1.0, + b: "foo".to_string(), + }; + let b = StructC { + a: 1.0, + b: "bar".to_string(), + }; + + // field b is excluded on StructC, hash should be the same. + assert_eq!(hash_value(&a), hash_value(&b)); +} + +#[test] +fn desync_hash_exclude_enum_variant_named_field() { + let a = EnumB::A { a: 1.0, b: 1 }; + let b = EnumB::A { a: 0.0, b: 1 }; + + // field a is excluded on EnumB::A variant, hash should be the same. + assert_eq!(hash_value(&a), hash_value(&b)); +} + +#[test] +fn desync_hash_exclude_enum_variant() { + let a = EnumB::C { a: 1.0 }; + let b = EnumB::C { a: 0.0 }; + + // Variant EnumB::C Is excluded, should be equal. + assert_eq!(hash_value(&a), hash_value(&b)); + + // Although variant may be excluded and its fields not hashed, + // two variants (even if both excluded) should give unique hash. + let c = EnumB::B; + assert_ne!(hash_value(&c), hash_value(&a)) +} + #[test] fn desync_hash_glam() { let a = Vec3::new(1.0, 2.0, 3.0); From 83ec3fbf9cac4317c32eb1e410789c45771d79b0 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Thu, 19 Sep 2024 18:36:42 -0700 Subject: [PATCH 19/29] fix: fix missing brace --- .../bones_utils/macros/src/lib.rs | 49 ++++++++++--------- 1 file changed, 25 insertions(+), 24 deletions(-) diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index 9be483a83b..f3e8da090c 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -219,30 +219,31 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { // Helper to get hash invocations of struct fields let hash_struct_fields = |fields: &StructFields| { match fields { - venial::StructFields::Tuple(tuple) => tuple - .fields - .iter() - .enumerate() - .map(|(idx, (field, _))| { - let ty = &field.ty; - quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#idx, hasher);} - }) - .collect::>(), - venial::StructFields::Named(named) => named - .fields - .iter() - .filter_map(|(field, _)| { - let name = &field.name; - let ty = &field.ty; - if !field.attributes.iter().any(|attr| { - attr.path[0].to_string() == "desync_exclude" - }) { - return Some(quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#name, hasher);}) - } - None - }) - .collect::>(), - venial::StructFields::Unit => vec![], + venial::StructFields::Tuple(tuple) => tuple + .fields + .iter() + .enumerate() + .map(|(idx, (field, _))| { + let ty = &field.ty; + quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#idx, hasher);} + }) + .collect::>(), + venial::StructFields::Named(named) => named + .fields + .iter() + .filter_map(|(field, _)| { + let name = &field.name; + let ty = &field.ty; + if !field.attributes.iter().any(|attr| { + attr.path[0].to_string() == "desync_exclude" + }) { + return Some(quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#name, hasher);}) + } + None + }) + .collect::>(), + venial::StructFields::Unit => vec![], + } }; // Get fields of enum variant From 2a35abc4aea263d9bcde78674d98b8f879b71b40 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 11:56:58 -0800 Subject: [PATCH 20/29] refactor: Support entity Names, make tree iteratable for testing, cleanup generics / simplify. --- framework_crates/bones_ecs/src/components.rs | 34 ++++- .../bones_ecs/src/components/untyped.rs | 56 ++++--- framework_crates/bones_ecs/src/entities.rs | 16 ++ framework_crates/bones_ecs/src/resources.rs | 17 ++- framework_crates/bones_ecs/src/world.rs | 25 +++- .../bones_framework/src/networking/desync.rs | 1 + framework_crates/bones_utils/Cargo.toml | 11 +- .../bones_utils/src/desync_hash.rs | 141 ++++++++++++++---- 8 files changed, 235 insertions(+), 66 deletions(-) diff --git a/framework_crates/bones_ecs/src/components.rs b/framework_crates/bones_ecs/src/components.rs index caf6b7ef97..bd8dc9a000 100644 --- a/framework_crates/bones_ecs/src/components.rs +++ b/framework_crates/bones_ecs/src/components.rs @@ -2,7 +2,7 @@ use fxhash::FxHasher; use once_map::OnceMap; -use std::{any::Any, sync::Arc}; +use std::sync::Arc; use crate::prelude::*; @@ -81,12 +81,16 @@ impl DesyncHash for ComponentStores { } } -impl BuildDesyncNode for ComponentStores { +impl BuildDesyncNode for ComponentStores { fn desync_tree_node( &self, include_unhashable: bool, ) -> DefaultDesyncTreeNode { let mut any_hashable = false; + + // We get the Name component store so we can lookup entity names and set those on component leaves. + let names = self.get::().borrow(); + let mut child_nodes = self .components .read_only_view() @@ -104,7 +108,24 @@ impl BuildDesyncNode for ComponentStores { } if include_unhashable || is_hashable { - let child_node = component_store.desync_tree_node::(include_unhashable); + let mut child_node = component_store.desync_tree_node::(include_unhashable); + + // Our child here is a component store, and its children are component leaves. + // Iterate through children, retrieve metadata storing entity_idx if set, and use this + // to update the node's name from Name component. + // + // This is fairly hacky, but should be good enough for now. + for component_node in child_node.children_mut().iter_mut() { + if let DesyncNodeMetadata::Component { entity_idx } = + component_node.metadata() + { + // Constructing Entity with fake generation is bit of a hack - but component store does not + // use generation, only the index. + if let Some(name) = names.get(Entity::new(*entity_idx, 0)) { + component_node.set_name(name.0.clone()); + } + } + } return Some(child_node); } @@ -126,7 +147,12 @@ impl BuildDesyncNode for ComponentStores { None }; - DefaultDesyncTreeNode::new(hash, Some("Components".into()), child_nodes) + DefaultDesyncTreeNode::new( + hash, + Some("Components".into()), + child_nodes, + DesyncNodeMetadata::None, + ) } } diff --git a/framework_crates/bones_ecs/src/components/untyped.rs b/framework_crates/bones_ecs/src/components/untyped.rs index ab3d58c917..efa1a7c13d 100644 --- a/framework_crates/bones_ecs/src/components/untyped.rs +++ b/framework_crates/bones_ecs/src/components/untyped.rs @@ -82,29 +82,42 @@ impl DesyncHash for UntypedComponentStore { } } -impl BuildDesyncNode for UntypedComponentStore { +impl BuildDesyncNode for UntypedComponentStore { fn desync_tree_node( &self, _include_unhashable: bool, ) -> DefaultDesyncTreeNode { let mut hasher = H::default(); - let child_nodes: Vec = self - .iter() - .map(|component| -> DefaultDesyncTreeNode { - let hash = if component - .schema() - .type_data - .get::() - .is_some() - { - // Update parent node hash from data - DesyncHash::hash(&component, &mut hasher); - Some(component.compute_hash::()) - } else { - None - }; - - DefaultDesyncTreeNode::new(hash, None, vec![]) + + // Iterate over components by index so we can save entity ID. + let iter = 0..self.bitset().bit_len(); + let child_nodes: Vec = iter + .filter_map(|entity_idx| -> Option { + if let Some(component) = self.get_idx(entity_idx) { + let hash = if component + .schema() + .type_data + .get::() + .is_some() + { + // Update parent node hash from data + DesyncHash::hash(&component, &mut hasher); + Some(component.compute_hash::()) + } else { + None + }; + + return Some(DefaultDesyncTreeNode::new( + hash, + None, + vec![], + DesyncNodeMetadata::Component { + entity_idx: entity_idx as u32, + }, + )); + } + + None }) .collect(); @@ -114,7 +127,12 @@ impl BuildDesyncNode for UntypedComponentStore { None }; - DefaultDesyncTreeNode::new(hash, Some(self.schema().full_name.to_string()), child_nodes) + DefaultDesyncTreeNode::new( + hash, + Some(self.schema().full_name.to_string()), + child_nodes, + DesyncNodeMetadata::None, + ) } } diff --git a/framework_crates/bones_ecs/src/entities.rs b/framework_crates/bones_ecs/src/entities.rs index 50f8f5ecdd..7ec80c453e 100644 --- a/framework_crates/bones_ecs/src/entities.rs +++ b/framework_crates/bones_ecs/src/entities.rs @@ -85,6 +85,22 @@ impl Default for Entities { } } +/// Utility component storing a name for entity +#[derive(HasSchema, Clone, Debug)] +pub struct Name(pub String); + +impl Default for Name { + fn default() -> Self { + Self("Unnamed".to_string()) + } +} + +impl From<&str> for Name { + fn from(value: &str) -> Self { + Self(value.into()) + } +} + /// A type representing a component-joining entity query. pub trait QueryItem { /// The type of iterator this query item creates diff --git a/framework_crates/bones_ecs/src/resources.rs b/framework_crates/bones_ecs/src/resources.rs index 607710bb46..06a72e7a21 100644 --- a/framework_crates/bones_ecs/src/resources.rs +++ b/framework_crates/bones_ecs/src/resources.rs @@ -34,7 +34,7 @@ impl DesyncHash for UntypedResource { } } -impl BuildDesyncNode for UntypedResource { +impl BuildDesyncNode for UntypedResource { fn desync_tree_node( &self, _include_unhashable: bool, @@ -49,10 +49,10 @@ impl BuildDesyncNode for UntypedResource { } else { None }; - return DefaultDesyncTreeNode::new(hash, name, vec![]); + return DefaultDesyncTreeNode::new(hash, name, vec![], DesyncNodeMetadata::None); } - DefaultDesyncTreeNode::new(None, name, vec![]) + DefaultDesyncTreeNode::new(None, name, vec![], DesyncNodeMetadata::None) } } @@ -197,7 +197,7 @@ impl DesyncHash for UntypedResources { } } -impl BuildDesyncNode for UntypedResources { +impl BuildDesyncNode for UntypedResources { fn desync_tree_node( &self, include_unhashable: bool, @@ -230,7 +230,12 @@ impl BuildDesyncNode for UntypedResources { } } - DefaultDesyncTreeNode::new(Some(hasher.finish()), Some("Resources".into()), child_nodes) + DefaultDesyncTreeNode::new( + Some(hasher.finish()), + Some("Resources".into()), + child_nodes, + DesyncNodeMetadata::None, + ) } } @@ -314,7 +319,7 @@ impl DesyncHash for Resources { } } -impl BuildDesyncNode for Resources { +impl BuildDesyncNode for Resources { fn desync_tree_node( &self, include_unhashable: bool, diff --git a/framework_crates/bones_ecs/src/world.rs b/framework_crates/bones_ecs/src/world.rs index eba09cb7ee..efc7de3d9a 100644 --- a/framework_crates/bones_ecs/src/world.rs +++ b/framework_crates/bones_ecs/src/world.rs @@ -43,7 +43,7 @@ impl DesyncHash for World { } } -impl BuildDesyncNode for World { +impl BuildDesyncNode for World { fn desync_tree_node( &self, include_unhashable: bool, @@ -64,7 +64,12 @@ impl BuildDesyncNode for World { } child_nodes.push(resources_node); - DefaultDesyncTreeNode::new(Some(hasher.finish()), Some("World".into()), child_nodes) + DefaultDesyncTreeNode::new( + Some(hasher.finish()), + Some("World".into()), + child_nodes, + DesyncNodeMetadata::None, + ) } } @@ -246,6 +251,22 @@ impl World { // Always maintain to clean up any killed entities self.maintain(); } + /// Build [`DefaultDesyncTree`] from [`World`]. + /// + /// `include_unhashable` sets whether components or resources be included as non-contributing nodes + /// in tree, to see what could be opted-in to desync hashing. + /// + /// # Panics + /// + /// This will immutably borrow all components and resources, if any are mutably borrowed, this will panic. + pub fn desync_hash_tree( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTree { + let root = self.desync_tree_node::(include_unhashable); + + DefaultDesyncTree::from_root(root) + } } /// Creates an instance of the type this trait is implemented for diff --git a/framework_crates/bones_framework/src/networking/desync.rs b/framework_crates/bones_framework/src/networking/desync.rs index b5f8156a6b..131164eaf6 100644 --- a/framework_crates/bones_framework/src/networking/desync.rs +++ b/framework_crates/bones_framework/src/networking/desync.rs @@ -7,6 +7,7 @@ use bones_lib::{ecs::World, prelude::default}; /// but is private so cannot be used directly. const MAX_DESYNC_HISTORY_BUFFER: usize = 32; +/// Settings for desync detection #[derive(Clone)] pub struct DetectDesyncs { /// Interval in frames of how often to hash state and check for desync with other clients. diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index b240148649..8a76a35c73 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -12,22 +12,23 @@ keywords.workspace = true [features] default = ["ulid"] -glam = ["dep:glam"] -serde = ["dep:serde", "hashbrown/serde"] -ulid = ["dep:ulid", "instant", "turborand"] +glam = ["dep:glam"] +serde = ["dep:serde", "hashbrown/serde"] +ulid = ["dep:ulid", "instant", "turborand"] [dependencies] bones_utils_macros = { version = "0.4", path = "./macros" } fxhash = { workspace = true } hashbrown = { workspace = true } +tree_iterators_rs = { version = "1.2.1" } # Optional instant = { version = "0.1", features = ["wasm-bindgen"], optional = true } serde = { version = "1.0", optional = true } turborand = { version = "0.10", optional = true } ulid = { version = "1.0", optional = true } -glam = { version = "0.24", optional = true } -paste = { version = "1.0" } +glam = { version = "0.24", optional = true } +paste = { version = "1.0" } # Make sure that the getrandom package, used in `ulid` works on web # when compiling for WASM. diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index f3a8d6e14b..afb37739a3 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -2,14 +2,15 @@ //! //! In order to use [`DesyncHash`] with [`glam`] types, the "glam" feature flag must be used. -use std::time::Duration; - +use std::{slice::Iter, time::Duration}; use ustr::Ustr; +pub use tree_iterators_rs::prelude::BorrowedTreeNode; + /// [`DesyncHash`] is used to hash type and compare over network to detect desyncs. /// /// In order to opt in a `HasSchema` Component or Resource to be included in hash of World in networked session, -/// `#[net]` or `#[derive_type_data(SchemaDesyncHas)]` must also be included. +/// `#[net]` or `#[derive_type_data(SchemaDesyncHash)]` must also be included. /// /// Fields may be excluded from hash by using attribute: `#[desync_exclude]` pub trait DesyncHash { @@ -33,46 +34,127 @@ impl DesyncHashImpl for T { } /// Tree of desync hashes -pub trait DesyncTree: Clone { +pub trait DesyncTree: Clone { + /// Node type type Node; - fn get_hash(&self) -> Option; + /// Get root hash of tree + fn get_hash(&self) -> Option; - fn name(&self) -> &Option; + /// Get root node + fn root(&self) -> &Self::Node; + /// make tree from root node fn from_root(root: Self::Node) -> Self; } /// [`DesyncTree`] node trait, built from children and hash. A node is effectively a sub-tree, /// as we build the tree bottom-up. -pub trait DesyncTreeNode: Clone + PartialEq + Eq { - fn new(hash: Option, name: Option, children: Vec) -> Self; +pub trait DesyncTreeNode: Clone { + /// Get node hash + fn get_hash(&self) -> Option; + + /// Get children + fn children(&self) -> &Vec; - fn get_hash(&self) -> Option; + /// Get children mut + fn children_mut(&mut self) -> &mut Vec; } /// Implement to allow type to create a [`DesyncTreeNode`] containing hash built from children. -pub trait BuildDesyncNode -where - N: DesyncTreeNode, -{ +pub trait BuildDesyncNode { /// `include_unhashable` sets whether components or resources be included as non-contributing nodes /// in tree, to see what could be opted-in. - fn desync_tree_node(&self, include_unhashable: bool) -> N; + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode; +} + +/// Metadata optionally included with ['DesyncTreeNode`]. +#[derive(Copy, Clone, Default)] +pub enum DesyncNodeMetadata { + /// No additional metadata + #[default] + None, + /// Node is a component + Component { + /// Entity idx of component + entity_idx: u32, + }, } /// Default impl for [`DesyncTreeNode`]. -#[derive(Clone, PartialEq, Eq)] +#[derive(Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct DefaultDesyncTreeNode { name: Option, hash: Option, - children: Vec, + children: Vec, + + /// Some userdata that can be included in node. + #[cfg_attr(feature = "serde", serde(skip))] + metadata: DesyncNodeMetadata, } +impl DefaultDesyncTreeNode { + /// Create new node + pub fn new( + hash: Option, + name: Option, + children: Vec, + metadata: DesyncNodeMetadata, + ) -> Self { + Self { + name, + hash, + children, + metadata, + } + } + + /// Get node metadata + pub fn metadata(&self) -> &DesyncNodeMetadata { + &self.metadata + } + + /// Name of node + pub fn name(&self) -> &Option { + &self.name + } + + /// Set the name of node + pub fn set_name(&mut self, name: String) { + self.name = Some(name); + } + + /// Get node hash + pub fn get_hash(&self) -> Option { + self.hash + } + + /// Get children + pub fn children(&self) -> &Vec { + &self.children + } + + /// Get children mut + pub fn children_mut(&mut self) -> &mut Vec { + &mut self.children + } +} + +impl PartialEq for DefaultDesyncTreeNode { + fn eq(&self, other: &Self) -> bool { + self.hash == other.hash + } +} + +impl Eq for DefaultDesyncTreeNode {} + impl PartialOrd for DefaultDesyncTreeNode { fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) + Some(self.hash.cmp(&other.hash)) } } @@ -82,17 +164,16 @@ impl Ord for DefaultDesyncTreeNode { } } -impl DesyncTreeNode for DefaultDesyncTreeNode { - fn new(hash: Option, name: Option, children: Vec) -> Self { - Self { - name, - hash, - children, - } - } +/// Auto impl support for iterating over tree +impl<'a> BorrowedTreeNode<'a> for DefaultDesyncTreeNode { + type BorrowedValue = &'a Self; - fn get_hash(&self) -> Option { - self.hash + type BorrowedChildren = Iter<'a, DefaultDesyncTreeNode>; + + fn get_value_and_children_iter( + &'a self, + ) -> (Self::BorrowedValue, Option) { + (self, Some(self.children.iter())) } } @@ -109,15 +190,15 @@ impl From for DefaultDesyncTree { } } -impl DesyncTree for DefaultDesyncTree { +impl DesyncTree for DefaultDesyncTree { type Node = DefaultDesyncTreeNode; fn get_hash(&self) -> Option { self.root.get_hash() } - fn name(&self) -> &Option { - &self.root.name + fn root(&self) -> &Self::Node { + &self.root } fn from_root(root: Self::Node) -> Self { From f88be6cc0f5246134a4971f42135f5f7d234e244 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 12:09:11 -0800 Subject: [PATCH 21/29] chore: Fixup errors after rebase --- Cargo.lock | 19 +++++++++++++++++++ framework_crates/bones_ecs/Cargo.toml | 1 + framework_crates/bones_framework/Cargo.toml | 1 + .../bones_framework/src/networking.rs | 14 ++++++++------ framework_crates/bones_utils/Cargo.toml | 1 + 5 files changed, 30 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 33ccaa0f60..c94e7df568 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1430,6 +1430,7 @@ dependencies = [ "bones_schema", "bones_utils", "branches", + "fxhash", "glam 0.24.2", "once_map", "paste", @@ -1481,6 +1482,7 @@ dependencies = [ "fluent", "fluent-langneg", "futures-lite 2.3.0", + "fxhash", "ggrs", "gilrs", "glam 0.24.2", @@ -1619,8 +1621,10 @@ dependencies = [ "instant", "paste", "serde", + "tree_iterators_rs", "turborand", "ulid", + "ustr", ] [[package]] @@ -6983,6 +6987,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b2231b7c3057d5e4ad0156fb3dc807d900806020c5ffa3ee6ff2c8c76fb8520" + [[package]] name = "strsim" version = "0.11.1" @@ -7734,6 +7744,15 @@ dependencies = [ "cc", ] +[[package]] +name = "tree_iterators_rs" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "35bd22d378c78c58e20672ecd4ec4507a43325742daa79c5db037de084eb5f0c" +dependencies = [ + "streaming-iterator", +] + [[package]] name = "triple_buffer" version = "8.0.0" diff --git a/framework_crates/bones_ecs/Cargo.toml b/framework_crates/bones_ecs/Cargo.toml index 605828b892..c101b817ea 100644 --- a/framework_crates/bones_ecs/Cargo.toml +++ b/framework_crates/bones_ecs/Cargo.toml @@ -36,6 +36,7 @@ serde = { version = "1", features = ["derive"], optional = true } anyhow = "1.0" branches = { workspace = true } +fxhash = { workspace = true } atomicell = "0.2" bitset-core = "0.1" once_map = "0.4.12" diff --git a/framework_crates/bones_framework/Cargo.toml b/framework_crates/bones_framework/Cargo.toml index a558b94275..bcabd489c7 100644 --- a/framework_crates/bones_framework/Cargo.toml +++ b/framework_crates/bones_framework/Cargo.toml @@ -98,6 +98,7 @@ bevy_tasks = "0.11" bytemuck = "1.12" either = "1.8" futures-lite = { workspace = true } +fxhash = { workspace = true } glam = "0.24" hex = "0.4" instant = { version = "0.1", features = ["wasm-bindgen"] } diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index b4fc55c6af..a1319498f9 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -65,7 +65,8 @@ impl From for NetworkInputStatus { /// Module prelude. pub mod prelude { pub use super::{ - desync::DetectDesyncs, input, lan, online, proto, random, DisconnectedPlayers, RngGenerator, SyncingInfo, RUNTIME, + desync::DetectDesyncs, input, lan, online, proto, random, DisconnectedPlayers, + RngGenerator, SyncingInfo, RUNTIME, }; #[cfg(feature = "net-debug")] @@ -553,9 +554,9 @@ pub struct GgrsSessionRunner<'a, InputTypes: NetworkInputConfig<'a>> { /// The random seed used for this session pub random_seed: u64, - + /// When provided, desync detection is enabled. Contains settings for desync detection. - detect_desyncs: Option, + detect_desyncs: Option, /// History buffer for desync debug data to fetch it upon detected desyncs. /// [`DefaultDesyncTree`] will be generated and saved here if feature `desync-debug` is enabled. @@ -582,13 +583,12 @@ pub struct GgrsSessionRunnerInfo { /// /// `None` will use Bone's default. pub local_input_delay: Option, - + /// The random seed used for this session pub random_seed: u64, - + /// When provided, desync detection is enabled. Contains settings for desync detection. pub detect_desyncs: Option, - } impl GgrsSessionRunnerInfo { @@ -625,6 +625,7 @@ where target_fps: Option, max_prediction_window: Option, local_input_delay: Option, + detect_desyncs: Option, matchmaker_resp_game_starting: OnlineMatchmakerResponse, ) -> Option { if let OnlineMatchmakerResponse::GameStarting { @@ -641,6 +642,7 @@ where max_prediction_window, local_input_delay, random_seed, + detect_desyncs, ), )) } else { diff --git a/framework_crates/bones_utils/Cargo.toml b/framework_crates/bones_utils/Cargo.toml index 8a76a35c73..8a2965de83 100644 --- a/framework_crates/bones_utils/Cargo.toml +++ b/framework_crates/bones_utils/Cargo.toml @@ -21,6 +21,7 @@ bones_utils_macros = { version = "0.4", path = "./macros" } fxhash = { workspace = true } hashbrown = { workspace = true } tree_iterators_rs = { version = "1.2.1" } +ustr = { workspace = true } # Optional instant = { version = "0.1", features = ["wasm-bindgen"], optional = true } From 87bf925bf8b629eaa865b938c064eaec7accf37b Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 12:29:38 -0800 Subject: [PATCH 22/29] test: Add test for desync tree entity names --- .../bones_ecs/tests/desync_tree.rs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 framework_crates/bones_ecs/tests/desync_tree.rs diff --git a/framework_crates/bones_ecs/tests/desync_tree.rs b/framework_crates/bones_ecs/tests/desync_tree.rs new file mode 100644 index 0000000000..73e986e30a --- /dev/null +++ b/framework_crates/bones_ecs/tests/desync_tree.rs @@ -0,0 +1,46 @@ +//! Tests for [`DesyncTree`] build that rely on types in [`bones_ecs`]. + +use bones_ecs::prelude::*; + +#[derive(Clone, HasSchema, Debug, Eq, PartialEq, Default, DesyncHash)] +#[net] +#[repr(C)] +struct Pos(i32, i32); + +#[test] +fn desync_tree_entity_names() { + let world = World::default(); + + // Scope this so mut borrows are finished by time tree is being created. + let (ent1, ent2) = { + let mut entities = world.resource_mut::(); + let mut positions = world.component_mut::(); + let mut names = world.component_mut::(); + + let ent1 = entities.create(); + positions.insert(ent1, Pos(0, 0)); + + let ent2 = entities.create(); + positions.insert(ent2, Pos(1, 1)); + names.insert(ent2, "entity2".into()); + (ent1, ent2) + }; + + let mut found_ent1_metadata = false; + let mut found_ent2_metadata = false; + + let desync_tree = world.desync_hash_tree::(false); + for node in desync_tree.root().dfs_preorder_iter() { + if let DesyncNodeMetadata::Component { entity_idx } = node.metadata() { + if *entity_idx == ent1.index() { + found_ent1_metadata = true; + } else if *entity_idx == ent2.index() { + found_ent2_metadata = true; + assert_eq!(*node.name(), Some("entity2".to_string())); + } + } + } + + assert!(found_ent1_metadata); + assert!(found_ent2_metadata); +} From 7eaf0dd0a27da3aa92939008957cffe3d70ef589 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 12:45:11 -0800 Subject: [PATCH 23/29] fix: Remove `derive(Serialize)` from `Transform` --- framework_crates/bones_framework/src/render/transform.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework_crates/bones_framework/src/render/transform.rs b/framework_crates/bones_framework/src/render/transform.rs index a8e6c7d094..0fc8a04585 100644 --- a/framework_crates/bones_framework/src/render/transform.rs +++ b/framework_crates/bones_framework/src/render/transform.rs @@ -5,7 +5,7 @@ use crate::prelude::*; /// The main transform component. /// /// Currently we don't have a hierarchy, and this is therefore a global transform. -#[derive(Clone, Copy, Debug, HasSchema, DesyncHash, Serialize)] +#[derive(Clone, Copy, Debug, HasSchema, DesyncHash)] #[net] #[repr(C)] pub struct Transform { From 0eb14118924aa9561a26c7e6c5bf4fff45fd9c01 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 12:59:09 -0800 Subject: [PATCH 24/29] chore(ci): Clippy + doc warnings --- .../bones_framework/src/networking/desync.rs | 11 ++++++----- framework_crates/bones_utils/macros/src/lib.rs | 2 +- framework_crates/bones_utils/tests/tests.rs | 1 + other_crates/bones_matchmaker/src/matchmaking.rs | 2 +- 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/framework_crates/bones_framework/src/networking/desync.rs b/framework_crates/bones_framework/src/networking/desync.rs index 131164eaf6..d760371bc8 100644 --- a/framework_crates/bones_framework/src/networking/desync.rs +++ b/framework_crates/bones_framework/src/networking/desync.rs @@ -1,11 +1,12 @@ -//! +//! Desync detection and history buffer for desync trees. + use std::collections::VecDeque; -use bones_lib::{ecs::World, prelude::default}; +use bones_lib::prelude::*; /// Max frames of data in desync history buffer - this is set to match `ggrs::MAX_CHECKSUM_HISTORY_SIZE`, /// but is private so cannot be used directly. -const MAX_DESYNC_HISTORY_BUFFER: usize = 32; +pub const MAX_DESYNC_HISTORY_BUFFER: usize = 32; /// Settings for desync detection #[derive(Clone)] @@ -18,7 +19,7 @@ pub struct DetectDesyncs { /// By default, [`World`]'s [`DesyncHash`] impl is used. pub world_hash_func: Option u64>, - /// When using feature `desync-debug`, a [`DesyncTree`] will be built. Resources and Components + /// When using feature `desync-debug`, a [`bones_utils::DesyncTree`] will be built. Resources and Components /// that do not support hashing can be optionally included in tree to help highlight candidates /// to be opted into desync-detection. pub include_unhashable_nodes: bool, @@ -33,7 +34,7 @@ impl Default for DetectDesyncs { } } } -/// Store history of desync detection data, such as a [`DesyncTree`]. When ggrs finds a desync in past, +/// Store history of desync detection data, such as a [`bones_utils::DesyncTree`]. When ggrs finds a desync in past, /// we can retrieve this data for debugging. Ggrs has a fixed limit of pending desync frames it tests, /// so we match it by keeping the last [`MAX_DESYNC_HISTORY_BUFFER`] of frame data at the desync detect interval. /// diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index f3e8da090c..cbb0ee7001 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -23,7 +23,7 @@ fn is_simple_named_attr(attr: &venial::Attribute, name: &str) -> bool { /// Attribute adding extra functionality for networking. /// /// For example, provides sugar for `#[derive_type_data(SchemaDesyncHash)]`, which -/// opts in [`SchemaRef`] to support [`DesyncHash`], so it maybe be included in hash for desync detection. +/// opts in `SchemaRef` to support [`DesyncHash`], so it maybe be included in hash for desync detection. #[proc_macro_attribute] pub fn net(_attr: TokenStream, item: TokenStream) -> TokenStream { // Error if #[net] is used with #[schema(no_clone)]. diff --git a/framework_crates/bones_utils/tests/tests.rs b/framework_crates/bones_utils/tests/tests.rs index 9c40e5eeca..e35847516f 100644 --- a/framework_crates/bones_utils/tests/tests.rs +++ b/framework_crates/bones_utils/tests/tests.rs @@ -23,6 +23,7 @@ struct StructB { #[derive(HasSchema, DesyncHash, Debug, Clone, Default)] #[desync_hash_module(crate)] +#[allow(dead_code)] struct StructC { a: f32, #[desync_exclude] diff --git a/other_crates/bones_matchmaker/src/matchmaking.rs b/other_crates/bones_matchmaker/src/matchmaking.rs index 68128f6d6b..07c9a44378 100644 --- a/other_crates/bones_matchmaker/src/matchmaking.rs +++ b/other_crates/bones_matchmaker/src/matchmaking.rs @@ -216,7 +216,7 @@ async fn send_matchmaking_updates( })?; // Send first update and check active connections - for (_index, conn) in connections.into_iter().enumerate() { + for conn in connections.into_iter() { if let Ok(mut send) = conn.open_uni().await { if send.write_all(&first_update_message).await.is_ok() && send.finish().is_ok() From ee4ba1022bc5f3408633da3508c2c83f3270a070 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 13:24:42 -0800 Subject: [PATCH 25/29] chore: Fix derive_desync_hash tuple indexing warning --- Cargo.lock | 1 + framework_crates/bones_utils/macros/Cargo.toml | 5 +++-- framework_crates/bones_utils/macros/src/lib.rs | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c94e7df568..1299ee2009 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1633,6 +1633,7 @@ version = "0.4.0" dependencies = [ "proc-macro2", "quote", + "syn 2.0.77", "venial", ] diff --git a/framework_crates/bones_utils/macros/Cargo.toml b/framework_crates/bones_utils/macros/Cargo.toml index 42b2ac8423..8e45a8a463 100644 --- a/framework_crates/bones_utils/macros/Cargo.toml +++ b/framework_crates/bones_utils/macros/Cargo.toml @@ -14,6 +14,7 @@ keywords.workspace = true proc-macro = true [dependencies] -quote = "1.0" -venial = "0.5" +quote = "1.0" +venial = "0.5" proc-macro2 = "1" +syn = { version = "2" } diff --git a/framework_crates/bones_utils/macros/src/lib.rs b/framework_crates/bones_utils/macros/src/lib.rs index cbb0ee7001..8f044b289a 100644 --- a/framework_crates/bones_utils/macros/src/lib.rs +++ b/framework_crates/bones_utils/macros/src/lib.rs @@ -225,6 +225,7 @@ pub fn derive_desync_hash(input: TokenStream) -> TokenStream { .enumerate() .map(|(idx, (field, _))| { let ty = &field.ty; + let idx = syn::Index::from(idx); quote! {<#ty as #desync_hash_module::DesyncHash>::hash(&self.#idx, hasher);} }) .collect::>(), From dda49645030e8ccbf5c99207661c98aa84bec572 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Sun, 3 Nov 2024 13:26:48 -0800 Subject: [PATCH 26/29] chore: desync tree iteration: Pass None if children is empty instead of iter() --- framework_crates/bones_utils/src/desync_hash.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index afb37739a3..8f73a72ca5 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -173,6 +173,10 @@ impl<'a> BorrowedTreeNode<'a> for DefaultDesyncTreeNode { fn get_value_and_children_iter( &'a self, ) -> (Self::BorrowedValue, Option) { + if self.children.is_empty() { + return (self, None); + } + (self, Some(self.children.iter())) } } From 603e911020bfa66dd6487358eec9864d0361d94f Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Mon, 4 Nov 2024 17:20:30 -0800 Subject: [PATCH 27/29] chore(tests): Fix comments in tests --- framework_crates/bones_utils/tests/tests.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/framework_crates/bones_utils/tests/tests.rs b/framework_crates/bones_utils/tests/tests.rs index e35847516f..67fe46017c 100644 --- a/framework_crates/bones_utils/tests/tests.rs +++ b/framework_crates/bones_utils/tests/tests.rs @@ -153,8 +153,7 @@ fn desync_hash_glam() { #[test] fn desync_hash_schemaref() { - // Test that these hash to different values, StructA - // has SchemaDesyncHash typedata. + // Test that these hash to different values. let a = StructA { a: 1.0, b: "foo".to_string(), @@ -167,9 +166,9 @@ fn desync_hash_schemaref() { let b_hash = hash_value(&b.as_schema_ref()); assert_ne!(a_hash, b_hash); - // StructB does not have SchemaDesyncHash typedata, + // StructB does not support hashing, // its SchemaRef does not have impl for DesyncHash, - // even if data is different, should just get 0. + // even if data is different, it will hash to 0. let a = StructB { a: 1.0, b: "foo".to_string(), @@ -180,8 +179,8 @@ fn desync_hash_schemaref() { }; let a_hash = hash_value(&a.as_schema_ref()); let b_hash = hash_value(&b.as_schema_ref()); - // Test that these hash to differnet values, StructA - // has SchemaDesyncHash typedata. + + // They should both hash to 0, StructB doesn't support hashing. assert_eq!(a_hash, b_hash); assert_eq!(a_hash, 0); } From b864466b707a91404afd793cbc4c2bde5e158d62 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Mon, 4 Nov 2024 17:25:34 -0800 Subject: [PATCH 28/29] chore: move DesyncTree types to own file. --- .../bones_utils/src/desync_hash.rs | 181 +---------------- .../bones_utils/src/desync_hash/tree.rs | 182 ++++++++++++++++++ 2 files changed, 185 insertions(+), 178 deletions(-) create mode 100644 framework_crates/bones_utils/src/desync_hash/tree.rs diff --git a/framework_crates/bones_utils/src/desync_hash.rs b/framework_crates/bones_utils/src/desync_hash.rs index 8f73a72ca5..9aeb36431f 100644 --- a/framework_crates/bones_utils/src/desync_hash.rs +++ b/framework_crates/bones_utils/src/desync_hash.rs @@ -2,10 +2,11 @@ //! //! In order to use [`DesyncHash`] with [`glam`] types, the "glam" feature flag must be used. -use std::{slice::Iter, time::Duration}; +use std::time::Duration; use ustr::Ustr; -pub use tree_iterators_rs::prelude::BorrowedTreeNode; +pub mod tree; +pub use tree::*; /// [`DesyncHash`] is used to hash type and compare over network to detect desyncs. /// @@ -33,182 +34,6 @@ impl DesyncHashImpl for T { } } -/// Tree of desync hashes -pub trait DesyncTree: Clone { - /// Node type - type Node; - - /// Get root hash of tree - fn get_hash(&self) -> Option; - - /// Get root node - fn root(&self) -> &Self::Node; - - /// make tree from root node - fn from_root(root: Self::Node) -> Self; -} - -/// [`DesyncTree`] node trait, built from children and hash. A node is effectively a sub-tree, -/// as we build the tree bottom-up. -pub trait DesyncTreeNode: Clone { - /// Get node hash - fn get_hash(&self) -> Option; - - /// Get children - fn children(&self) -> &Vec; - - /// Get children mut - fn children_mut(&mut self) -> &mut Vec; -} - -/// Implement to allow type to create a [`DesyncTreeNode`] containing hash built from children. -pub trait BuildDesyncNode { - /// `include_unhashable` sets whether components or resources be included as non-contributing nodes - /// in tree, to see what could be opted-in. - fn desync_tree_node( - &self, - include_unhashable: bool, - ) -> DefaultDesyncTreeNode; -} - -/// Metadata optionally included with ['DesyncTreeNode`]. -#[derive(Copy, Clone, Default)] -pub enum DesyncNodeMetadata { - /// No additional metadata - #[default] - None, - /// Node is a component - Component { - /// Entity idx of component - entity_idx: u32, - }, -} - -/// Default impl for [`DesyncTreeNode`]. -#[derive(Clone)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct DefaultDesyncTreeNode { - name: Option, - hash: Option, - children: Vec, - - /// Some userdata that can be included in node. - #[cfg_attr(feature = "serde", serde(skip))] - metadata: DesyncNodeMetadata, -} - -impl DefaultDesyncTreeNode { - /// Create new node - pub fn new( - hash: Option, - name: Option, - children: Vec, - metadata: DesyncNodeMetadata, - ) -> Self { - Self { - name, - hash, - children, - metadata, - } - } - - /// Get node metadata - pub fn metadata(&self) -> &DesyncNodeMetadata { - &self.metadata - } - - /// Name of node - pub fn name(&self) -> &Option { - &self.name - } - - /// Set the name of node - pub fn set_name(&mut self, name: String) { - self.name = Some(name); - } - - /// Get node hash - pub fn get_hash(&self) -> Option { - self.hash - } - - /// Get children - pub fn children(&self) -> &Vec { - &self.children - } - - /// Get children mut - pub fn children_mut(&mut self) -> &mut Vec { - &mut self.children - } -} - -impl PartialEq for DefaultDesyncTreeNode { - fn eq(&self, other: &Self) -> bool { - self.hash == other.hash - } -} - -impl Eq for DefaultDesyncTreeNode {} - -impl PartialOrd for DefaultDesyncTreeNode { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.hash.cmp(&other.hash)) - } -} - -impl Ord for DefaultDesyncTreeNode { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.hash.cmp(&other.hash) - } -} - -/// Auto impl support for iterating over tree -impl<'a> BorrowedTreeNode<'a> for DefaultDesyncTreeNode { - type BorrowedValue = &'a Self; - - type BorrowedChildren = Iter<'a, DefaultDesyncTreeNode>; - - fn get_value_and_children_iter( - &'a self, - ) -> (Self::BorrowedValue, Option) { - if self.children.is_empty() { - return (self, None); - } - - (self, Some(self.children.iter())) - } -} - -/// Tree of desync hashes, allows storing hash of world and children such as components and resources. -#[derive(Clone)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct DefaultDesyncTree { - root: DefaultDesyncTreeNode, -} - -impl From for DefaultDesyncTree { - fn from(value: DefaultDesyncTreeNode) -> Self { - Self::from_root(value) - } -} - -impl DesyncTree for DefaultDesyncTree { - type Node = DefaultDesyncTreeNode; - - fn get_hash(&self) -> Option { - self.root.get_hash() - } - - fn root(&self) -> &Self::Node { - &self.root - } - - fn from_root(root: Self::Node) -> Self { - Self { root } - } -} impl DesyncHash for Duration { fn hash(&self, hasher: &mut dyn std::hash::Hasher) { self.as_nanos().hash(hasher); diff --git a/framework_crates/bones_utils/src/desync_hash/tree.rs b/framework_crates/bones_utils/src/desync_hash/tree.rs new file mode 100644 index 0000000000..d485dad114 --- /dev/null +++ b/framework_crates/bones_utils/src/desync_hash/tree.rs @@ -0,0 +1,182 @@ +//! Implementation of [`DesyncTree`] trait for hash tree in desync detection. + +use std::slice::Iter; + +pub use tree_iterators_rs::prelude::BorrowedTreeNode; + +/// Tree of desync hashes +pub trait DesyncTree: Clone { + /// Node type + type Node; + + /// Get root hash of tree + fn get_hash(&self) -> Option; + + /// Get root node + fn root(&self) -> &Self::Node; + + /// make tree from root node + fn from_root(root: Self::Node) -> Self; +} + +/// [`DesyncTree`] node trait, built from children and hash. A node is effectively a sub-tree, +/// as we build the tree bottom-up. +pub trait DesyncTreeNode: Clone { + /// Get node hash + fn get_hash(&self) -> Option; + + /// Get children + fn children(&self) -> &Vec; + + /// Get children mut + fn children_mut(&mut self) -> &mut Vec; +} + +/// Implement to allow type to create a [`DesyncTreeNode`] containing hash built from children. +pub trait BuildDesyncNode { + /// `include_unhashable` sets whether components or resources be included as non-contributing nodes + /// in tree, to see what could be opted-in. + fn desync_tree_node( + &self, + include_unhashable: bool, + ) -> DefaultDesyncTreeNode; +} + +/// Metadata optionally included with ['DesyncTreeNode`]. +#[derive(Copy, Clone, Default)] +pub enum DesyncNodeMetadata { + /// No additional metadata + #[default] + None, + /// Node is a component + Component { + /// Entity idx of component + entity_idx: u32, + }, +} + +/// Default impl for [`DesyncTreeNode`]. +#[derive(Clone)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct DefaultDesyncTreeNode { + name: Option, + hash: Option, + children: Vec, + + /// Some userdata that can be included in node. + #[cfg_attr(feature = "serde", serde(skip))] + metadata: DesyncNodeMetadata, +} + +impl DefaultDesyncTreeNode { + /// Create new node + pub fn new( + hash: Option, + name: Option, + children: Vec, + metadata: DesyncNodeMetadata, + ) -> Self { + Self { + name, + hash, + children, + metadata, + } + } + + /// Get node metadata + pub fn metadata(&self) -> &DesyncNodeMetadata { + &self.metadata + } + + /// Name of node + pub fn name(&self) -> &Option { + &self.name + } + + /// Set the name of node + pub fn set_name(&mut self, name: String) { + self.name = Some(name); + } + + /// Get node hash + pub fn get_hash(&self) -> Option { + self.hash + } + + /// Get children + pub fn children(&self) -> &Vec { + &self.children + } + + /// Get children mut + pub fn children_mut(&mut self) -> &mut Vec { + &mut self.children + } +} + +impl PartialEq for DefaultDesyncTreeNode { + fn eq(&self, other: &Self) -> bool { + self.hash == other.hash + } +} + +impl Eq for DefaultDesyncTreeNode {} + +impl PartialOrd for DefaultDesyncTreeNode { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.hash.cmp(&other.hash)) + } +} + +impl Ord for DefaultDesyncTreeNode { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.hash.cmp(&other.hash) + } +} + +/// Auto impl support for iterating over tree +impl<'a> BorrowedTreeNode<'a> for DefaultDesyncTreeNode { + type BorrowedValue = &'a Self; + + type BorrowedChildren = Iter<'a, DefaultDesyncTreeNode>; + + fn get_value_and_children_iter( + &'a self, + ) -> (Self::BorrowedValue, Option) { + if self.children.is_empty() { + return (self, None); + } + + (self, Some(self.children.iter())) + } +} + +/// Tree of desync hashes, allows storing hash of world and children such as components and resources. +#[derive(Clone)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct DefaultDesyncTree { + root: DefaultDesyncTreeNode, +} + +impl From for DefaultDesyncTree { + fn from(value: DefaultDesyncTreeNode) -> Self { + Self::from_root(value) + } +} + +impl DesyncTree for DefaultDesyncTree { + type Node = DefaultDesyncTreeNode; + + fn get_hash(&self) -> Option { + self.root.get_hash() + } + + fn root(&self) -> &Self::Node { + &self.root + } + + fn from_root(root: Self::Node) -> Self { + Self { root } + } +} From 8ab4485ab6671909396b5014680770763ff363b3 Mon Sep 17 00:00:00 2001 From: Max Whitehead Date: Wed, 6 Nov 2024 17:59:53 -0800 Subject: [PATCH 29/29] chore: Serialize tree as pretty json --- Cargo.lock | 1 + Cargo.toml | 1 + framework_crates/bones_asset/Cargo.toml | 2 +- framework_crates/bones_framework/Cargo.toml | 3 ++- framework_crates/bones_framework/src/networking.rs | 2 +- 5 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1299ee2009..1bd2c62c4e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1503,6 +1503,7 @@ dependencies = [ "rustls 0.21.12", "send_wrapper", "serde", + "serde_json", "serde_yaml", "smallvec", "sys-locale", diff --git a/Cargo.toml b/Cargo.toml index 9f6a805e82..e99a3843a8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ fxhash = "0.2" hashbrown = "0.14" maybe-owned = "0.3" parking_lot = "0.12" +serde_json = "1" smallvec = "1.11" ustr = "0.10" iroh-net = "0.27" diff --git a/framework_crates/bones_asset/Cargo.toml b/framework_crates/bones_asset/Cargo.toml index ebd115c260..a32f131564 100644 --- a/framework_crates/bones_asset/Cargo.toml +++ b/framework_crates/bones_asset/Cargo.toml @@ -37,7 +37,7 @@ paste = "1.0" path-absolutize = { version = "3.1", features = ["use_unix_paths_on_wasm"] } semver = { version = "1.0", features = ["serde"] } serde = { version = "1.0", features = ["derive"] } -serde_json = "1.0" +serde_json = { workspace = true } serde_yaml = "0.9" sha2 = "0.10" tracing = "0.1" diff --git a/framework_crates/bones_framework/Cargo.toml b/framework_crates/bones_framework/Cargo.toml index bcabd489c7..def89511af 100644 --- a/framework_crates/bones_framework/Cargo.toml +++ b/framework_crates/bones_framework/Cargo.toml @@ -31,7 +31,7 @@ scripting = ["dep:bones_scripting"] net-debug = ["ui"] ## Enables advanced desync debugging, generates hash tree of world for frames -desync-debug = [] +desync-debug = ["dep:serde_json"] #! ### Audio formats #! These features enable different audio formats @@ -161,6 +161,7 @@ iroh-quinn = { version = "0.11" } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } turborand = { version = "0.10.0", features = ["atomic"] } iroh-net = { workspace = true, features = ["discovery-local-network"] } +serde_json = { workspace = true, optional = true } directories = "5.0" diff --git a/framework_crates/bones_framework/src/networking.rs b/framework_crates/bones_framework/src/networking.rs index a1319498f9..eb9d1f7bd2 100644 --- a/framework_crates/bones_framework/src/networking.rs +++ b/framework_crates/bones_framework/src/networking.rs @@ -870,7 +870,7 @@ where if let Some(desync_hash_tree) = desync_debug_history.get_frame_data(frame as u32) { - let string = serde_yaml::to_string(desync_hash_tree) + let string = serde_json::to_string_pretty(desync_hash_tree) .expect("Failed to serialize desync hash tree"); error!("Desync hash tree: frame: {frame}\n{}", string); }