Desync in spellcasting and attacks

[qndel]

Caused by ab8afa7cd7e83b7df295cac7956cd6358f068728

To reproduce: just get 2 players, 1 casts/attacks once, 2 sees more than one cast/attack Only reproductible on lowest game speed, cast/attack multiple times, it doesn't always happen

[julealgon]

Could this be related to the "hold button" feature you added?

[AJenbo]

probably not

[obligaron]

I looked a bit at the problem and want to share what I discovered so far.

• When you click the right mouse button RightMouseDown is called. RightMouseDown calls CheckPlrSpell and CheckPlrSpell calls NetSendCmdLocParam3 with CMD_SPELLXY. No player.destAction is updated.
• When the next rendering happens ProcessInput is called and ProcessInput calls RepeatMouseAction. RepeatMouseAction calls CheckPlrSpell, because it thinks the player has new new command (player.destAction was not set). Then CheckPlrSpell calls NetSendCmdLocParam3 with CMD_SPELLXY. No player.destAction is updated. Note: This step can happen multiple times depending on the rendering speed / vsync and when right mouse button (relative to next game tick) is pressed.
• When a gametick is due multi_process_network_packets is called. multi_process_network_packets calls ParseCmd and ParseCmd calls OnSpellTile. Now player.destAction is updated to ACTION_SPELL. Note: OnSpellTile will be called multiple times depending how often NetSendCmdLocParam3 with CMD_SPELLXY was called before.
• After that game_loop is called and in this ProcessPlayers starts the spell and set player.destAction to ACTION_NONE.
• In the next rendering RepeatMouseAction sees that a spell is performed and didn't trigger a new NetSendCmdLocParam3 with `CMD_SPELLXY.

That's why on singleplayer/host everything is fine... but why is there a desync?

Host Example:

INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: OnSpellTile - MyPlayer: 0 Tick: 1451 INFO: StartSpell - MyPlayer: 0 Tick: 1452

The client gets the same messages as the host. So it should be okay or?

INFO: OnSpellTile - MyPlayer: 1 Tick: 1296 INFO: StartSpell - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: OnSpellTile - MyPlayer: 1 Tick: 1297 INFO: StartSpell - MyPlayer: 1 Tick: 1305

Not quite, cause the client gets the same messages but his turn/game_loop happen at a different time/are desynced. Cause of this the OnSpellTile after the (earlier) tick sees that the player is currently executing a spell (PM_SPELL). And now queue a second spell for the player (set player.destAction to ACTION_SPELL). This queued spell is the visible desync.

So I think the main culprint is the not aligned turn/game_loop call. I think this was always in the game. The repeat mouse action feature only made it happen (much) more often.

Possible fixes could be:

1. Align turn/game_loop calls with network messages.
2. Avoid sending multiple network messages in RepeatMouseAction (for one game tick).

The second option would be only a temporary fix, cause I think this desync could happen also with other network messages. But perhaps I missed or misinterpreted something, cause I'm not familiar with the network code. Anyway I hope this post helps a little bit for fixing the issue. 🙂

[AJenbo]

I'm no longer able to replicate this. Solution 2 has been implemented as well as other checks to ensure that spell casting is synced. As such I'm removing the critical label and moving it to a later release for a full refactor in to solution 1.