[partly vanilla] Issue when walking and changing weapon

[Chance4us]

There is an issue when walking and changing weapon at the same time. How to reproduce.

1. Load attached savegame
2. Open the inventory
3. Start walking in any direction
4. Start changing weapons while the hero is walking (The time interval between the changes must not be too long)

Screenshot:

Short video how it looks like: output.mkv.zip

Caused by commit 7ed009ecb71804bfe50a8f58d24417430aaeb7dc.

In vanilla, the problem is also present, rogue slides but the effect is shorter. Short video how it looks like in vanilla: vanilla.mkv.zip

Savegame: glitch.zip

[Chance4us]

By the way, the lighting also gets very bright.

[qndel]

[AJenbo]

@obligaron any clue to what's happening :)

[FitzRoyX]

The game stops you from unequipping items while attacking. Perhaps a similar check should be added to moving?

[obligaron]

@obligaron any clue to what's happening :)

Only after I started debugging. 🧐

The game stops you from unequipping items while attacking. Perhaps a similar check should be added to moving?

this is a possible solution. 🙂

Culprit

The issue (in vanilla and devilutionX) is this line. It's start a new animation (NewPlrAni with Stand) without setting the _pmode to PM_STAND. That's why we see the stand animation while walking.

Difference vanilla to devilutionX

The reason why vanilla is shorter then devilutionX is that the counting differs. In vanilla actionFrame is used and actionFrame doesn't get reset by NewPlrAni. So we simple show some actionFrame with the walk animation. 😉
In devilutionX (after 7ed009e) we use use CurrentFrame to calculate the progress for the walk animation. CurrentFrame is reset byNewPlrAni, so we call PM_ChangeOffset oftener. Also note one cycle of the stand animation is longer then the walk animation cause of the specified delay. That's why the effect is worse in devilutionX.

Possible solutions

1. Disallow unequipping items while walking. This is similar to attacks, so it would be consistent.
2. Instead of calling NewPlrAni hot-swap the AnimationInfo.pData (CEL2 Data). This is elegant but would require that all animations have the same length. This is currently the case but could differ if we supported loading this animation info's dynamic and a mod would change them. Example mod change: walking with heavy armor takes longer.
3. Stop walking and call StartStand Similar to the block animation, we simply would stop walking and start standing. This would give the hero time to adjust to his new equipment. 😁

All solutions are possible. Currently I have coded solution 2 (see example) but the others are also good and valid solutions. Let me know what solution is wanted and I will open a pr. 🙂

Example video for solution 2

https://user-images.githubusercontent.com/25415264/118388049-6552ca80-b622-11eb-94d6-51cd44956299.mp4

[Chance4us]

The first solution would change the gameplay too much, that would make the hero much more vulnerable when running away. The third solution would also change the game logic of vanilla a bit and this should first be tried out over a longer period of time to be able to judge about it.

The second solution looks like it wouldn't change the game in any way. I vote for second although this isn't optimal for mods, because a creator of a mod have to make the necessary adjustments by himself anyway.

[Chance4us]

I already tried out from your repository and it works so perfectly that it is better than vanilla. Thanks a lot for the investigation.

[obligaron]

Thanks for reporting and testing the changes. This really helps a lot. 🙂

[AJenbo]

2\. Instead of calling `NewPlrAni` hot-swap the `AnimationInfo.pData` (CEL2 Data).
    This is elegant but would require that all animations have the same length. This is currently the case but could differ if we supported loading this animation info's dynamic and a mod would change them. Example mod change: walking with heavy armor takes longer.

This seams like the right solution, it fixes the bug from Vanilla. I know some people aren't able to do things while walking, but it goes to reason that the heros dex is high enough for this :D

Thanks for looking in to the source of the issue.

[obligaron]

This seams like the right solution, it fixes the bug from Vanilla. I know some people aren't able to do things while walking, but it goes to reason that the heros dex is high enough for this :D

Thanks for looking in to the source of the issue.

No problem. 🙂 PR is online #1988 :rocket:

Should we add vanilla tag as this also occurs in vanilla?

[Chance4us]

Should we add vanilla tag as this also occurs in vanilla?

I vote yes, because it is a part of the issue.

[AJenbo]

Yeah solving this requires solving a vanilla bug