SR fixes questions.

[grodrigues]

Here are some questions related to particular fixes. All of these were dealt with in a previous post of mine but no definite answer was given:

- Scaling: the biggest one. Several spells are documented as having a scaling but in the implementation they do not. What is wrong, the docs, the implementations, depends on the case? Assuming it is the implementation how should I go about fixing it? I suggest this. Suppose the spell has level l, and that the first l level spell is gained at character level c. Then change the first header to min level 1 but with scaling corresponding to character level c - 1 then add headers starting at c until the level cap 20.

- Glitterdust: Has protection against non-existent SPPR114F. Delete opcode? If not, what is the fix?

- Spell Thrust: trigger and explosion radius of projectile dvsplthr.pro do not coincide. Fix? And what is the radius to use?

- Fire Shield: The range of the spell retaliating with fire damage is 6 instead of documented 5. Fix or is +1 accounting for some engine quirk?

- Secret Word: says it dispels Dispelling Screen. Are the docs correct, as in it is intended as such? And if so, then the implementation seems incorrect as it lacks the proper remove sectype opcode for k1-dispel (or whatever its exact name is). How to add it, since the implementations relies on casting auxiliary spells and so it it is bound to affect other spell protection removal spells?

- Greater Malison: The "take an extra point of spell damage for each die rolled" is implemented as a -1 penalty to luck and +1 to thac0 (??). This is clearly not correct, but what is the exact fix?

[subtledoctor]

58 minutes ago, grodrigues said:

- Scaling: the biggest one. Several spells are documented as having a scaling but in the implementation they do not. What is wrong, the docs, the implementations, depends on the case? Assuming it is the implementation how should I go about fixing it? I suggest this. Suppose the spell has level l, and that the first l level spell is gained at character level c. Then change the first header to min level 1 but with scaling corresponding to character level c - 1 then add headers starting at c until the level cap 20.

Probably worth going through instances individually if you find them.  Can you give an example?  I know Demi was fairly averse to spells scaling every level (probably because he worked in NI and it is a huge PITA to apply that in NI).  I vaguely recall that SR prefers to scale spells by "every 3 levels" or "every 5 levels" etc.  And for durations, it often removes scaling in favor of 1 turn/2 turns/5 turns set durations.  (See e.g. Barkskin for a good example of the "SR way" of doing spell scaling.  One set duration, three different leveled effects, so three headers in the spell.  We have nice things like CamDawg's function for level-based scaling, so we are not bound by Demi's limitations... but frankly I like the straightforward nature of the current SR approach anyway.)

 

58 minutes ago, grodrigues said:

- Glitterdust: Has protection against non-existent SPPR114F. Delete opcode? If not, what is the fix?

SPPR114.spl was, pre-4b17, Faerie Fire.  I gather SPPR114F is a subspell that sets the "dvgldust" local variable to 0 on the target, at the end of the duration of FF.  FF itself sets that LV to 1 on the target for its duration, and makes the target immune to the subspell for its duration (to prevent LV being reset to 0 for the duration of the effect, in case of multiple castings).  (It uses a subspell to undo the LV because I think opcode 309, which sets LVs, can only be used with permanent timing.) 

Glitterdust, SPWI224.spl in pre-4b17,  also sets the same LV to 1, and applies its own subspell to undo it (SPWI224D), and also makes the target immune to its own subspell as well as FF's subspell, for its duration.  With the reasonable idea that Faerie Fire is basically a single-target Glitterdust, and Glitterdust is basically an AoE Faerie Fire.

Since SR now uses ADD_SPELL and spells do not have static RES, the fix must be in Weidu.  When Glitterdust is added it must look up FF's subspell filename and apply it to its internal 206 effect; and when FF is added it must lookup Glittersdust's subspell filename and add it to its internal 206 effect.  Of course, that cannot happen - whichever spell is added later will not exist when the earlier one is added.  (This is an example of why Demi so resisted using ADD_SPELL, I think.  Well, it illustrates part of the reason anyway.) 

