[bug] spotted on Reddit: Hallowed Redeemer vs. Fire Shield

[subtledoctor]

Causes an infinite loop. Apparently the originals did not, it is purportedly an EE-introduced bug.  

Edited March 22, 2022 by subtledoctor

[Luke]

Courtesy of @jmerry:

The 2.6 patch updated fireshields in several ways, one of which was to route the retaliation damage through a new projectile. This has an unintended side effect; if one creature with a fireshield up hits another, damage bounces back and forth every tick until either one of the creatures is dead or they're out of range of each other.
This component fixes that issue, reverting "dueling fireshields" to the older behavior of delivering exactly one hit each way.

[jmerry]

This is more general than Keldorn's sword, of course. I first noticed it when my Sun Soul Monk with Greater Sun up punched a mage with Fireshield: Red. Any two fireshields can set it off.

The specific nature of that component: find every spell that uses missiles "One_Target_Range_05", "One_Target_Range_07", or "One_Target_Range_10"; these are the fireshield subspells. Then have every spell in that group give the caster one-tick immunity to every spell in that group. I tried immunity to the projectile instead; that didn't work. Those missiles have different numbers between games, by the way; they're 344 through 346 in BG2EE and 363 through 365 in BGEE, because of new stuff added for SoD. I've shared an earlier version of the component's code on these forums before, but that's not current - I was using numbers rather than checking MISSILE.IDS, and so my first version didn't find any fireshields in BGEE.

The full static list of fireshield subspells in BG2EE: BALSHLD2, BDSHA12A, KELDORN, LEAT23A, SPCL237D, SPIMIXD, SPPR730D, SPPR951D, SPPR952D, SPWI403D, SPWI418D. I don't have a convenient list written down anywhere for any other games.

[DavidW]

22 hours ago, subtledoctor said:

Causes an infinite loop. Apparently the originals did not, it is purportedly an EE-introduced bug.  

No, it could happen in the originals. I recall seeing it several times.

[subtledoctor]

1 hour ago, DavidW said:

No, it could happen in the originals. I recall seeing it several times.

I know there were several instances of this sort of thing in the old engine, but it has been long enough that I cannot argue either way whether this particular combo did or didn't. I was just taking the linked post at face value. I do believe jmerry when he says something changed here specifically in the 2.6 patch, which deserves addressing in a FixPack and/or a putative 2.7 update.

EDIT - actually I just tested and confirmed that in EE 2.5 it works as expected (Keldorn strikes Charname with Stoneskin and Fire Shield Red, once: Charname takes 5 magic damage once and Keldorn takes a bit of fire damage once) while in EE 2.6 it generates a cascade and Charname goes to zero hp in less than one second.

So jmerry's fix seems like a good candidate for inclusion. From a modding perspective, I don't love that it uses a static whitelist of subspells for 206 protection, given that this will be installed before other mods; but if the purpose is to fix the unmodded game, then it should be fine. In the interest of creating a stable base to which mods can be added, my only suggestion would be, instead of adding multiple 206 effects to every Fire Shield-like spell, maybe it would be better to put those 206 effects into a single universal subspell and apply that subspell in every Fire Shield-like spell. That way, a mod that comes along and adds Mestil's Acid Sheath or Gedlee's Electric Loop etc. can simply 1) patch their damage spells into the unversal subspell, and 2) apply the universal subspell in the added spell.

[jmerry]

I didn't use a static list in my released mod. I just posted it here because this is a fixpack we're working on. That list was in fact generated by my mod running over every spell and looking for the relevant missiles, and looked up by checking one of the altered spells in NI.

The specific 2.6 change that did this:

- Pre-2.6 fireshields had no missile, and any "out of range" issues generated "Contingency failed" messages.

- 2.6 added a trio of missiles/projectiles: One_Target_Range_05, One_Target_Range_07, and One_Target_Range_10. Fireshield subspells, and only fireshield subspells, each use one of these for their damage. No contingency messages, because now the projectile just misses, but the infinite loop is on instead.

The subspell sounds like a good idea to me. Less clutter, and an easy standard to add on to with new mod-added fireshield effects. Though, testing required - is it fast enough?

[subtledoctor]

1 hour ago, jmerry said:

I didn't use a static list in my released mod. I just posted it here because this is a fixpack we're working on. That list was in fact generated by my mod running over every spell and looking for the relevant missiles, and looked up by checking one of the altered spells in NI.

