DavidW

I've had a more careful look at this, and it's better than I had thought (the EE implementation differs significantly from the oBG2 implementation). Here's how Crushing Hand works (Clenched Fist is similar). If you *make* your saving throw, you're hit by a 1-round, no-save Sleep (39) effect. if you *fail* your saving throw, a secondary spell hits you with a 2-round no-save sleep effect *that bypasses Sleep immunity* (via opcode 337) and inflicts various bits of secondary damage. After one round you're hit with another secondary spell which, if you make a saving throw, removes the effects of the first secondary spell. Observations: (1) that is a seriously messy way to implement the spell. (2) Still, there is a fairly clear (Beamdog) developer intent. If you save immediately, you can bypass the no-save Hold effect if you're sleep-immune. If you fail the save, the hold locks you in place even if you're sleep-immune, which nicely addresses my aesthetic concerns. (3) It is *slightly* dangerous to bypass immunity to an incapacitating power. If you use Bigby on, say, Abazigal, you'll freeze him in place for up to 18 seconds, during all of which he might be indestructible at 1 hit point. Still, it's probably not unacceptably dangerous, because it has a fixed duration (and there is something cool about finishing the battle with Abazigal by crushing him into a pulp and finally releasing him to gasp out his death speech. (4) Having tested it, the hand icon is not unacceptably unaesthetic on dragons. It's not terrific, but it's okay. So 'm now inclined to say that the Hand spells are fine, except (as noted elsewhere) we should move the opcode from 32 to 185. Implosion dumps instant damage on the target and then plays the chasm animation over them; dragons indeed walk out of the animation, and that does indeed look a bit silly (though, to be fair, not as silly as I thought it might). I am inclined to pursue a purely cosmetic solution here: give 185-immune creatures immunity to the Implosion animations. Then the spell will just shake the screen and dump some fiery/crushing damage on the creature, source unspecified - leaves the balance unchanged but sorts out the aesthetics.

My assumption is that in each case someone wrote the verbal description of the item, put it in the tlk file so that translators could get to work on it, and then sent it to a technical designer to implement that verbal description; since BG2 probably wasn't keeping better track of what 'death magic' meant than we are, different technical designers did it differently. (And in the case of Avoid Death, they were also working under the much-accelerated QA deadline that comes with an expansion.) On that basis my preference is: (1) standardize on a single use of 'death magic' over all those items: different implementations isn't really evidence of different developer intent. (2) specifically standardize on Death Ward: immunity to things that kill (not merely incapacitate) the target instantly, i.e. attacks associated with opcodes 13,55,209,238. That's because (a) I think it's reasonable to think that Death Ward, a core spell, is the most careful implementation of what the designers had in mind by 'death magic'; (b) it fits the AD&D definition of 'death magic', which is the most likely thing the developers had in mind; (c) death magic is the only one of the items that explicitly explains what 'death magic' is in its description, and it defines it that way; (d) I think it's the natural interpretation (death magic is magic that directly kills you, not just removes you from battle in a way reversible without resurrection.

One observation: 'death magic' is a technical term in 2nd edition AD&D: there is a 'save vs. death magic' after all. The 2nd edition Player's handbook (p.101) defines death magic as 'certain spells and magical items that otherwise kill the character outright'. That suggests that anything that uses opcodes 13, 55, 209, 238 counts, but petrification and imprisonment don't. (I'm not in favor of using PnP as a resource to decide first-order questions, but it's useful sometimes to work out what developers may have had in mind.)