Frankly I think the easiest answer it to have both spells use the same subspell.  Give it a name relating to the LV, and use it in the 146 and 206 effects in both Faerie Fire and Glitterdust.  Easy-peasy, I think.

 

Quote

- Spell Thrust: trigger and explosion radius of projectile dvsplthr.pro do not coincide. Fix? And what is the radius to use?

I don't know much about projectiles, gonna have to punt here.  Someone like Mike or kreso or kjeron might know.  I do know vaguely that some values in projectiles don't match what you might expect them to.  So it's possible this is working as intended.  Are you seeing weird behavior in-game, or did you just notice the mismatch in NI or something?

In pre-4b17, I see "Trap Size" = 84, and "Explosion Size" = 150.  I don't know how or whether they interact with each other, or which one is actually relevant to the spell's effect.

 

Quote

- Fire Shield: The range of the spell retaliating with fire damage is 6 instead of documented 5. Fix or is +1 accounting for some engine quirk?

Ditto.  Could be intentional - maybe for Fire Shield users with large selection circles, or something?

 

Quote

- Secret Word: says it dispels Dispelling Screen. Are the docs correct, as in it is intended as such? And if so, then the implementation seems incorrect as it lacks the proper remove sectype opcode for k1-dispel (or whatever its exact name is). How to add it, since the implementations relies on casting auxiliary spells and so it it is bound to affect other spell protection removal spells?

Secret Word (and Pierce Magic/Pierce Shield) cast 2 subspells, each with opcode 230 to cancel Spell Protections and Spell Shields respectively.  Dispelling Screen has the Spell Protection sectype, with nothing but a 146 effect casting K1#SCRE, which is the actual dispelling screen and has the K1#Dispel sectype.  I don't know if when opcode 230 removes the base DS spell it also removes its subspell... but I assume it does? 

If so, then this is working as intended.  If it was coded differently - if you added a third subspell with opcode 230 removing the K1#Dispell sectype - then there would be no need to have DS cast a subspell.  The base DS spell could simply use the K1#Dispel sectype.  Further, this would mean that SW/PM/PS could remove Spell Deflection and Dispelling Screen, which is not intended and would be undesired.  The current implementation - assuming it is working - will remove either SD or DS, but not both in one go.

 

Quote

- Greater Malison: The "take an extra point of spell damage for each die rolled" is implemented as a -1 penalty to luck and +1 to thac0 (??). This is clearly not correct, but what is the exact fix?

This is actually correct.  The -1 Luck penalty imposes a negative adjustment to to-hit rolls; if the intent of the spell is solely to increase spell damage, then it must be offset with a thac0 bonus.

...

I know I didn't have answers to all the questions; in some cases I basically restated them because I'd like to hear more discussion about some of them, if only to learn more about how some of these things work.

Edited January 6, 2020 by subtledoctor

[grodrigues]

1 hour ago, subtledoctor said:

Probably worth going through instances individually if you find them.  Can you give an example?

Just the ones I caught: Wraith Form, MGoI, Greater Malison. I do not have a preference either way, as in here I am just concerned with fixes. If I want to change the behavior I imlement it in my own copy of SR tweaks.

1 hour ago, subtledoctor said:

Easy-peasy, I think.

Heh, gonna have to read that description a second time...

1 hour ago, subtledoctor said:

I don't know much about projectiles, gonna have to punt here.

Oh me neither, I cannot even remember why I poked in there. But SRR makes them the same, and in all cases I have seen they are the same, and since I am already fixing things anyway...

1 hour ago, subtledoctor said:

I don't know if when opcode 230 removes the base DS spell it also removes its subspell... but I assume it does? 

Then this is the real question, because I assumed it did not. Glad I asked.

1 hour ago, subtledoctor said:

