Desync detection appears to compare the checksum of predicted frames to confirmed frames

[nezuo]

Describe the bug
Desyncs are reported because predicated checksums and confirmed checksums are being compared.

To Reproduce
My repro can be found here on the desync-detection-issue branch: https://github.com/gschup/bevy_ggrs/compare/main...nezuo:bevy_ggrs:desync-detection-issue?expand=1

I used clumsy so that it predicts inputs/rollbacks.

Expected behavior
Only the checksums of confirmed frames should be compared.

Screenshots
Frame 18s for player 1 (in order):

Frame 18 for player 2:

The second frame 18 for player 1 is the same as player 2's frame 18. However, it's comparing the first frame 18 with player 2's frame 18.

[PraxTube]

Yeah, checksums as they are right now only work when there is no ping (or the ping is lower then the input delay), which is almost only the case when testing locally. However, when testing with high ping you get desyncs non-stop because the receiving client is constantly mispredicting and we have to rollback a bunch.

Comparing the checksum of only confirmed frames would fix this?

[gschup]

Oh wow, thanks for catching and reporting this. What an oversight!

Comparing only confirmed frames is indeed the answer here.

[johanhelsing]

Thanks for taking the time looking into this! It does indeed not happen if input delay is higher than ping.

I didn't catch this earlier, since my game breaks on rollbacks due to the regression #63

[johanhelsing]

This is bug in ggrs. I created a bug and a fix for it there (see referenced issues above).

...in other words, we should probably keep this issue open until we've updated to a fixed version of ggrs

[gschup]

The fix is now merged in the ggrs repo, so the issue can be closed here, as well! (unless I am missing something)

[johanhelsing]

Just wondering if it's good to keep it open until at least the main branch of this repo depends on a ggrs version that works (just for visibility in case this is tripping people up)

[gschup]

You are right. I was under the impression that the main branch of this repo depends on the main branch of ggrs. At least that was my original intention in the past. It must have slipped through my fingers that we don't do that currently.

[gschup]

this repo now depends on ggrs main, which should fix this issue.