Devilutionx: Remove animation skipping on player attacks

[julealgon]

When attacking, the game allows the player to override the attack animation right after the "hit frame" with a new attack. This new attack then abruptly interrupts the previous attack animation and starts a new one. This makes the attack animation look bad as all the frames after the hit frame are lost.

I'm not entirely sure if this was done on purpose by Blizzard (I assume it was, since it is quite obvious), but this doesn't seem to exist for monster attacks or spells, only for player attacks.

Additionally, it seems like frames after the hit-frame are not affected by attack speed modifiers, which also seems inconsistent and discourages kiting. This easy to test with a fastest attack speed weapon by performing a single attack: the initial frames of animation are much faster, then we get the hit frame somewhere in the middle of the animation, and then all the frames after that (could be called attack recovery frames) are the same speed as in a normal attack.

It would be nice to have this quirk removed from the game so that we have a smoother, more consistent play experience.

Optionally, one could increase the overall speed of the whole animation so as to maintain the same attacks per second as the original version.

I'd like to help with this, if at all possible.

EDIT:

Here are a few animations to showcase what I'm proposing:

| Vanilla (animation skipping) | Smooth (full animation) |
| --- | --- |
| | |
| | |

[AJenbo]

Smooth animation is definitely on the road map, but it's going to be a bit tricky since the attack triggers by checking the frame number to see if it is the action (or hit frame as you called it) frame. Animations are sped up by simply skipping hardcoded frame IDs.
I'm thinking that this might need some major work where instead things happen based on logical frames that aren't tied to the animations and the animations can then run independently, to ensure that the game still behaves the same. But maybe there is a simpler solution to this.

[julealgon]

I found the place where attack skipping is enabled. It really seems like an intentional change, so this is going to be controversial 😆

Turns out this same skipping logic is what causes one to be able to use scrolls twice, and spells after no mana is available. This was interesting as the change ends up fixing that issue as a side effect.

I'll send the PR anyways and see what folks think about it. I don't think a setting to turn this on/off would make too much sense, however.

[julealgon]

After further investigation, this animation skipping and the double-casting bug are separate issues: removing the skip does _not_ fix the bug as I suspected.

[AJenbo]

If the hit frame is at the same time and this doesn't change the rate of fire then i'm all for it.

[malvarenga123]

This change effectively reduces your DPS, right?
Would it be possible to speed up the animation speed to maintain the same DPS?
Otherwise, you would need some balance in enemy health/weapon damage with this change.

[AJenbo]

I agree the current solution is to aggressive as it affects gameplay. I agree that the way it was originally done is a hack and doesn't look nice, but it was probably done for game balance.

[qndel]

well, it was the only way to make attacks faster, how else would you do that? : P

[AJenbo]

Not lock the animations speed to the render, not lock the render at 20fps. Increase the animation speed when you have a fast attack weapon, call it a day :P

[julealgon]

Let me try to answer your questions here 😛

---

@AJenbo

If the hit frame is at the same time and this doesn't change the rate of fire then i'm all for it.

Hit frame stays the same, yes. "Rate of fire" can be misleading... if you mean overall attack speed, no, that's going to be reduced by introducing all the frames that were previously skipped. The warrior would take 10 frames / attack if attacking in sequence, now it will be the full 16 frames / attack.

I agree the current solution is to aggressive as it affects gameplay. I agree that the way it was originally done is a hack and doesn't look nice, but it was probably done for game balance.

Yes, it affects gameplay. That's why I said it was going to be a controversial change 😆 . From the way the code looks, it really does seem to have been added later as a hack to make attacks faster overall. The method where the logic is is even questionable from my little understanding of this code.

More on game balance below.

---

@malvarenga123

This change effectively reduces your DPS, right?

Yes. Overall DPS will be reduced for longer swing sequences. The longer the sequence, the more prominent the reduction in DPS will be.

Would it be possible to speed up the animation speed to maintain the same DPS?

There is hardly a concept of "animation speed" in this engine at the moment. The entire engine operates at 20 FPS, and these attack animations just forward a frame per game frame. "Speeding up" an animation means cutting some of it's frames. That's actually how speed suffixes work today:

• "quick attack" skips 1 frame
• "fast attack" skips 2
• "faster attack" skips 3
• "fastest attack" skips 4

