The General AI component is too delicate

[subtledoctor]

The most recent example: my mod adds the Totemic Druid's Spirit Wolf summon spell to Beastmasters. Problem is, that file (spcl625.spl) doesn't exist in the old TOB engine, it was apparently added by the EE devs. So clabrn04.2da has a GA_xxxxx line pointing to a spell that doesn't exist.

 

Technically it's an error, but the effect on the game is nil. It doesn't matter, doesn't cause any problems. When SCS sees it, however, it throws up its hands and refuses to install. So it's worth asking what SCS does with this ability when it successfully installs on BGEE. My understanding is,

1) SCS patches the ability into all AI Beastmasters in the game - of which there are none.

2) SCS patches the ability into any non-joinable NPC Beastmasters added by mods - of which there are none.

3) Having given the ability to the (non-existent) Beastmasters, as far as I can tell SCS does not actually cause them to *use* the ability.

 

So SCS is doing precisely nothing here, but still finds reason to throw a tantrum when it isn't able to do it. Why??? Can there seriously not be some kind of "ACTION_IF FILE_EXISTS_IN_GAME" check before running this routine? Right now SCS is creating problems where none exist and it is very frustrating.

[Roxanne]

The most recent example: my mod adds the Totemic Druid's Spirit Wolf summon spell to Beastmasters. Problem is, that file (spcl625.spl) doesn't exist in the old TOB engine, it was apparently added by the EE devs. So clabrn04.2da has a GA_xxxxx line pointing to a spell that doesn't exist.

 

Technically it's an error, but the effect on the game is nil. It doesn't matter, doesn't cause any problems. When SCS sees it, however, it throws up its hands and refuses to install. So it's worth asking what SCS does with this ability when it successfully installs on BGEE. My understanding is,

1) SCS patches the ability into all AI Beastmasters in the game - of which there are none.

2) SCS patches the ability into any non-joinable NPC Beastmasters added by mods - of which there are none.

3) Having given the ability to the (non-existent) Beastmasters, as far as I can tell SCS does not actually cause them to *use* the ability.

 

So SCS is doing precisely nothing here, but still finds reason to throw a tantrum when it isn't able to do it. Why??? Can there seriously not be some kind of "ACTION_IF FILE_EXISTS_IN_GAME" check before running this routine? Right now SCS is creating problems where none exist and it is very frustrating.

Unnecesary checks in SCS are an issue for other problems as well.

The main component is required for all others, even for components that clearly do not need it (i.e. components that just add contents via dialogues, e.g. send NPCs to an inn). Having former SCS1 and 2 in one monolithic block has not improved things (neither for players nor for other modders).

Trying to install it in sensible batches in BWP/BWS does not work, as all compnents require the main one.

[Jarno Mikkola]

You both are free to submit a .tp2 code based tweaks. Say in this very thread. Someone with enough power will usually come and take the fixes and release another version. Say like CamDawn, if there's enough to warrant one for the hassle.

 

1)2)3)

1. Well, you might not have seen it, but I would remember that there is one in the Copper Coronet if you install the SCS. Or I could be wrong...

2. Well, read the 3, and apply it to non Beastmasters.

3. Yeah, the BM doesn't... but lets take a predefined kit that has a reference, say Berserker, all the monsters the game has marked as Berserker via the kit or in this specific case the creature name(the Ogre Berserker for example) get the upgraded with the ability to have, can you guess what, to berserk of course. And their script will of course use that, cause it's easy to do so. Or relatively so.

[subtledoctor]

There is a guy in the Copper Coronet *named* "the beast master" because he has animals in cages... but he is not an actual

Beastmaster ranger, at best he is Fallen and I think he uses equipment that is illegal for Beastmasters to use. Long story short, no.

 

