SPELL.IDS coding discussion

[subtledoctor]

1 hour ago, CamDawg said:

Right, and they (plus a few other flags) were being used by SPC since 2004. I'm not against using limited resources but I will ask--as should anyone--why they're being used when there are easy, equivalent methodologies available.

Well, that's the $64,000 question in each case. (In that particular case, it's worth noting that I requested an extension to opcode 319 to make usability stuff easier about 5 years ago, and Beamdog never saw fit to do it. I also requested improvements to allow a better way to do at-will innate abilities, which would obviate the need to use an item slot like ammo3, but that didn't happen either. Sometimes easy equivalent methodologies don't exist.) (Also I have no idea what SPC is. )

Edited January 19, 2019 by subtledoctor

[DavidW]

1 hour ago, subtledoctor said:

Sometimes easy equivalent methodologies don't exist.

But in the case we're discussing (WIZARD_SHOUT and the like) they obviously do. (Indeed, I coded one for you, two pages up.)

[subtledoctor]

13 hours ago, DavidW said:

respect install order, but don't needlessly code in a way that forces one install order rather than another. 

Terms like that, and "easy equivalent," have very different values for different people.  This is the first iteration of a mod doing something that's never been done before, made by someone who has never done this kind of thing until very recently.  Allowing that 1) it worked; 2) there were no reported bugs; 3) given typical usage and past experience, there did not appear to be a high risk of conflicts (the only conflict that has arisen is with a mod that didn't exist at the time and whose existence was not readily foreseeable); and 4) given my skills, there was no apparent superior way to implement it that didn't involve major design trade-offs and the risk of bugs... I honestly don't think it was unreasonable.  Until SCSv32 fairly suddenly came into existence and knocked the legs out from under one of the assumptions underlying my mod's design, everything was going swimmingly.  Now there is a conflict; now that assumption has been exposed as poor, or at least no longer valid; now a superior methodology - equivalent, but not necessarily "easy" - has been identified.  So a second iteration can appear, improving on the first.  I just don't see anything wrong with that chain of events.  SCS has been through 32 different iterations (well, depending on how you count) and I'm sure with each one you have improved on the prior one in many ways, not least in the reduction of bugs and inter-mod conflicts.

13 hours ago, DavidW said:

I'm really quite perplexed to be having to have this conversation with you given you were actually part of the SR conversation about standardizing spell.ids entries!

a) See above.  b) The context was wildly different and currently inapposite.

But I'm not sure why we're having this conversation either.  I already said that a fix will be forthcoming.  I only posted those details to help identify and understand the interaction, and glean more information useful for fixing the issue - not to justify anything.  That was seemingly met with various instances of needless finger-wagging, which I don't think is justified.

[DavidW]

I quite appreciate that people’s skills vary and develop (though actually, the coding in M&G is pretty sophisticated on casual inspection, so don’t do yourself down). There’s no moral opprobrium in my criticism. But: at least until this last post you’ve been disagreeing with me as to whether it’s a good idea to put a WIZARD_SHOUT reference pointing to a spell that isn’t the wizard Shout spell, not just explaining how it came about.  So obviously I’m going to continue objecting, until and unless I’m persuaded I’m wrong or the person I’m discussing with changes their mind.

[DavidW]

Sorry, should also have replied to “the context [of standardizing spell.ids] was wildly different and currently inapposite”. It’s really not. The context of that conversation is making sure that spell.ids is consistent and accurate, so that one can work out whether a spell is already present in game and so determine whether to install it. That’s exactly the issue I’m concerned about here too. 

 

[CamDawg]

32 minutes ago, DavidW said:

There’s no moral opprobrium in my criticism.

Second. The community is better with more authors, and--speaking for myself--I'm too old and jaded to bother offering feedback for someone (or their project) unless I care about it and want to see them succeed. M&G set some lofty goals and does pretty well by them.

[subtledoctor]

On 1/19/2019 at 5:14 PM, DavidW said:

 “the context [of standardizing spell.ids] was wildly different and currently inapposite” - It’s really not. The context of that conversation is making sure that spell.ids is consistent and accurate, so that one can work out whether a spell is already present in game and so determine whether to install it. That’s exactly the issue I’m concerned about here too.

