search

diasurgical / devilutionX

Diablo build for modern operating systems
Other
8.08k stars 794 forks source link

[Issue Report]: Wall spells produce twice as many visible missiles on the center tile #7205

Closed kphoenix137 closed 2 months ago

kphoenix137 commented 3 months ago

Operating System

Windows x64

DevilutionX version

1.5.2

Describe

Fire Wall (and Lightning Wall?) use a control missile to move over tiles, creating Fire Wall tiles that are visible to the player and damage entities. The incorrect implementation of this spell logic ends up placing 2 Fire Wall tiles at the direct center of where the Fire Wall spreads out from, which result in 2 visual Fire Wall tiles that end up dealing more damage to entities as a result. This should be corrected by modifying the implementation of GrowWall() so that one of the AddMissile() calls skips the first frame. (Credit to @StephenCWills)

To Reproduce

Observe Fire Wall derp

Expected Behavior

No response

Additional context

No response

julealgon commented 3 months ago

Nice find. Was the double damage issue documented before?

StephenCWills commented 3 months ago

@NiteKat mentioned the double damage when I was watching him stream his speedrun attempts the other day. That was the first I'd heard of it.

Fire Wall is created by 2 control missiles.

This isn't true. There's only one control missile, and it makes liberal use of missile vars to keep track of state when growing in both directions. You'd have to change the implementation of GrowWall() so that one of the two calls to AddMissile() can be skipped on the first frame.

julealgon commented 3 months ago

Might want to add the vanilla tag to this for documentation purposes.

kphoenix137 commented 3 months ago

@NiteKat mentioned the double damage when I was watching him stream his speedrun attempts the other day. That was the first I'd heard of it.

Fire Wall is created by 2 control missiles.

This isn't true. There's only one control missile, and it makes liberal use of missile vars to keep track of state when growing in both directions. You'd have to change the implementation of GrowWall() so that one of the two calls to AddMissile() can be skipped on the first frame.

My mistake, I was a bit too quick on the jump to document this issue before I forgot about it and didn't properly review the code (went from memory). I will update original post.

kphoenix137 commented 3 months ago

Can someone confirm this issue affects Lightning Wall as well?

qndel commented 3 months ago

The reason why I've added the "insufficient information" label is because of "Observe Fire Wall derp" - the least you could do is provide a screenshot/gif :/

kphoenix137 commented 3 months ago

The reason why I've added the "insufficient information" label is because of "Observe Fire Wall derp" - the least you could do is provide a screenshot/gif :/

I'm not saying you're wrong :D Problem is fixed with PR now. I intended to fix this myself but I wanted to quickly address it with an issue report in case it took me too long or someone else wanted to fix first.