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

Enable desync detection in p2p example #48

Merged
merged 3 commits into from
Mar 30, 2023

Conversation

johanhelsing
Copy link
Collaborator

This is a very useful feature, we should either enable it by default, or at least inform about it in example code.

Unfortunately, I think it's currently broken? I get desyncs as soon as the game starts without giving any input.

@HeatXD

@HeatXD
Copy link

HeatXD commented Feb 9, 2023

Mm just tested it in the normal examples and it worked fine. i will look at this.

@johanhelsing
Copy link
Collaborator Author

looks like the actual checksums are different. I added a log statement in GameStateCell::save:

Peer 1

GGRS Event: Synchronizing { addr: 127.0.0.1:7000, total: 5, count: 1 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7000, total: 5, count: 2 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7000, total: 5, count: 3 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7000, total: 5, count: 4 }
frame: 0, checksum: Some(377796134171315194)
frame: 0, checksum: Some(377796134171315194)
GGRS Event: Synchronized { addr: 127.0.0.1:7000 }
frame: 1, checksum: Some(7837177710828900633)
frame: 2, checksum: Some(15495205509060815232)
frame: 3, checksum: Some(11699597999696539875)
frame: 4, checksum: Some(11020218334025782011)

Peer 2:

GGRS Event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 1 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 2 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 3 }
GGRS Event: Synchronizing { addr: 127.0.0.1:7001, total: 5, count: 4 }
frame: 0, checksum: Some(11950335332055537129)
frame: 0, checksum: Some(11950335332055537129)
GGRS Event: Synchronized { addr: 127.0.0.1:7001 }
frame: 1, checksum: Some(10010806704571646955)
frame: 2, checksum: Some(16190560606418438605)
frame: 3, checksum: Some(3571966077237507259)
frame: 4, checksum: Some(9411370770399689895)

@HeatXD
Copy link

HeatXD commented Feb 9, 2023

Does bevy's worlds have some non gameplay relevant state which is included in the snapshot?
making the desync function fire off?

@johanhelsing
Copy link
Collaborator Author

At least resource.reflect_hash() seems to return a different value each time I start the app, even if I disable the increase frame system. Maybe the hash is seeded or something?

@johanhelsing
Copy link
Collaborator Author

seems they're using AHasher::default in bevy

@johanhelsing
Copy link
Collaborator Author

bevy_utils depends on ahash with default features, which includes its std feature, which means the hasher keys will be generated on the first invocation: https://docs.rs/ahash/latest/ahash/struct.AHasher.html#method.default

In other words, as far as I can see the desync detection mode can't be used without patching bevy.

@johanhelsing
Copy link
Collaborator Author

Was able to make it work with this branch: https://github.com/johanhelsing/bevy/tree/fixed-reflect-hash-0.9.1

@gschup
Copy link
Owner

gschup commented Feb 10, 2023

very interesting! let's see how this goes :)

@johanhelsing
Copy link
Collaborator Author

Tested it in Cargo Space as well, works great!

And I had no desyncs 🎉 (or at least not any for my components that implement reflect_hash)

Disconnections are warnings because it is something that can happen by
normal usage. Desyncs are errors because it's something you'd probably
want to fix as the developer of the game.
@gschup
Copy link
Owner

gschup commented Mar 30, 2023

is this PR still a draft?

@johanhelsing
Copy link
Collaborator Author

We need bevy 0.10 first (because we needed the deterministic, bevyengine/bevy#7583), but otherwise I think it's ready.

@gschup gschup marked this pull request as ready for review March 30, 2023 06:53
@gschup gschup merged commit 7ef8de3 into gschup:main Mar 30, 2023
@johanhelsing johanhelsing deleted the enable-desync-detection-in-examples branch March 30, 2023 06:55
@johanhelsing
Copy link
Collaborator Author

Did a Ggrsevent use get lost in the merge?

@gschup
Copy link
Owner

gschup commented Mar 30, 2023

yes, I made a mistake but pushed a fix already :) Sorry, my bad!

@johanhelsing
Copy link
Collaborator Author

Yeah, no problem! Just trying to help. Thanks for the super quick release!

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.

3 participants