But that's the thing: the purpose of standardizing spell.ids is to make sure six different mods don't all install different versions of Mordenkainen's Force Missiles and give them all different IDS names, which would result in six different versions of the same spell, as well as six precious SPWI slots being taken up. What M&G installs into the game IS Shout, the wizard spell, identical in form and effect to the wizard spell in IWDEE.  It checks to make sure it hasn't already been installed, just like these other mods do.  Not calling it WIZARD_SHOUT creates a real danger of  the player seeing doubles of the spell in the game.  And again, the crucial assumption is that it would not cause IDS slot problems, because no mod existed at the time that would add spells that went after M&G in any recommended install order.

In fact I did install Shout as a wizard spell at first.  That is what it is meant to represent after all, and anyway early versions of the spontaneous casting system only worked with wizard spells.  (This component is an evolution of my multiclass sorcerers mod.)  Except, I didn't have code to add scrolls at hand, and right about the time I got this done, my free time veered into negative values, so learning something new was not an option.  The lack of scrolls, plus the danger that players might use one of the dumb megamods that take up all the SPWI slots, and the fact that the code uses innate clones of wizard spells anyway, led me to more or less say "screw it," and just shift Shout into the SPCL/SPIN space.  That was a separate decision made at a later time.

I don't mean to say it was an ideal decision, or even a good one; but there were no conflicts at the time, and I live in a world of triage.  Given the choice between letting it rot unused on my hard drive and uploading something imperfect and hoping I'll find the time to improve it in the future, I prefer the latter.  Especially because this is such a small part of a component that I think is really cool.  Managing to shoehorn into this engine a sorcerer-mimicking spontaneous casting system, usable by any class and with spell availability that can be defined on a kit-by-kit basis, via a noob-readable text file is, in its small way, kind of a big deal.  There are a few major improvements I still need to find time to make: changing the dialogue-based spell learning system over to use the sorcerer's UI, and figuring out how to implement sequencers and contingencies.  This conflict is much smaller potatoes - I've got a quick interim fix about ready to upload, and then I'll look into implementing your code using a variable in place of an IDS name.  You know - "when I can."

Edited January 22, 2019 by subtledoctor

[DavidW]

Most of this I'm fine with: I absolutely understand about time constraints and the value of actually releasing things rather than holding out for perfection, and I can see that adding the spell to the innate spellspace as WIZARD_SHOUT was a very fast way of fixing a problem in your mod. This is a limited response to the first point only, because I still think the general principles about spell.ids are worth discussing and trying to reach agreement on.

21 hours ago, subtledoctor said:

But that's the thing: the purpose of standardizing spell.ids is to make sure six different mods don't all install different versions of Mordenkainen's Force Missiles and give them all different IDS names, which would result in six different versions of the same spell, as well as six precious SPWI slots being taken up. What M&G installs into the game IS Shout, the wizard spell, identical in form and effect to the wizard spell in IWDEE.  It checks to make sure it hasn't already been installed, just like these other mods do.  Not calling it WIZARD_SHOUT creates a real danger of  the player seeing doubles of the spell in the game. 

This is still a place where we disagree. The spell is NOT the wizard spell Shout, any more than INNATE_CURE_LIGHT_WOUNDS is the cleric spell Cure Light Wounds. Specifically, it doesn't have an accompanying scroll, it isn't available to wizards on character creation, it isn't available to sorcerers on level-up, and it engages slightly differently with scripting. 

I don't think calling it something else (BARD_SHOUT, or perhaps better - since I think you just clone it and don't use the original spell - SUBTLEDOCTOR_PLACEHOLDER) runs any risk of a player seeing duplicates. At least, I can't think of a way. If another mod were to come along and look for WIZARD_SHOUT, it would *correctly* conclude that the spell was absent, and proceed accordingly; if it added the spell, that would be fine, and wouldn't lead to duplicates since the innate version of shout is only made available via your bespoke bard system. (But I may be missing something; I'd be interested in what scenario you had in mind.)

On the other hand, adding it creates the opposite risk: a mod erroneously concluding that the wizard version of Shout is present when it isn't (as in fact happened with SCS). 

The general principles I'm arguing for here are:

1) for spell.ids in particular: the ids file should tell the truth, i.e. should unambiguously identify which spells are actually present, and the identification should be accurate as regards spell type (INNATE/CLERIC/WIZARD/[CLASS]) as well as spell payload.

