diasurgical / devilutionX

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

Fix Teleport Desync #7382

Closed kphoenix137 closed 4 weeks ago

kphoenix137 commented 1 month ago

Since all non-wall spells can be cast with three modes (lock onto monster, lock onto player, lock onto tile), this creates a predicament with the one spell that changes player position. If the player casting Teleport casts locked onto a non-tile target, if that target's position is not synced between all clients at the time the casting animation completes, the Teleporting player will land next to the target and have a desynced position on all other clients until the game is able to correct it. This can result in a "warping" effect where the Teleported player is snapped to a new location after the correction occurs.

Regardless if desync is a factor or not, I get the impression that Teleport locking onto entities is just a result of how spells work, rather than intent, given that any spell being cast with an entity selected will lock the destination to that entity. It seems more intuitive to me that the Teleport spell would send you to the position your mouse is over, rather than locking on to something. This seems quite absurd behavior when for example, you cast a Teleport on a Teleporting player and their Teleport completes first and your Teleport is redirected to wherever this player lands.

StephenCWills commented 4 weeks ago

This actually changes the behavior of the spell on the local client as well. I haven't done any testing to really talk about how the change feels, but I would expect that players may notice the biggest difference when attempting to Teleport next to a Gargoyle because of their weird cursor hitboxes.

EDIT: I guess cursPosition is typically snapped to the tile of the selection under the cursor so at most it probably only makes a small difference if the selection is a moving target.

EDIT 2: cursPositionAbs is what I thought was happening in the first place.

kphoenix137 commented 4 weeks ago

This actually changes the behavior of the spell on the local client as well. I haven't done any testing to really talk about how the change feels, but I would expect that players may notice the biggest difference when attempting to Teleport next to a Gargoyle because of their weird cursor hitboxes. Good find. cursPosition will take the tile position of the highlighted entity which means this positional desync potential still exists.

I added a global variable cursPositionAbs to always track the tile under the cursor regardless of what's highlighted. Now teleport will always target where the player is aiming and won't lock onto entities.

kphoenix137 commented 4 weeks ago

I made a grave misjudgement about the nature of spell targeting. I guess I forgot. All spells acquire a target at the moment of casting; this does not update if the target is in a new location when the action frame is reached. Therefore this fix does nothing.