SR version of Detectable Spells

[DavidW]

As discussed here, the version of DS used in SR (and SRR) creates some (fairly edge-case) bugs due to removing duplicate entries from stats.ids. The more up-to-date version of DS in SCS avoids that (and has other minor advantages) but wasn't previously compatible with SR because it required AUTO_EVAL_STRINGS (which is part of my standard modding environment but breaks SR).

The new version of DS I've just coded (see link here) doesn't require AUTO_EVAL_STRINGS, so you might want to update.

[subtledoctor]

I replaced the SR version of DS with that new one, and got a quick error:

Stopping installation because of error.
Stopping installation because of error.

ERROR Installing [Spell Revisions], rolling back to previous state
Will uninstall 359 files for [spell_rev/setup-spell_rev.tp2] component 0.
Uninstalled    359 files for [spell_rev/setup-spell_rev.tp2] component 0.
ERROR: Failure("Unknown function: ds_altered_spell")

I don't know enough about DS, or how SR uses DS, to troubleshoot this effectively or efficiently.  Maybe @Bartimaeus can help?

[Bartimaeus]

2 hours ago, subtledoctor said:

I replaced the SR version of DS with that new one, and got a quick error:

Stopping installation because of error.
Stopping installation because of error.

ERROR Installing [Spell Revisions], rolling back to previous state
Will uninstall 359 files for [spell_rev/setup-spell_rev.tp2] component 0.
Uninstalled    359 files for [spell_rev/setup-spell_rev.tp2] component 0.
ERROR: Failure("Unknown function: ds_altered_spell")

I don't know enough about DS, or how SR uses DS, to troubleshoot this effectively or efficiently.  Maybe @Bartimaeus can help?

Must strip the main_component.tpa of ds_altered_spell macro calls, as the macro no longer exists. If I recall correctly, the spell_shield.tpa (name might not be exact) also has one such call that must be deleted as well. From my understanding of what David said earlier in the thread, there is not really much of a need to try to replace the function and it's okay to just outright delete it.

Edited August 14, 2020 by Bartimaeus

[subtledoctor]

8 minutes ago, Bartimaeus said:

If I recall correctly, the spell_shield.tpa (name might not be exact) also has one such call that must be deleted as well. From my understanding of what David said earlier in the thread, there is not really much of a need to try to replace the function and it's okay to just outright delete it.

Ah - that's what was giving me pause.  Thanks!

[DavidW]

Correct. Theoretically that function call is trying to tell subsequently-installed mods (e.g. SCS) that SR has changed a spell so that DS needs to respond to it differently. But it was never fully and consistently used and is now defunct.

That said, there probably still are issues in how DS reacts to SR spell changes - it's never been quite a 100% consistent match, I think largely because DS wasn't really in Demi's comfort zone (reasonably enough). The vast majority of DS stats clone the portrait icon in the spell, which presumably is pretty stable between SR and the vanilla spell system, but there are exceptions - see DS lines 223-242. 

I should have a look myself at some point but I'm afraid it's not a priority. If someone else feels like it (e.g., go through the spells on lines 223-242 on an SR install and see if they're working) I might be willing to help out coding the changes. (It should be quite easy to do the way DS is currently coded.)

[Bartimaeus]

26 minutes ago, DavidW said:

Correct. Theoretically that function call is trying to tell subsequently-installed mods (e.g. SCS) that SR has changed a spell so that DS needs to respond to it differently. But it was never fully and consistently used and is now defunct.

That said, there probably still are issues in how DS reacts to SR spell changes - it's never been quite a 100% consistent match, I think largely because DS wasn't really in Demi's comfort zone (reasonably enough). The vast majority of DS stats clone the portrait icon in the spell, which presumably is pretty stable between SR and the vanilla spell system, but there are exceptions - see DS lines 223-242. 

I should have a look myself at some point but I'm afraid it's not a priority. If someone else feels like it (e.g., go through the spells on lines 223-242 on an SR install and see if they're working) I might be willing to help out coding the changes. (It should be quite easy to do the way DS is currently coded.)

I don't fully understand what lines 223-242 are doing/supposed to do to begin with, so I'm unsure how I would determine whether it's functioning correctly or not. I see resource names on the 1st column (presumably the resources being modified), the second column refers exclusively to IDS entries (matching the first column to the second column...presumably for detection purposes?). And the fourth column...is the specific effect it's matching/detecting against? Maybe? Might help if you just explained it quickly (if possible).

Edited August 14, 2020 by Bartimaeus

[DavidW]

ds.tph contains the actual detectable_spells function and its helper functions, but also the inlined data that the function uses (so that we can distribute a single file). In this version of DS, that data comes in a bunch of tables - for instance:

- the table in lines 63-147 is spells for which DS clones the portrait (142) effect block (to get durations etc) and sets the appropriate stat to 1. (spells in column 1, stat in column 2). These should work on SR without changes.

- the table in lines 152-209 is spells for which DS clones the portrait effect block and sets the appropriate stat to some other number. (Spells in column 1, stat in column 2, value in column 3.) Again these should work fine on SR.

- the table in lines 223-242 is spells where cloning the portrait effect block doesn't work and we need to clone another effect block. (Spells in column 1, stat in column 2,  opcode to clone in column 4). These are the main ones where I suspect DS may struggle with SR. 

[Bartimaeus]

Thanks, that all makes sense, I'll take a look and see what gets set correctly.

[CamDawg]

On Discord, we found an issue with this new DS when used with IWDification on oBG2. Essentially, EE versions (via IWDidifcation or SCS) use spells named sppr5XXa and sppr5XXb for the subspells of Righteous Wrath, whereas the original uses fixed spell names unrelated to what the main spell gets named. DS expects the former and craps out on the latter.

I'll be adjusting this on IWDification's end for the next release so no DS changes are needed.

[DavidW]

Yes, I can see how that could come about. (And my IWD DS testing is mostly with SCS's own version of the IWD spells, and mostly on EE.) I agree that matching oBG2 usage to the EE versions of IWD spells is probably best.

[Bartimaeus]

@DavidW

These are the problems I noted with the current official version of SR and the new ds.tph:

CLERIC_SUMMON_INSECTS -> SPPR319B, duration of 7 for the matched 215 effect
CLERIC_INSECT_PLAGUE -> SPPR517B, duration of 7 for the matched 215 effect
CLERIC_CREEPING_DOOM -> SPPR717B, duration of 7 for the matched 215 effect
SPRA302 is Non-Detection, not Spell Deflection; no match, so no problem?
All Spell Deflections except for GREATER_SPELL_DEFLECTION accounted for
Spell Trap - no match for 259; match for 201, but there's no set proficiency stat, but there is a set state stat
Spell Shield - no match for 226; use graphical effect (215), icon (142), or specific spell immunity (206) instead?
Physical Mirror - no match for 197; use icon (142) or protection from projectile (83) instead?

https://dl.dropboxusercontent.com/s/wbwuh9w4jnsw5vu/ds.tph

That's my current "corrected" file if you want to take a look at it, but bear in mind (and don't pay much heed to the fact) that I made corrections for IRR as well.

Edited August 20, 2020 by Bartimaeus

[DavidW]

Thanks, that’s great. Will look when I get a chance.