Pushing the analysis further: ToTSC adds 2 abilities that use 175: spin999 (Tanar'ri paralyze) and spin999 (tanar'ri death gaze). That all seems to strengthen the idea that 175 is associated with magical paralysis; that said, it's really not dispositive because they're enemy-only powers usable only in one situation, where the 109/175 distinction isn't that visible. And there's room to debate whether the Tanar'ri paralyze should really be a 109 (especially as it is not IDS-gated - is it really supposed to work on undead?) This matters more when it gets imported into BG2, of course. BG2 adds the following (while keeping all the BG1 content): Using 185: Hold Undead (SPCL742,SPWI324). As I say elsewhere, I think it's a design error to use 185 rather than an IDS-gated 175 here. SPIN708, TRAP_SLIME. Logical: having a mass of corrosive slime dumped on you probably isn't resistable by free action. SPIN769, HELL_HOLD. First use of 185 as a cutscene unblockable power. SPWM114, probably an error (FP fixes). Using 175: - Beholders, weirdly, have both a Hold Person (SPIN988) and a Paralyze (SPIN990) eyestalk, but both use 175. (I actually think there is some reason to think the Hold Person version is an earlier implementation that should be dropped in favor of the more-standardly-used Paralyze ray, but it doesn't matter too much here.) - Bigby's Clenched Fist and Bigby's Crushing Hand use 175; as I note elsewhere, it should probably be 185. Using 109: - several more paralyzing creatures use 109: ghoul lords, maurezhi demons, lacedons, liches - several more variants of the web effect also have 109 backstopping 157 - a cutscene-used effect, HOLD_PARTY, uses 109 but should probably be moved to 185 - implosion uses 175, and again should probably be 185 - sphere of chaos inflicts 109 (and is described as 'paralyzes' in the spell text) - DAGG17 uses 109, with no IDS gate, despite being specifically called out in text as applying a Hold Person effect; I think that might be worth shifting to a Humanoid-gated 175. - RODSWORD, the sword created by the Rod of Lordly Might, uses 109 even though its descriptive text says 'held', not 'paralyzed'. I think this continues to point to a reasonably clear distinction between 190, 175 and 185, against which bugs can be reasonably easily spotted. For the same reason, the changes I'm suggesting have relatively minor consequences: they're as much about clearing up messy code as anything else. The ones I've spotted are Many fewer creatures (notably including undead) are immune to the movement-blocking effects of Implosion and Bigby Undead can use Free Action spells to avoid Hold Undead Wyverns, Invisible Stalkers, Kuo-Toa, and certain jellies are affectable by Hold Monster The Rod of Lordly Might holds more creatures I'm sure there'll be others, but they will be similarly edge-case. (The fact that undead are immune to most Holds has a big impact here.)

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

Huh. That's quite annoying, actually. It's not implemented afaict, and it's not in PnP... still, it's hard to argue against developer intent when there's a new line explicitly added to the spell. Agreed, we'd better enact that. 324 immunity to undead is probably the simplest way.

It might help to list explicitly how original BG1 (before even TotSC) does this, since it's pretty clean and simple: 185 is used only by Otiluke, and nothing is protected. 175 is used by Hold Person (wizard, priest, and trap forms), Hold Animal, and Hold Monster. The Greenstone Amulet, the Chaotic Commands spell, the Free Action spell, and the Ring of Free action protect against it. 109 is used by (1) paralyzing attacks: ghouls, ghasts, carrion crawlers, vampiric wolves, Kaldran the bear. It's accompanied by the 'held' icon but not by any string. (In particular, 109 attacks never say 'Held'.) (2) The Web spell (157 is treated as cosmetic). (3) basilisks, for technical reasons that I don't think are relevant. Anything immune to 175 is immune to 109, as are undead, wyverns, spiders, certain mages, and some scripted creatures. (The potion of free action protects against 109 but not 175; surely a bug.) I think the logic of this is pretty transparent: - 185 is being used as a bespoke hold-in-place. - 175 is specifically the Hold spell's payload, and nothing is innately immune to it - you get immune by using a spell or magic item. Creature immunity is entirely handled by the restrictions in Hold: to 'persons' (=humanoids), animals, or no restriction at all. - 109 is mostly paralysis, as SD says. The player can't really generate this at all except through summons, so creature immunity to 109 is basically there just to guard against undead-on-undead edge cases (and not to generate the sweeping immunities that are granted by the time we get to the EEs). - BUT to complicate things, Web doubles up on 109, and that means that things that need web immunity (wyverns, spiders, and, I'm fairly sure, certain prebuffed mages) end up getting paralyzation immunity. BUT that doesn't matter too much in original BG1 because, as I say, 109 immunity is something of an edge case. Again, by the EEs this has ramified enormously. Even by the time we reach ToTSC this is getting a bit confused, incidentally - Aec'letec's powers aren't a completely clean fit.

Pretty much, I think, except I think it's better to think of 185 as doing double duty as (1) an unblockable CS hold, and (2) miscellaneous effects (being stuck in a chasm, being trapped in a bubble of force, being held onto by a disembodied hand) that block action but aren't formally 'Hold' and so aren't blocked by Hold immunity.