The only way to properly "increase the animation speed" would be to increase the internal game frequency, from 20 FPS to something higher, and then adjust animation frame persistence accordingly (there is actually a Delay property for animations, which indicate "how many times a given animation frame should be displayed before switching to the next frame". The idle animations use this heavily from what I could see on debugging sessions.

Otherwise, you would need some balance in enemy health/weapon damage with this change.

I don't think that's necessary (I know, controversial). More on that below.

---

@qndel

well, it was the only way to make attacks faster, how else would you do that? : P

That's true, but it clearly indicates they "messed up" when actually creating the animations. My feeling here is that they created the animations first, then noticed that it wasn't as fast as they wanted it to be, but instead of going back and recreating the sprites from the render, they just introduced frame skipping.

Also, the way they did it looks really poor IMHO, because contrary to what they did for speed suffixes (skip a few sparse frames here and there) they actually skipped a whole bunch of sequential frames (the ones after the hit-frame). End result is insane stuttering on all attacks (and some animations that look broken, like monks' kicks... check that out sometime).

---

So.... back to game balance. Yes, I'm fully aware this directly impacts game balance. However, I think this is fine. Now hear me out...

The original Diablo is an easy game. Pretty much all existing mods to this game have much harder difficulty than the original. I think the game is still very much easy even with this change in place. What this change does is make it easier for monster to surround you, which in turn make it more important to back step and avoid being surrounded. Overall, a massive improvement to gameplay if you ask me.

Obviously, this is just my opinion. I for sure will be playing with this modification in place even if I can't convince you folks of merging it into devilutionx 😆

What we should move towards IMHO is what @AJenbo mentioned.

• Fully decouple rendering from game logic
• Increase engine update frequency (from 20 FPS to 60 perhaps)
• With 60 frames, there is a lot of ways to animate a 16 frame animation to look slow and fast without compromising it's fluidity. You can introduce frame delays per frame (see current idle animation), or play it "without skips" for a very fast attack. Basically, reversing what the original game did.

What we could look into as part of this same change is perhaps add a few more skips to speed suffix logic. Maybe "quickness" skips the first, but also the last animation frame. Maybe "fast" skips 3 or 4 frames instead of 2, so on and so forth. If you think that's interesting, I could look into that as an option. The only challenge would be to ensure we are always skipping frame numbers that exist for all animations... the amount of hardcoding in this section is really annoying.

[malvarenga123]

What we should move towards IMHO is what @AJenbo mentioned.

• Fully decouple rendering from game logic
• Increase engine update frequency (from 20 FPS to 60 perhaps)
• With 60 frames, there is a lot of ways to animate a 16 frame animation to look slow and fast without compromising it's fluidity. You can introduce frame delays per frame (see current idle animation), or play it "without skips" for a very fast attack. Basically, reversing what the original game did.

Just my 2 cents, but I think you should only remove animation skipping when this was implemented.
That fast attack rate when you got a King's Sword of Haste, for example, felt really good. Even if the DPS doesn't matter, I think that feeling of speed should be kept.

[julealgon]

@malvarenga123

Just my 2 cents, but I think you should only remove animation skipping when this was implemented.

That's fair. While I disagree on a personal level since the game is just too easy anyway, I think a lot of people would agree with you there.

Since I see this as a hack in the original game, I'll probably play without it whenever I play on my fork. The constant animation stutter combined with the weirdness of only being able to skip it with another attack (meaning attempts to move etc don't work) is just too much for me 😆

Sending it as a PR was done exactly to gather reactions from folks here and see if it is worthwhile merging before any further engine changes, or if we should wait for those to come first. So I see 2 possible scenarios here:

1. The issue stays open, but the PR is closed. Issue can only be fixed in a way that preserves overall attack speed (needs drastic animation engine changes).
2. The issue is closed as done and PR is merged. We get back to balancing speeds later when the animation engine is updated.

That fast attack rate when you got a King's Sword of Haste, for example, felt really good. Even if the DPS doesn't matter, I think that feeling of speed should be kept.

I think you'd still end up having the same feeling on this version. To me, that satisfaction comes from the difference it makes, not necessarily it's actual speed. When you get used to the slower overall attack speed, the increases will still be felt. YMMV

[AJenbo]

1, but you are welcome to keep the PR as open if you change it to draft. That way others can find it and use it if they prefer it's behavior

[AJenbo]

Since we have already decoupled the frame rate from the game logic we "just" need to do the same for the animations.

I don't think we can/should target/require a specific fps like 60, instead we should make the animations time based, when rendering a game frame the renderer should calculate what would be the current animation frame of all ongoing animation.
@julealgon I think what you mentioned regarding giving each frame a delay time might actually be the solution here. If we then also set the animation start time of each actor then we can calculate what the current frame is at any given time, this can probably be optimized on. It would then probably not be too hard to change the code that looks at the hit frame to instead calculate what engine tick it should happen on (current tick + action frame - frameskip).

The most tricky part will be the attached animation, as the animation speed will change depending on if you have hasting equipment, and whether or not you have multiple attacks lined up.

The good news is that now that we have save games done per filed we are free to change the various structures.

A good place to start on this would be to add some tests that check if actions happen on the correct frame, then change the code to calculate the tick frame instead of looking at the animation's current frame.

[julealgon]

While I'm with you 100% here, I was wondering if there was an intermediary step we could use.

I mentioned the frame delay specifically because the current engine already has support for that. On every animation tick, the game checks the current delay value and increments a temporary counter if it has not been reached yet, while then _not_ moving forward in the animation for that tick. This is how the town idle animation works, each frame has a value of 2 or 3 set as a delay, so the actual animation has very few frames at the end of the day.

My idea of increasing the frequency of animation updates would be as follows.

1. Double or triple the update frequency
2. Make sure only animation updates are done on the new frequency and leave other updates alone
3. Double or triple the animation delay value accordingly (multiply it by 2 or 3 for animations that already have a delay, or add 1 or 2 for those that don't)

Now you have a much bigger range of frames to work with.

Since the standard speed is now delayed, removing the delay speeds it up, without losing any frames.

Take the warrior sword swing, which is 16 frames long. With triple animation speed, it would have a 2 frame delay for every frame, meaning a total of 48 frames of animation. 16 frames @ 20FPS results in the same speed as 48 frames @ 60FPS.

Now you can play around with these 48 frames in a way that you never have to skip a single "real" frame, just by reducing delays for various frames.

I think this would be somewhat achievable now without that many changes but I'd have to check.

[AJenbo]

It souds like this would also affect the tick rate for the game logic and there by network trafic and that's probalby not a good idea.

[julealgon]

While I don't quite understand every aspect of the engine yet, I think network packages are only sent on specific triggers, and not necessarily on every frame. For example, hit frames are still a single frame, no matter if we are running logic at 20 ticks/s or 40 ticks/s. Missile creation should only happen once as well, regardless of number of updates. So on and so forth.

If there are messages that are sent every frame no matter what, then I agree with you.