This is actually correct.  The -1 Luck penalty imposes a negative adjustment to to-hit rolls; if the intent of the spell is solely to increase spell damage, then it must be offset with a thac0 bonus.

Again, glad I asked.

Then I better ask another question, just to make sure I did not made a mistake: in Ice Storm I am altering the duration of the movement rate penalty opcode 24 -> 6 because it is my understanding that, given the projectile, the opcode is applied once a round. Am I wrong in my reasoning?

[subtledoctor]

Like I say, I really don't understand projectiles very well, but I wish I did.  I see that the Ice Storm .PRO has 4 repetitions, but I don't see the frequency/duration anywhere.  I see "Explosion Frequency" = 100... does that mean 6 seconds? (Is that 100 ticks?)  Or is the 1x/round frequency just the way projectile repetitions always work?  (...which would make some sense)

I'd be inclined to say that the Ice Storm effects should be reduced to 6-7 seconds.  Comparing to SR Grease: the Grease effects (movement reduction, trip and "sleep") are coded to last 7 seconds, and the Grease projectile has 10 repetitions.  (Same "Explosion Frequency" of 100, but the spell duration is 10 rounds so 10 repetitions results in the same frequency as Ice Storm's 4 repetitions in 4 rounds.)

Arguably, on the EEs both spells should have their effects set to 7 seconds and a 321 effect should be added at the top of the effect order, so the the effects are seamless and don't get doubled for those 1-second crossover periods.

[Mike1072]

I commented on Faerie Fire in your original thread.  This is a bug that I can fix.

[grodrigues]

Three more notes:

