Fix false positives in remote desync detection

[johanhelsing]

Fixes #63 and also fixes a bug that checksums are removed prematurely if the interval is > MAX_CHECKSUM_HISTORY_SIZE

Confirmed working with the test case for #63

Bonus: Remote checksums are now only checked once (removed once verified).

[johanhelsing]

@HeatXD may want to review, because they did the initial implementation #50

[PraxTube]

I keep getting the following panic right after peers connect in my bevy_ggrs project.

thread 'main' panicked at 'cell not found!: frame 100', ggrs-c119670c0a115c50/94604ce/src/sessions/p2p_session.rs:921:44

The frame not found seems to depend on the specified desync interval.

.with_desync_detection_mode(DesyncDetection::On { interval: 100 })

Changing the max prediction window and max frames behind didn't do much either

.with_max_prediction_window()
.with_max_frames_behind()

Is there anything that has to be configured in order for this PR to properly work in
exisiting bevy_ggrs projects, or this not expected to work with bevy_ggrs at all yet?

[johanhelsing]

I keep getting the following panic right after peers connect in my bevy_ggrs project.

thread 'main' panicked at 'cell not found!: frame 100', ggrs-c119670c0a115c50/94604ce/src/sessions/p2p_session.rs:921:44

The frame not found seems to depend on the specified desync interval.

.with_desync_detection_mode(DesyncDetection::On { interval: 100 })

Changing the max prediction window and max frames behind didn't do much either

.with_max_prediction_window()
.with_max_frames_behind()

Is there anything that has to be configured in order for this PR to properly work in exisiting bevy_ggrs projects, or this not expected to work with bevy_ggrs at all yet?

It's expected to work with bevy_ggrs. But very possible that I overlooked something.

I tested with box_game_p2p on this branch https://github.com/gschup/bevy_ggrs/compare/main...johanhelsing:bevy_ggrs:trigger-desync-issues?expand=1 and in my own game.

[johanhelsing]

@PraxTube are you able to reproduce your panic by modifying this branch? https://github.com/gschup/bevy_ggrs/compare/main...johanhelsing:bevy_ggrs:desync-debug?expand=1

EDIT: Managed to by setting input_delay back to 2. Will look into the issue

[PraxTube]

Ah yeah, for input_delay = 0 I don't get the panic (I had it on 2 in my tests). The desync detection seems to work properly when using 0 delay.

[johanhelsing]

I added a test, which is good in itself, but for some reason I don't get the panic there :/

EDIT: managed to repro with a higher input delay :)

[johanhelsing]

This is ready for review now. There is one thing that bugs me (see comment above), but I'm giving up trying to understand it for now.

This is a lot more correct than the previous implementation. And has a test case that failed prior to this PR (and another one to make sure the panic reported by @PraxTube is fixed)

[gschup]

This looks correct to me and everything is structured the same as it was before, so I really can't complain. Nice cleanups here and there :)

I am still baffled that this very obvious detail of comparing checksums only for confirmed frames didn't occur to me in the original PR.