diasurgical / devilutionX

Diablo build for modern operating systems
Other
7.99k stars 781 forks source link

Blocking spells activates block animation and still deals damage #1531

Closed Psojed closed 3 years ago

Psojed commented 3 years ago

single_0.zip

We tested using a single succubus enemy type shooting their Blood Stars. Sometimes the spell is blocked completely, but sometimes the spell gets blocked but still deals damage. The same issue is present in vanilla Hellfire.

Hellfire 1.0.1: https://www.youtube.com/watch?v=8l9SDX6BPjQ (thx Maxpire) devilutionX 1.2.1 Hellfire: https://cdn.discordapp.com/attachments/764970679564501052/831966291337543680/snow_witch_blocking.mp4

In the second video you can see instances of "damage taken" sound triggering, but me not taking damage, that is most likely because I'm wearing 64% magic resistance and items with -5 damage taken.

The expected behaviour is that every blocked Blood Star should be fully blocked and deal no damage, regardless of damage reductions.


PS: Attached is my save from devilutionX 1.2.1 Hellfire in a position ready to test.

StephenCWills commented 3 years ago

I suspected as much just by watching the video, but I also confirmed this through testing. Looks like this is happening because the blocking logic causes the missile to glance off the player's shield/staff, allowing the missile to continue moving after the colliding with the player. Blood Star doesn't move fast enough to vacate the player's space in the next frame after the first collision frame so it ends up colliding with the player a second time.

Also, the save file reproduces another issue you can see in the snow_witch_blocking.mp4 video. The Blood Star projectile frequently disappears for several frames after the missile is blocked. When this happens, I get a bunch of these messages in the debug output.

INFO: Draw Missile 2 type 24: NULL Cel Buffer

This seems to suggest there's some missing animation data for the Blood Star projectile.

julealgon commented 3 years ago

Should we mark missiles as "already hit player X" or something like that? The missile would need to keep track of this for all players I guess, and then check if it has already hit the player on the hit check and skip if so.

StephenCWills commented 3 years ago

I couldn't think of a way to fix this that wouldn't involve expanding the missile struct or using the _miVarX variables. Your solution seems viable to me if we're willing to make use of _miVar3.

StephenCWills commented 3 years ago

Actually, there is a couple things I forgot to mention.

  1. A strange alternative solution I thought of would be to change the missile flags to have the missile target monsters instead of players after blocking. Obviously, this would change gameplay a bit.
  2. I tested the issue of the disappearing Blood Star in vanilla Hellfire and was able to reproduce it there.
AJenbo commented 3 years ago

woudn't just marking it for deletion or setting range to 0 work?

julealgon commented 3 years ago

woudn't just marking it for deletion or setting range to 0 work?

Shouldn't it still atempt to hit other players for example?

julealgon commented 3 years ago

What would be the most intuitive approach to me would be for the game to have 2 missile types: single-hit, and multi-hit.

For single-hit missiles, the missile should keep track of all entitities it has already made a hit check against, and avoid doing the checks against entities it has already done so before.

I have no idea how missiles work today so disregard if this makes no sense.

StephenCWills commented 3 years ago

As @julealgon suggested, if there was another player standing behind the blocking player, they could potentially still be hit by the missile. The game does delete the missile if the player is hit. Marking the missile for deletion would make it behave more like the spell had hit the player. That might be fine for Blood Star, but I wouldn't recommend doing that for all projectiles. For example, I believe arrows fly crooked when blocked, which is a nice touch.

Actually, I'm pretty sure that's the cause of the disappearing Blood Star. Whenever the following random number returned 1, the projectile would disappear for a few frames. If it returned 0 (resulting in -1), there was no issue. If you follow the method calls, it's basically looking for an adjacent animation in the mAnimData array for that missile, but Blood Star only has animation data at index 0. https://github.com/diasurgical/devilutionX/blob/2969b80163c4bbd5dbb624fdef28e8a7e017450c/Source/missiles.cpp#L1089

StephenCWills commented 3 years ago

I played around with editing projectile velocities, and this issue still occurs at arrow speeds. To be sure, I went ahead and tested arrows as well, and I can also confirm that this issue is not isolated to Blood Star. It's just that Blood Star is one of the slowest projectiles that the player has to deal with so it happens a lot more often.

qndel commented 3 years ago

woudn't just marking it for deletion or setting range to 0 work?

It probably would but why get rid of such a cool mechanic? Deflecting missiles and seeing them fly away is AWESOME

StephenCWills commented 3 years ago

True to my usual process, I decided to compile a list of missiles that would potentially be affected by this bug as well as those which should not be affected by this bug.

Affected

Should not be affected

Additional notes

AJenbo commented 3 years ago

??? MIS_LICH https://diablo2.diablowiki.net/Lich ??? MIS_PSYCHORB https://diablo2.diablowiki.net/Psychorb ??? MIS_NECROMORB https://diablo2.diablowiki.net/Psychorb ??? MIS_ARCHLICH https://diablo2.diablowiki.net/Lich ??? MIS_BONEDEMON https://diablo2.diablowiki.net/Skullwing ??? MI_Lightball() Individual Nova lighnings ??? MI_Krull() For an unused monster ??? MI_HorkSpawn() https://diablo2.diablowiki.net/Hork_Spawn ??? MI_Weapexp() weapons with fire/lightning damage

julealgon commented 3 years ago

??? MI_Flame() (unblockable)

Inferno?