If this mod is actually abandoned then yes it might be appropriate for someone to submit code and have the G3 folks include it in the mod. (Though, Camdawg hasn't been around in a long while, I'be submitted code for other mods, including Camdawg's own, and it has not been incorporated.)

 

Insofar as DavidW still owns and maintains this, or someone else like MadMate is updating it, my code suggestion is right there in the post above: in whatever loops the AI component is adding abilities to NPCs and their scripts, simply add an ACTION_IF FILE_EXISTS_IN_GAME or similar check to look for the existence of the file, before running the subroutine. Problem solved.

[Mike1072]

Sounds like the problem could be solved just as easily if you checked that the spell existed before putting it into the CLAB.

 

I don't think it's unreasonable for SCS to assume that all of the spells listed in a CLAB should exist in the game.

[Jarno Mikkola]

Though, Camdawg hasn't been around in a long while...

Well, look at here. CamDawn might not have been in here, but people do things still. If they are easy. Or they 'feel' like it. Yes, peoples internal reasoning is mysterious. Why he submitted that code ? No freaking idea, probably because it would help at something.

 

my code suggestion is right there in the post above: in whatever loops

That's not a proper code suggestion, it's a suggestion to add in code that... instead of change line xyz to include code, like these 2 for example. That's a proper code suggestion. One that can be easily applied and then tested by a n00b. And thus seen if it work on general. Yes, the trick is to make the code too easy to not to apply. Yep, that's the only code for SCS that the BWS has. A redefinition of a WRITE_ASCII to be WRITE_ASCIIE in one of the lines.

 

I don't think it's unreasonable for SCS to assume that all of the spells listed in a CLAB should exist in the game.

Yeah, and I don't think it's unreasonable to be carried around by a personal giant.

But, in practicality, improving the SCS'es behavior is desirable. Now of course I am not a good enough coder to submit a code suggestion, cause I don't know where the improvement needs to be made. Or how to make it, and probably never will be.

[CrevsDaak]

You're not only bugging SCS, but adding unnecessary clutter (unless 1. you copy the spellor 2. the spell exists) to the CLAB file, and, bugging other mods that assume this. It's easier for you to fix it on your end, be it by not patching the CLAB if the spell does not exist, or by adding the spell to the game.

So, even if it is fixed in SCS's end, it'll be nice if you fix it in yours too

 

On the other side, improving giant mods prone to failure due to small errors like this as SCS is also nice, so here's the .diff to change this:

--- lib_macro.tpa	2015-12-17 03:33:34.000000000 -0300
+++ crevs.daak_lib_macro.tpa	2015-12-17 03:32:44.000000000 -0300
@@ -335,7 +335,7 @@
                      DELETE_BYTES 0x0 3
                   END
                   TO_UPPER resource
-                  PATCH_IF !VARIABLE_IS_SET "RESREF_%resource%_LEVEL" BEGIN
+                  PATCH_IF !VARIABLE_IS_SET "RESREF_%resource%_LEVEL" && FILE_EXISTS_IN_GAME "%resource%.spl" BEGIN
                       INNER_ACTION BEGIN
                          COPY_EXISTING "%resource%.spl" "%workspace%"
                                LPF SPL_read_level RET "RESREF_%resource%_LEVEL"=value END
@@ -349,7 +349,7 @@
                      DELETE_BYTES 0x0 3
                   END
                   TO_UPPER resource
-                  PATCH_IF !VARIABLE_IS_SET "RESREF_%resource%_KIT_EFFECTS_ADD" BEGIN
+                  PATCH_IF !VARIABLE_IS_SET "RESREF_%resource%_KIT_EFFECTS_ADD" && FILE_EXISTS_IN_GAME "%resource%.spl" BEGIN
                       INNER_ACTION BEGIN
                         ACTION_IF FILE_EXISTS_IN_GAME "%resource%.spl" BEGIN
                          COPY_EXISTING "%resource%.spl" "%workspace%"

The file that needs this patch applied is stratagems/sfo/general/lib_macro.tpa

 

Edit: My patch doesn't warn the user that the file doesn't exist. I could add that if needed, but right now I'm off to sleep.

Now of course I am not a good enough coder to submit a code suggestion, cause I don't know where the improvement needs to be made. Or how to make it, and probably never will be.

Don't worry, that happens to everyone, SCS's too intricate... I spent a good while searching, until I realised it was into SFO itself, and since I knew it was inside a macro (from my previous findings in stratagems/genai/genai.tpa), I checked in lib_macro.tpa and got it. Dunno if you're getting notifications form the edit quote but well... Why do I care about that when it's almost 4am? Oh well I'm sleeping already.

[subtledoctor]

Sounds like the problem could be solved just as easily...

It's easier for you to fix it on your end, be it by not patching the CLAB if the spell does not exist, or by adding the spell to the game.

It's not a question of what's easier, I'm just pointing out that SCS tends to make very small bugs into big installation-failure bugs, and in this case there is no benefit to offset or justify doing so.

 

On the other side, improving giant mods prone to failure due to small errors like this as SCS is also nice

What I'm saying.

 

So, even if it is fixed in SCS's end, it'll be nice if you fix it in yours too

Already done.

 

Edit: My patch doesn't warn the user that the file doesn't exist.

I don't think it needs to - it's not the responsibility of SCS to do so. Users who cannot use an advertised kit ability can report the bug to the responsible mod author. (Instead of what usually happens, which is complaints on the SHS BWS forum that "SCS won't install and I don't know why!") And the functional outcome with your patch should simply be both the player and AI missing the ability... a desirable parity IMHO.

[Mike1072]

There are benefits to making assumptions like "every spell listed in a CLAB should exist in the game".

 

- You don't waste your time and effort trying to handle situations that should never exist

- If the installation ever fails, you've discovered a bug in some other mod

 

If SCS hadn't failed on this file, you might never have realized that your mod was not behaving as you expected on non-EE games.

[subtledoctor]

- People using that component and playing Beastmasters might have noticed that they do not have an ability advertised in the readme, and they would have reported it. In which case it gets quickly fixed.

- Or, they might not have read the readme and might not have noticed the missing ability. In which case, if the bug is so minor that nobody notices, then it's not the end of the world.

- Or, people might never play Beastmasters. In which case the bug never even manifests.

 

None of those are particularly horrible outcomes. Conversely, currently SCS just refuses to install. Most players don't know how Weidu works, they don't use NI or DLTCEP, they may not know what a CLAB file is or why it might cause problems. They might complain in the SCS forum or the BWS forums, or might just give up and not use SCS. Most players don't actually post on forums, and therefore don't report errors. IMHO the current behavior of SCS is far more harmful than the original bug itself, and Creevsdak's generous code donation would be a great improvement. Of course others may reasonably disagree.

[DavidW]

Coming late to this:

 

- yes, SCS should fail more gracefully in situations like this (and does, as of v31);

- equally, I think it's good practice to make sure mod-changed files carry on conforming to the game's conventions (especially in a period where the game itself is under active development)

- as a point of interest, what SCS is doing is going through and making sure that all (non-joinable) creatures with kits are legal. It does so by reading in the CLAB files to learn what kit powers ought to be. It's significantly simpler, and more future-proof, to code it to do this automatically for all kits, rather than to do specific ones on a case-by-case basis.

[Magus]

On the other hand, I believe that the installations should fail more. There are too much "IF_EXISTS", "if_valid", etc clauses in the mods. As a user, when I install a mod, I want it to actually do what it's supposed to do, not what it can do given this and that and that over there.

What is technically an error is an error. While SCS may not do anything with that error, other mods may. So my approach on this would be "fix the bug and thank SCS author for pointing it out".

[DavidW]

I'm not unsympathetic. But in practice, what happens is:

 

1) some other mod mangles a file

2) SCS chokes on it and refuses to install

3) I get blamed!

[subtledoctor]

1) some other mod mangles a file

2) SCS chokes on it and refuses to install

3) I get blamed!

Precisely. The errors that occasioned this thread were not (easily) being tracked back to the offending mods (like mine! )

 

As for the quite valid concern about mods doing what they are supposed to do: in this particular case, of clab tables pointing to missing .spl files, SCS will function perfectly well in that situation. Therefore, there's no reason to stop SCS from being installed.