Checked the ranges of other shield-retaliating spells (Mestil's Acid Shield, Aura of Flaming Death, the remains of Cold Shield) and they all have range 6 so dropping the proposed fix.

Also checked other cloud spells that impose movement rate penalties (Grease, Stinking Cloud, Web). The duration is 7 -- why not 6 I do not know, since the opcodes fire once a round as many times as the repetitions in the projectile, and that is a question I submit to anyone reading this -- but at any rate the 24 in Ice Storm seems a clear mistake so the fix will be in, but I will use duration 7 instead of 6 to conform with the other area spells.

As far as scaling goes, the cases I caught (Wraith Form, MGoI, Greater Malison), all had to do with scaling duration not scaling of effects, so my inclination is to follow the docs. At any rate, I will leave that to a second batch of changes (the first should go in in a couple of days, mostly easily fixable low hanging fruit).

Edited January 7, 2020 by grodrigues
Movement Rate penalty

[kjeron]

2 hours ago, grodrigues said:

Also checked other cloud spells that impose movement rate penalties (Grease, Stinking Cloud, Web). The duration is 7 -- why not 6 I do not know, since the opcodes fire once a round as many times as the repetitions in the projectile, and that is a question I submit to anyone reading this -- but at any rate the 24 in Ice Storm seems a clear mistake so the fix will be in, but I will use duration 7 instead of 6 to conform with the other area spells.

6 seconds is 90 ticks, 7 seconds is 105 ticks, the projectile trigger every 100 ticks.

There is a complicated mix of (1 round = 6 seconds) and (1 round = 100 ticks) throughout the games, with enough of it hardcoded that it can't be modded to all work one way or the other.

On 1/6/2020 at 9:42 AM, grodrigues said:

- Spell Thrust: trigger and explosion radius of projectile dvsplthr.pro do not coincide. Fix? And what is the radius to use?

Trigger/Trap radius is the proximity check done by traps (Snare, Skull Trap) to trigger them, while they affect all creatures within the explosion radius.

The purpose of a smaller proximity is to lure more creatures into the effect area before it triggers.  For instantaneous, non-trap projectiles (Fireball), trap/trigger radius is ignored - the explosion automatically triggers.  For non-trap, non-instantaneous projectiles (grease, web), both trap/trigger and explosion radius must be the same: those within the explosion radius but not within the trap/trigger radius will be affected every "tick", rather than the projectiles stated frequency.

2 hours ago, grodrigues said:

Checked the ranges of other shield-retaliating spells (Mestil's Acid Shield, Aura of Flaming Death, the remains of Cold Shield) and they all have range 6 so dropping the proposed fix.

This mirrors the base game, which has the same inconsistency.  There are engine quirks related to the range of the subspells fired in this manner - they use an exact range check, while normal spellcasting adds up to 2ft to casting range.

[subtledoctor]

Malison and both GOI spells in my SR install all have a single extended header, with all effects lasting 120 seconds... this seems like the 'standard' SR 2-turn duration.  At a guess I'd say the description text should simply be changed to reflect this.

My 4b16 version of Wraithform actually has lots of headers, with duration of 2 rounds/level, matching the spell description.  I also don't think SR actually touched the spell... dunno if something changed in 4b17+ but if SR is simplifying it to a single 2-turn duration, I'd say the same there, it's probably just a case of needing to change the description.

[grodrigues]

On 1/7/2020 at 4:58 PM, subtledoctor said:

I also don't think SR actually touched the spell...

Yeah, SR does not touch Wraith Form so dropping it.

Two more questions before I submit the next PR. First, the spells Resist Fear and Protection from Petrification have Modify Script State [282] opcodes, in the first case for state find trap bonus, in the second for the state lock pick bonus. Maybe this is NI, but are these opcodes doing any work or are they spurious -- can be deleted? Second, the spell Sunscorch has two saves, a vs. breath to halve the damage, and a vs. spell to avoid blindness. If this is not a mistake, I suggest making it explicit in the description, e.g. "In addition to sustaining damage, victims who fail their saves are blinded for 1 round." -> "In addition to sustaining damage, victims who fail their saves vs. spell are blinded for 1 round.", which I will implement if everyone is ok with it.

Edited January 9, 2020 by grodrigues
Typos

[subtledoctor]

At a guess I would say those 282 effects are probably there for Detectable Spells.  I wouldn't touch them.  They almost certainly do no harm, at any rate.

[grodrigues]

Two more questions.

- Miscast Magic: the cast spell opcode has dispel/resist 1 and a save: just in case I am misreading the docs, this means that a successfull save or mr *will*  block the subsequent effects and that that is the intended effect.
- Vampiric Touch: has resist dispel 1 on the damage opcode and 3 on the healing opcode. Two options now: (1) if mr on the target blocks the healing opcode, all is fine and dandy (2) if not, then it could happen that the damage opcode is blocked by mr and the caster gets healed anyway. In this is a real option, then I suggest to change the resist/dispel of the damage opcode to 3 and mention that the spell bypasses mr in the docs. The same problem with Larloch's Drain.

 

Edited January 12, 2020 by grodrigues
Some rewriting for clarity

[Bartimaeus]

Miscast Magic: Seems intended. If you succeed the initial save, you completely avoid all rounds of wild magic - if you fail it, you're subjected to a save each ensuing round. A quirk here is that even if you make one of the ensuing saves, you will still have to make the others to keep avoiding it. If there is an error/oversight, it's probably there, but it's difficult to say for sure.

Ice Storm: As implied by kjeron, stationary spells like this should always have durations of 7 for lasting effects like Ice Storm's movement speed reduction. It is very slightly longer than appropriate, but it is better to last a tiny bit too long than a little too short, as characters will otherwise be unaffected by the spell for a moment before it reapplies (in this case, characters would start moving at normal speed for a moment).

Secret Word: I got rid of this part of the description because 1. it doesn't happen, and 2. it makes no sense for it to happen and therefore shouldn't. It doesn't make sense because Dispelling Screen does not protect anything Secret Word is attempting to dispel, and Secret Word itself is not in any way special vs. other higher level anti-magic spells that would suggest it should dispel Dispelling Screen where they don't.