I'm (rather belatedly) closing this thread because I don't want one single thread that gets reopened any time someone has a new set of critical comments on SCS: it confuses people and subsequent posters lose track of who's being replied to. I'm very happy to have criticism of SCS, and it doesn't have to be constructive ('I hate this mod because it does XYZ' is fine.) Anyone who wants to say something critical, by all means start a new thread to do so.

I'm largely repeating my initial post, but I think if you look at original BG1 you can see a quite clear distinction in how the opcodes are used and immunized against: - 185 is used *only* by Otiluke, and nothing is immune to it - 175 is used by Hold spells, and player-usable Free Action-type effects protect you from it; no creature is innately immune to 175, immunities by creature type are handled by IDS targeting inside the 175 opcode. - 109 is used by other paralyzing effects, mostly creature touch attacks. It's not associated with the string 'Held'. That architecture then gets successively confused through SoA, ToB, and the EEs. I think it is reasonably clear in-game that the level designers were just losing track themselves of which opcode did which (I think by now the development environment for modding the IE games is quite substantially better than the actual environment Bioware had back in the day). By the EEs, where the 109/175 distinction is almost entirely elided, we have in-house testimony (Cam) that the distinction had been lost. But I think you can see its bones clearly enough even in BG2 that it's worth using it as a baseline for a FP. I'm partway through code that tries to spell this out more explicitly.

It's only worth doing once the EEFP is established as a standard (or when it's incorporated in 2.7). So don't spend time doing it now. It's trivial enough to add in a later release.

I’m not sure why petrification kills while imprisonment doesn’t. In-game, both trap you in an indefinite immobilized limbo without killing you. Mechanically, both kick you out of the party and require a specific spell to restore you.

I agree with this, and I’d stress the “otherwise animations look stupid” line, which is fairly clearly a design principle in BG2. (In BG1 nothing is really big enough for it to matter.)

Too large a step for me, and I don't see why a magical chasm can't magically pull in its target. I think my general don't-change-the-architecture point applies in this case.

And I thought I was paranoid about encapsulation...

It's not a typo: I use .tpc for live code, i.e. any inclusion that's not a function library. I don't think it makes a material difference to speed.

There is a question of developer intent... I think those are inherently sensible moves, but I don't know what Cam's comfort zone is like here!

OK, some persuasive points here. Thoughts: - Agreed that matching BD setup is a priority. - presumably, insofar as using IDS names for spell patching is concerned, if that's a problem for BD then using the extended EFFECT library is also a problem? (But if it's tolerable in both cases, great.) - That's clearly right about ACTION_BASH_FOR; I stand corrected. We clearly need a standardized order, in which case we may as well make it explicit. - Perhaps this is a different conversation, but I am much more skeptical than you about supporting people making their own changes, for several reasons: (i) it encourages people to think they can edit FP and then still install mods that require FP without problems, which is not at all true. (You might recall this was the same reason I didn't want IWDification to make it easy to pick and choose which spells to include.) (ii) Precisely because fixes can rely on other fixes, it can easily have unintended consequences even internal to FP. My feeling is that the gap between being competent enough to comment out bits of WEIDU code to get the right consequence, and being competent enough to write your own code (or use NI) to get the same effect, is not very large. - I'm not sure I'm persuaded by search. (i) It's not as if it's difficult to use a tool that searches a whole mod at once. (I use Windows Grep.) (ii) Searching by file name is anyway not a reliable way to find a fix given that we might be doing a REGEXP (unless you actively want to have a strictly-no-REGEXPs policy to facilitate this, which I think is more drastic than your last paragraph implies. (And of course even FP doesn't rigidly keep everything in one file, as I found last night when I missed the 109/175 libraries.) - That said, I don't want to be a pain about modular architecture, especially since realistically you'll be doing the bulk of coding and these are partly aesthetic calls. If you think it is better to put everything in a single megafile, that's fine. (I'd still prefer that megafile to be its own included file rather than put it directly into the tp2, but that too is partly aesthetic and not a big deal.) - I mostly agree with your last paragraph; having said that, there are sometimes advantages for readability, debugging, and future-patch-proofing to have the code be a little more explicit in what it's supposed to be doing. 'Patch everything that contains the string 'CDHELD' is easier to read and to check than 'patch the following 229 files [which the author of this code knows, but I don't, to be in fact the 229 files that contain the string 'CDHELD]'. (I mean: the extreme version of this would let us blindly patch entries in an extended header by raw hex address, since we know the header; I take it you wouldn't want to go that far!) I have draft code that (mostly, not quite done) implements the shifts of the immunity system we've been discussing (just to see what it looks like) and that's an example of something I think would be intolerably and unreadably complicated if done without regexp. - Yes, obviously we shouldn't use SFO. (I don't even use it in Ascension).

I think this is correct, on reflection. The aesthetics isn't a sufficient argument, at least in a fixpack.

You need to read the string out of the clastext file, then set it. Something like this: COPY_EXISTING - "clastext.2da" nowhere // we're read-only, no need to write the file COUNT_2DA_COLS colcount READ_2DA_ENTRIES_NOW clastext_data colcount FOR (row=0;row<clastext_data;++row) BEGIN READ_2DA_ENTRY_FORMER clastext_data row 0 kitname PATCH_IF "%kitname%" STR_EQ "DRAGON_DISCIPLE" BEGIN READ_2DA_ENTRY_FORMER clastext_data row 3 lower READ_2DA_ENTRY_FORMER clastext_data row 5 mixed row=clastext_data // done now, no point keeping going END END STRING_SET lower ~dragon disciple (fire)~ STRING_SET mixed ~Dragon Disciple (Fire)~

On reflection, a more lightweight option would be to put a movement rate=0 clone of the 185 on these effects. It won't do anything for creatures affected by 185, and it will freeze the 185-immune creatures in place, which is about the best we can do to simulate the effect of the spell given that bosses need immunity to 185 to avoid scripting problems. Most boss protections don't give immunity to movement speed changes. It still leaves the animation silly, but I'm sympathetic to the view that that's too much for a fixpack. I'll do it myself in SCS or something.

Fair enough. & yes, that's right I guess (again, we're mostly talking about bosses with scripting - hardly anything should be immune to 185).