Yes, the problem is not with your method but with the fact that this is intended to be #1 in the install order. So a FixPack can in fact use a static list - and generating the list programmatically is of no benefit because no mods will be installed first. Worse, is other mods add spells that re-introduce this problem, a player might reasonably think "well I already installed the Fixpack so I don't need jmerry's fix" and thus fail to fix those mod spells.

Now, at the end of the day it is the responsibility of mods to add content that doesn't involve insta-death loops - not the responsibility of this FixPack. But the single subspell is just an idea to make that as simple as possible. Agree testing is required.

[jmerry]

I ran a quick test on the subspell method, using blue and red (mage) fireshields. I wasn't entirely consistent about it, placing the cast effect before the damage effect in red and after in blue.

It stopped the loop, but not in a consistent way. When red attacked blue, I got one round of blue damage and that was it. When blue attacked red, I got either one round each way or two rounds each way.

Then I retested, with the effects in the same place on each spell. That was even worse, running anywhere from no fireshield damage at all to about a second's worth of loop.

I think we're stuck with the long list of 206 effects.

[subtledoctor]

What was the duration of the subspell? Any reason you had it set for one tick? Why not one whole second? I know that could potentially bock other applications (I guess if you somehow strike two different people with Fire Shield at almost the same time?), but the chance of it actually having a material effect seems remote. 

[jmerry]

The 1-tick duration? kjeron's suggestion in this Beamdog forum post: https://forums.beamdog.com/discussion/comment/1178754/#Comment_1178754. It works reliably for the version with all of the 206 effects in the fireshield subspells, but apparently not with those spells themselves casting a new subspell.

And I don't think that a longer duration would fix the issues. Particularly not when the fireshields do less than one complete cycle of damage. I don't know how that happens, but it does. Sometimes.

Others are welcome to run their own tests, of course. If someone figures out a way to make something better work consistently, I'd love to see it.

Edited March 24, 2022 by jmerry

[jmerry]

I had an idea for a different way of doing things. I tested it - unreliable. Then I got frustrated, and tested the stuff I had already written - and it turns out that's unreliable too. Back to the drawing board.

I started trying things. A longer duration? Now it's consistently too fast. Then I tried something really simple - and it worked. Just have each fireshield hit grant its target immunity to that particular fireshield subspell after dealing its damage. Duration 2 ticks. It seems to work, but I've been fooled before. Please test.

Also, while messing with this, I noticed that fireshield hits have inconsistent power levels. Some of them have power level 4; this actually looks like Fireshield Red and a bunch of spells that were cloned from it (Balors, Imix, Aura of Flaming Death, Greater Sun). Others, including all of the non-fire effects, have power level zero. I'm standardizing all of them to power level zero, so level-based spell protections don't interact with the damage.

And here's the code.

/* New fireshield code. Please test and cover any missing bits. */

ACTION_IF FILE_EXISTS_IN_GAME ~#rdremov.SPL~ BEGIN /* Patch 2.6 check */
  ACTION_FOR_EACH spell IN ~LEAT23A~ ~bdsha12a~ ~KELDORN~ ~spcl237d~ ~SPPR730D~ ~SPPR951D~ ~SPPR952D~ ~SPWI403D~ ~SPWI418D~ ~BALSHLD2~ ~SPIMIXD~ BEGIN /* Fireshield list for BGEE and BG2EE. May be missing something from SoD - please check by searching for projectiles RANGE05, RANGE07, RANGE10. This search picks up two false positives in non-SoD BGEE - mephit flame fan SPIN938 and hellhound flame breath SPIN956, both unused. */
    ACTION_IF FILE_EXISTS_IN_GAME ~%spell%.SPL~ BEGIN
      COPY_EXISTING ~%spell%.SPL~ ~override~
        LPF ALTER_SPELL_EFFECT INT_VAR match_opcode=12 power=0 END /* Power level is inconsistent between spells. 4 for LEAT23A, SPCL237D, SPPR730D, SPPR952D, SPWI418D, BALSHLD2, SPIMIXD, zero for all others. Standardizing to zero so damage doesn't interact with deflection/turning/globe effects. I think the level 4 damage effects are a mistake that got cloned a bunch of times. */
        LPF CLONE_EFFECT INT_VAR match_opcode=12 opcode=206 parameter1=0 timing=10 duration=2 STR_VAR insert=last resource=~%SOURCE_RES%~ END /* Each hit grants its target 2-tick immunity to further hits from the same fireshield. I think this one actually works. */
      BUT_ONLY
    END
  END
END

 

[CamDawg]

This is added, though I did keep power levels for a handful of these--I think it's entirely appropriate for, say, GoI to block bogstandard arcane Fireshield damage.