2) for mods more generally, ideally install order should be required, and relied upon, only when the later mod actually needs to dynamically respond to changes made by the earlier mod. Even that doesn't guarantee an unambiguous install order (which should be last, SCS or Tweaks? Each dynamically alters some things the other generates). But it minimizes the chance of inconsistent install order requirements.

But of course time limitations often get in the way of the ideal, and that's entirely understandable. No code is perfect (heaven knows mine isn't).

[subtledoctor]

Like I say, when I first made the component, it was in fact the wizard spell:

ADD_SPELL ~override/d5shout.spl~ 2 3 WIZARD_SHOUT 

Then, after some consideration I saw some chance of conflict, so I made a quick change:

ADD_SPELL ~override/d5shout.spl~ 4 3 WIZARD_SHOUT 

I had coded it in accordance with your general principles, which I agree with; only later, after I had considered those principles and then forgot to continue considering them, did I change it.  And even then, there were no observable problems, and it seemed unreasonable that someone would come along and add the spell after M&G in the install order. 

There is a problem with adding the same spell in an innate spot that may existed, or later come to exist, in a wizard slot: M&G bards are supposed to be able to cast that spell, and now must look for both to see if they exist and can be used.  So there is indeed a chance to see duplicates in-game.  I don't think that will actually happen - I think the code adequately prevents it - but it is introduced as a possibility, where it was not a possibility before.  At a gut level, I don't like the idea of that.  Taking your principles into account, the ideal solution is in fact to revert to adding the spell as a wizard spell:

ADD_SPELL ~override/d5shout.spl~ 2 3 WIZARD_SHOUT 

The problem there is, again, no scrolls and no time to learn how to place a bunch of scrolls in the game in a satisfactory manner.  And I think you would be kind of annoyed if you saw that SCS was skipping that spell due to a mod which didn't add it in all the ways that you would want it added.

Stepping back (because that particular issue is already fixed): install order is still an issue here.  Installing [M&G then SCSv32] will result in a different and degraded experience, vs. installing [IWDification then M&G].  M&G tries to make several IWD spells available to spontaneous-casting bards, and it cannot do that if those spells are added later.  I haven't looked at other mod interactions, but any mod that needs to have a look at what spells exist in the game will get an incomplete answer if they are installed before SCSv32.  (And just about every mod is installed before SCS.)  Off the top of my head, Tome & Blood will have problems as well.

As SCS does more things that are traditionally done early in the install order, I wonder if the conventional wisdom that it should be installed last is out of date.  E.g., do kit packs really need to be installed before SCS?  Or rule tweaks such as Tweaks Anthology?

[DavidW]

I'll leave the original issue aside - I'm not convinced (I still think just not putting it in the spell.ids namespace is the best solution, leaving aside coding and time costs, and I can't see how the duplication scenario is supposed to work) but as you say, it's moot now, since SCS now detects and removes spell.ids entries in the 'wrong' bit of namespace.

1 hour ago, subtledoctor said:

Stepping back (because that particular issue is already fixed): install order is still an issue here.  Installing [M&G then SCSv32] will result in a different and degraded experience, vs. installing [IWDification then M&G].  M&G tries to make several IWD spells available to spontaneous-casting bards, and it cannot do that if those spells are added later.  I haven't looked at other mod interactions, but any mod that needs to have a look at what spells exist in the game will get an incomplete answer if they are installed before SCSv32.  (And just about every mod is installed before SCS.)  Off the top of my head, Tome & Blood will have problems as well.

As SCS does more things that are traditionally done early in the install order, I wonder if the conventional wisdom that it should be installed last is out of date.  E.g., do kit packs really need to be installed before SCS?  Or rule tweaks such as Tweaks Anthology?

I don't think there's a problem exactly, it's just that your mods require (or at least, are better with) IWD spells, so they need to be installed after a copy of the IWD spell system. As you say, that mitigates in favor of people using your mods being advised to use IWDification first. That's not really a problem for me. (There are small ways in which I think the SCS version of the IWD spellsystem is better - at least for SCS - than the IWDification one, but it's not a big deal. In the long run I'd like to try to get the IWDification and SCS versions unified in content, but that requires liaising with Cam and is anyway not a priority.) 

As for install order, though: the problem is that SCS modifies, or responds to, a *lot* of files. Non-exhaustive examples:

1) I mark up every weapon in the game to detect its enchantment level. (Not necessary for EE, but still crucial on non-EE installs.)

