Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Desync hash detection via DesyncHash trait, opt in with #[net] attribute #462

Open
wants to merge 29 commits into
base: main
Choose a base branch
from

Conversation

MaxCWhitehead
Copy link
Collaborator

#[net] may be used to opt in a type that implements DesyncHash such that when DesyncHashis used on its SchemaRef, it will contribute to hash. (Otherwise it does not modify hash). This allows us to hash the World, conditionally over resources / components we want included.

Desync detection may now be configured on ggrs session, and the default hash function, which uses everything opted in in World, can be overridden as well.

Right now only Transform is opted in - need to see if there are other core types we want. It is possible there are common types missing impls for DesyncHash, but tried to cover most of what I could think of.

I made the api update a hasher from value, instead of just returning a hash. Unsure if this will pan out to be the best move, might change it, or add both. It is nice to be able to bring your own hasher though, and I'm not sure I can make these functions generic as need function pointer to them...

Details

  • #[net] will error if used with something that is no_clone. It does not derive DesyncHash, but requires it.
  • I used FxHasher for default ggrs hasher solely because it's already in dependencies, perhaps it is not the best candidate, need to look into it more.

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Sep 15, 2024

@RockasMockas I added desync tree, it gets dumped on frames that detect desync. Need to enable feature desync-debug on bones_framework to build hash trees + log on desync.

Right now it will show which component/resource desynced if you compare the two logs, but it's hard to determine which entity it is that is desync'd for components.

Decent amount of stuff that needs cleanup, will comment more later but mostly pushing so can test with it. I would enable this, check the first frame that is desync'd, compare the logs, and see what component or resource is causing it (you will need to opt them in - I will add something to make this easier to see what needs opt-in later).

@MaxCWhitehead
Copy link
Collaborator Author

DetectDesyncs is now optional on ggrs session runner info, has desync detection config such as frame interval, world hash function override, and if the tree should include unhashable nodes. Nodes now optionally have a hash, so an unhashable node will be present with None.

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Sep 20, 2024

#[desync_exclude] may now be used on named struct fields or named enum variant fields to opt-out of hashing them.

May also be used directly on enum variant to opt out all of its fields (but enum variant itself still included in hash).

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Nov 3, 2024

Rebased this onto latest + Added the Name component for named entities. The desync tree now stores metadata for component leaves with their entity id, and maps this to name (if entity is named) and stores in tree to help identity entities/components. A bit hacky at points but seems to work, and is self contained in tree build.

Did a bit of cleaning up of the desync tree types, made less generic and simplified. Made it iterable, mostly for testing.

I think this will probably be the last feature for now, will be doing cleanup pass next / prepping for merge.

@MaxCWhitehead MaxCWhitehead marked this pull request as ready for review November 7, 2024 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant