Equipment short cuts causes animation desync

[AJenbo]

Operating System

Windows x64

DevilutionX version

1.4.0 (latest release)

Describe

When switching or changing equipment using ctrl+click or shift+click it can cause the players to appear to be walking in place on the other end.

See the following video around the 25min mark https://www.twitch.tv/videos/1534790732

To Reproduce

ctrl+click shield during multiplayer

Expected Behavior

Player should continue to animate as normal.

Additional context

No response

[AJenbo]

@obligaron any insight :)

[obligaron]

I tried to reproduce this in master and 1.4. But had no luck. 😢

I noticed some interesting things from the video, when the bug happens

• You never see the sorc moving normally. When he moves he disappears.
• You never see the sorc attacking.
• You always see the sorc "walking" at the same direction.
• It seems like the "walking" animation is only shown when the sorc is standing.

I thought about a way how you could get the moonwalking. When swapping gear, we call changeAnimationData. The logic assumes swapping gear can only happen when walking or standing. But this is not necessarily true when you are in multiplayer and a remote player swaps gear. On your local game the player can do something else cause of a desync or latency (in 1.4 for example #2681). The swap command is still send. So we can end up with calling changeAnimationData with walk graphic but doing something else (for example attacking). But I'm not sure if this is really related, cause this desync would be fixed after any call to NewPlrAnim.

[StephenCWills]

I tried to reproduce this in master and 1.4. But had no luck. 😢

Add this code to the DebugToggle handler to force a desync when pressing that key.

TCmdLoc cmd;
cmd.bCmd = CMD_SATTACKXY;
cmd.x = cursPosition.x;
cmd.y = cursPosition.y;
NetSendHiPri(SNPLAYER_OTHERS, (byte *)&cmd, sizeof(cmd));

Changing equipment during the desync immediately reproduced the issue for me.

[qndel]

I tried to reproduce this in master and 1.4. But had no luck. 😢

Add this code to the DebugToggle handler to force a desync when pressing that key.

TCmdLoc cmd;
cmd.bCmd = CMD_SATTACKXY;
cmd.x = cursPosition.x;
cmd.y = cursPosition.y;
NetSendHiPri(SNPLAYER_OTHERS, (byte *)&cmd, sizeof(cmd));

Changing equipment during the desync immediately reproduced the issue for me.

omg someone used debug toggle! I knew it'd be useful one day :D

[StephenCWills]

lol, I've used it a few times now. I didn't know you were anticipating this or I would've said something sooner.

[obligaron]

Thanks @StephenCWills that helped a lot. 🙂

The issue is that the attack animation and the walk animation have different number of frames. That means player mode is still PM_ATTACK while numberOfFrames is 8 (set from changeAnimationData with walking graphics. This is done to prevent to display frames that don't exist). In DoAttack we check if the animation is finished and start standing after that. But this check is not performed with numberOfFrames. It uses _pAFrames that is higher then 8 (depends on class). That means the animation is cycling (currentFrame is set to 0). And so the remote player never leaves the PM_ATTACK player mode.

A possible fix could be to always change to the correct graphic (including numberOfFrames and ticksPerFrame).