2) I put a spellstate on every item that provides any protective effect.

3) I add an effect to every spell and item in the game that uses any of the Charm, Hold, Slay, Panic, Poison, Sleep, Slow, Petrify, Confusion, or Level Drain opcodes.  Actually, on checking, I only modify spells from a fixed list.

4) I add a global effect to every enemy-usable spell in the game.

5) I read in the current proficiency system and use it to set NPC proficiency choices

6) I read in the current spell allocation system and use it to decide how many spells casters get (so that, e.g., NPC bards get 7th level spells if the player has installed a mod to that effect)

Some of these are admittedly minor (5 and 6 are not that visible in-game). But some of them potentially have very noticeable effects on enemy targeting.

I don't want to say that every kit mod in the world does something that means it has to go before SCS, but you can probably see why I tend to give that blanket advice.

[Ardanis]

I'm not very familiar with what Subtle's mods really do or how they're coded, but my understanding is that if M&G wants to make innate copies of wizard spells, and if it doesn't detect Shout in the game then it simply adds it directly into innate slot. And if another mod later adds Shout to wizard slot, then M&G bards will be stuck with their own version, as it won't be updated by ADD_SPELL. I can see how it might become an issue, but I would just skip Shout altogether if the mod can't detect it. Bards are, after all, inferior casters compared to wizards - even if some editions grant them spells unavailable to any other class.

[subtledoctor]

On 1/24/2019 at 10:23 AM, Ardanis said:

 but I would just skip Shout altogether if the mod can't detect it. Bards are, after all, inferior casters compared to wizards - even if some editions grant them spells unavailable to any other class.

The mod already restricts bards to a small set of spells, and because they work like sorcerers, the player can only learn an even smaller subset of those.  If anything, in all likelihood I'll do the opposite: remove any checks for IWD spells, and add my version of Shout and Great Shout no matter what.  The downside here is that the resulting spell could be different from the equivalent wizard spell, depending on how the player adds the wizard spell.  But those spells are particularly apropos for this variant of bards, so I want them there no matter what, and in the most reliable fashion possible.

Edited January 25, 2019 by subtledoctor

[DavidW]

The other thing you could do is name your version BARD_SHOUT or similar, and copy WIZARD_SHOUT into that slot (and innate-ify it) if it’s already present.

[Luke]

On 1/18/2019 at 11:16 PM, DavidW said:

IWDification explicitly checks if the spell is there, and ADDs it only if it's not there. It has lines like this before every spell:

ACTION_IF !(FILE_CONTAINS_EVALUATED (~SPELL.ids~ ~[ %TAB%]WIZARD_SNILLOCS_SNOWBALL_SWARM[ %TAB%%WNL%%LNL%%MNL%$]~) OR override_arcane)	BEGIN

SCS has something similar (though the code is a bit more abstracted).

Does this mean that everyone is free to ADD his/her own version of the Monster Summoning spells and similar (e.g., WIZARD_SHADES, etc.)? I mean, your AI uses these spells regardless of what creatures are summoned, so it's safe, right?

Edited July 2, 2019 by Luke

[DavidW]

If you write your own version, SCS will avoid replacing it with its own. But (i) the AI will only use IWD spells if it detects that they’re all there, which currently means it needs either IWDification or SCS IWD spells; (ii) SCS doesn’t know what your spell does, so if it’s too different, it may do something stupid.

For Shades in particular, it doesn’t matter: I don’t use that spell.