I think actually this is okay, simply because basically nothing should have 185 immunity in the first place. The only exceptions, pretty much, are scripted bosses where being paralyzed will break a scripted interrupt, but they need to block 175 and 109 for the same reason. The original (un-FP) game pretty much matches that, I think - dragons are an exception, but I think they're 109/175 immune anyway (and I'm not actually sure they should be an exception in any case). ...which I didn't. Oops. Fixpack's architecture is more complex than I gave it credit for. Ignore some of what I say about things that are in EE but not FP - sorry.

I guess the right time to talk about the architecture of the code is at the beginning. I don't want to get too picky about this, but some suggestions: (1) Put each discrete fix or closely-associated group of fixes in its own file, with a memorable name, e.g. "some_spells_should_use_opcode_185.tpc" (2) Put each file in a directory called 'fixes' (we can also have a 'resources' directory for game files, a 'lib' directory for function libraries, etc). Then the tp2 component just needs to be ACTION_BASH_FOR "%MOD_FOLDER%/fixes" ".*\.tpc" BEGIN WITH_SCOPE BEGIN INCLUDE "%BASH_FOR_FILESPEC%" END END I think that architecture is fairly easy for people to contribute to, is properly encapsulated, doesn't end up with people working on the same file unnecessarily, and is easier to test and read. Given modern WEIDU I'd strongly prefer not to just have an enormous list of patches directly in the tp2 (no criticism of fixpack intended, WEIDU has evolved out of recognition since its architecture was developed). You could separate the fixes section into BGEE, BG2EE, IWDEE, PSTEE, shared if you wanted (and indeed could pretty easily rearrange like that later in the project - that sort of change is also easier if every fix is in its own file). (3) Use AUTO_EVAL_STRINGS. It makes for much cleaner and more readable code, but there are very rarely incompatibility issues with code not written under AUTO_EVAL_STRINGS and so it makes sense to have a definite decision to use it. (Or a definite decision not to use it, but in that case I probably won't contribute code because I'm very used to it by now.) (4) Follow SFO's protocol of defining every spell.ids entry in the preamble as a variable, i.e. WIZARD_FIREBALL is spwi304. I think it makes code much easier to read and catches a lot of bugs. (And no-one actually has to use it!) (5) (Possibly) use my slightly-extended versions of your *_EFFECT libraries (in, e.g., SFO) which define a bunch of bit variables and, probably more usefully, let you match on a function and patch with a function. (6) Make the whole fixpack depend on a very large number of libraries which are all mutually dependent and which contain hundreds of idiosyncratic functions that are largely undocumented. I'm willing to compromise on (6).

Read in the strings from clastext.2da