diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.01k stars 786 forks source link

Fix Phasing Desync #7381

Closed kphoenix137 closed 1 week ago

kphoenix137 commented 1 month ago

Phasing relies on RNG to pick a destination. Since RNG state isn't properly synced between clients, this means that all clients run RNG to decide the Phasing location for any player, which can result in different outcomes, then the game needs to correct the position of the player who cast Phasing.

This fix removes the RNG calculation that occurs on each client when the spell executes. Instead of calculating the target position at this point in time, the caster calculates the position and sends the calculated target position with the packet information for casting the Phasing spell, with the new position syncing properly for all clients.

NiteKat commented 3 weeks ago

Just curious, the other clients not burning RNG won't cause further desync issues? I'm guessing if the Phase result was going to be different, things are already desynced and it's not a concern, but just thinking out loud.

kphoenix137 commented 2 weeks ago

AFAIK the RNG state is never synced between clients

StephenCWills commented 2 weeks ago

RNG is haphazardly synced via monster AI.

https://github.com/diasurgical/devilutionX/blob/a0171b1b5a59464df57e10128548afb70382e5a5/Source/multi.cpp#L213-L219

It's hard for me to say what impact the RNG sync actually has on reducing the frequency of desync. My guess would be.. very little impact, considering it doesn't account for latency at all.

kphoenix137 commented 2 weeks ago

RNG is haphazardly synced via monster AI.

https://github.com/diasurgical/devilutionX/blob/a0171b1b5a59464df57e10128548afb70382e5a5/Source/multi.cpp#L213-L219

It's hard for me to say what impact the RNG sync actually has on reducing the frequency of desync. My guess would be.. very little impact, considering it doesn't account for latency at all.

Color me wrong. Although hopefully this fix helps a little bit by not broadcasting the wrong destination for any amount of time for phasing.

StephenCWills commented 1 week ago

Some basic testing reveals this PR doesn't resolve the desync.

kphoenix137 commented 1 week ago

Some basic testing reveals this PR doesn't resolve the desync.

That's a shame