You are here

Misc Bugs in MOTB I don't see posts on

87 posts / 0 new
Last post
kevL's

i'll look at recipies tomorrow.

re. debug
use, bOwnedCharacter=FALSE in GetFirstPC()

Otherwise if you're crafting (or whatever) with a Companion the message won't appear.

But if you are using an OwnedPC, save 'ginc_crafting' and compile 'gp_forgemagic_ca' for the Magical Workbench; 'gr_domagiccrafting' looks like it's called by 'i_nx1_container01_ci' for the MotB satchel ...

  • up
    50%
  • down
    50%
Vandervecken

Ok, for Issue 1: You really should not be using the function GetMatchesAffectedItems for anything. It is broken. Or more specifically, it delegates to GetIsItemCategory in ginc_item, which calls GetState to try to match two integers. GetState is just X & Y, which for integers 11 and 8 returns 1. This feeds back into permitting the category, when it is clearly forbidden. This is why spells that only work on category 11 appear in the list for an object of category 8.

I'm not sure why GetIsItemCategory would delegate to such a dumb function as GetState, but unless we want to override ginc_item to be smarter (which we could, we could easily just try to match exact integer values, taking into account special cases like -1), we should probably be doing something smarter when category matching.

  • up
    50%
  • down
    50%
kevL's

geez, i just spent the last 2-1/2 hrs fighting with the Gui Okay-btn callback ...

[ note: ended up having to break isSpellId() out of 'ginc_crafting' and putting a copy of it directly into 'gui_trigger_spell_set' as isSpellIdGUI() ]

no idea why -- it was working fine -- but it likely has something to do with the fact gui_* scripts compile on-the-fly.

- just a warning there!


Okay, addressing the item-type issue. So it sounds like, and looks like, someone tried to use bitwise comparison on integer that isn't set up to handle bitwise.

...

  • I created a Belt of Agility and put in a Ruby and a Radiant Fire Essence. If offered me Burning Hands and Flame Strike, both of which generated "Not a valid enchantment" errors.

confirmed: "8==11" in crafting!

  • just try to match exact integer values

- yep,

  • up
    50%
  • down
    50%
kevL's

aside: This is getting bludy 'orrible.

Eg, stock item-type #8 is Misc_Equippable, but TCC item-type #8 is Shield.

and there's so much spaghetti roaming through ginc_crafting it's difficult to assume which functions are being used, or if a function is even used at all.

It's about time to parse the whole dang mess, for sanity.

  • up
    50%
  • down
    50%
Vandervecken

Thoughts:

* We need to edit the GetMatchesAffectedItems function to handle a) integer matching, and b) special-casing against multiple categories (such as TCC's groups 1, 0, -1 and -2, but original game will have its own definition of which categories catch which others).

* The version that I'm looking at doesn't properly handle multicategories such as "11,1" - it just grabs the first integer. It expects any list to be preceded with a "B", and this is violated in TCC. I need to fix that.

* I clearly need to heavily modify ginc_item for the TCC version, so that it corresponds to the TCC types.



 

  • up
    50%
  • down
    50%
kevL's

Vandervecken said

  • * I clearly need to heavily modify ginc_item for the TCC version, so that it corresponds to the TCC types.


I suggest simply copying the needed functions from ginc_item (etc) to ginc_crafting, and modify their name & body. Leave the original functions basically intact where they were ....

- make ginc_crafting "your own"

after all, i don't think you really want have to distribute a bunch of modified stock includes ...

--

  • * We need to edit the GetMatchesAffectedItems function to handle a) integer matching, and b) special-casing against multiple categories (such as TCC's groups 1, 0, -1 and -2, but original game will have its own definition of which categories catch which others).

suspicion: sometimes the stock types are used, and other times the TCC types are used ..... That needs to be sorted out and conformed w/ the TAGS in Crafting.2da
 

  • * The version that I'm looking at doesn't properly handle multicategories such as "11,1" - it just grabs the first integer. It expects any list to be preceded with a "B", and this is violated in TCC. I need to fix that.

that jives with what I just saw in some debug: "11,1" looks like it only tested "11"

/cheers

  • up
    50%
  • down
    50%
Vandervecken

It looks like both TCC and the TCC patch I rebooted distribute ginc_item, but neither of them make any sense. The Item Category integers appear to me to have no connection to anything and I'm loathe to edit it now. I think I agree with you that I need to modify ginc_crafting instead. I will work on this.

Please note in a couple of days I head off to Iceland for a vacation (about 1.5 weeks). There's every chance I won't be able to make this work before I leave, and it's driving me crazy, too much to do. I'm going to drop this until I return. How do you feel about adjourning until around Oct 5?

  • up
    50%
  • down
    50%
kevL's

sure, np.

my goal is to crop my version of ginc_crafting down to basic & needed functions. Then it should make sense ...


iceland, cool :)

  • up
    50%
  • down
    50%
kevL's

- posting some notes as i find them ...


think i found another bug in

void DoSalvage(object oItem, object oPC)

this bit at the bottom of function:

    // Do the skill check and create the results
    int bSkillSuccess = FALSE;
    if (StringToInt(Get2DAString("tcc_config","value1",29)))
    {
        if (GetIsSkillSuccessful(oPC,SKILL_SPELLCRAFT,iSalvageDC))
            bSkillSuccess = TRUE;
    }
    if (StringToInt(Get2DAString("tcc_config","value1",29)) && bSkillSuccess)
    {
        DestroyObject(oItem);
        CreateListOfItemsInInventory(sSuccessProduct,oPC);
    }
    else
    {
        DestroyObject(oItem);
        CreateListOfItemsInInventory(sFailureProduct,oPC);
    }


what that means, is that if tcc_config value #29 (Toggle_SalvagingUsesSkillCheck) is 0 (ie. don't use a skillcheck when salvaging materials) the result will *always* be failure products. I don't think that's the intended behavior, and certainly not what I want in my version ... so,

    // Do the skill check and create the results
    string sProduct;
    if (!StringToInt(Get2DAString("tcc_config", "value1", 29)) // auto success if no skillcheck required
        || GetIsSkillSuccessful(oPC, SKILL_SPELLCRAFT, iSalvageDC))
    {
        sProduct = sSuccessProduct;
    }
    else
        sProduct = sFailureProduct;

    DestroyObject(oItem);

    CreateListOfItemsInInventory(sProduct, oPC);


Edit: opps, good policy to destroy oItem before creating new stuff.
--
btw.
If there are script "backups" in /Templates, compare them w/ the scripts in /Scripts and then remove them. They've been known to cause bugs when the GUI compiles on-the-fly and finds an #include in /Templates and goes with it (instead of the proper #include in /Scripts )

note: These observations are based off the Rebalanced edition of TCC.

  • up
    50%
  • down
    50%
kevL's

bug, DoMagicCrafting()

- when checking if a recipe has been disabled, a column labeled "DISABLED" is searched. But the name of the column in Crafting.2da is "DISABLE".

- fix: change either the label of the column or the string.


note. I went with "DISABLE". ( although i prefer "DISABLED" it's easier to change the script )

  • up
    50%
  • down
    50%
kevL's

bug for MotB, DoMagicCrafting()

this needs to be bypassed when using Mold or Malleate Spirit spellabilities:

    // Check for the required feat on caster
    int iItemType = GetTCCType(oItem);
    if (iItemType == TCC_TYPE_MELEE ||
        iItemType == TCC_TYPE_ARMOR ||
        iItemType == TCC_TYPE_SHIELD ||
        iItemType == TCC_TYPE_BOW ||
        iItemType == TCC_TYPE_XBOW ||
        iItemType == TCC_TYPE_SLING ||
        iItemType == TCC_TYPE_AMMO)
    {
        if (!GetHasFeat(FEAT_CRAFT_MAGIC_ARMS_AND_ARMOR,oPC) && StringToInt(Get2DAString("tcc_config","value1",4)))
        {
            ErrorNotify(oPC,ERROR_NO_CRAFT_MAGICAL_FEAT);
            return;
        }
    }
    else
    {
        if (!GetHasFeat(FEAT_CRAFT_WONDROUS_ITEMS,oPC) && StringToInt(Get2DAString("tcc_config","value1",4)))
        {
            ErrorNotify(oPC,ERROR_NO_CRAFT_WONDROUS_FEAT);
            return;
        }
    }


eg, wrap it in

if (iSpellID != 1111 && iSpellID != 1112) // Mold & Malleate Spirit
{
    // stuff from above.
}


note: should have a look at the stock function 'cause there it's done differently (and in the process it honors alternate required Feats, which can be other than Craft Wondrous and Craft Arms & Armor).

  • up
    50%
  • down
    50%
kevL's

bug, DoMagicCrafting()

// Check to see if property to be added is free
int bFreeProp = FALSE;
int iType = GetIntParam(sEffect,1);


ip-type ought be GetIntParam(sEffect,0)


also, no check appears to be made for GetIsLegalItemProp() -- cf. stock function.

  • up
    50%
  • down
    50%
kevL's

bug ... 'ginc_crafting'

// ====================================================================================
// Support function for the recipe exclusion system
// ====================================================================================

int GetHasExcludedProp(object oItem,int iRecipeMatch)
{
    string sExclusions = Get2DAString("crafting","EXCLUDE",iRecipeMatch);
    
    if (sExclusions == "")
        return FALSE;
    
    int iParamVal = GetIntParam(sExclusions,0);
    int iTicker = 0;
    
    while (iParamVal != 0)
    {
        if (GetNumberPropertiesOfType(oItem,iParamVal) >= 1)
            return TRUE;
        iTicker = iTicker + 1;
        iParamVal = GetIntParam(sExclusions,iTicker);
    }
    return FALSE;    
}


returns False if (iParamVal == ITEM_PROPERTY_ABILITY_BONUS). Try something like this:

// Checks if oItem has an ip that excludes that of iRecipeMatch.
int GetHasExcludedProp(object oItem, int iRecipeMatch)
{
    string sExclusions = Get2DAString(CRAFTING_2DA, "EXCLUDE", iRecipeMatch);
    if (sExclusions != "")
    {
        int iExclusiveType;
        itemproperty ipScan;

        int iTokens = GetNumberTokens(sExclusions, ",");
        int i;
        for (i = 0; i != iTokens; ++i)
        {
            iExclusiveType = GetIntParam(sExclusions, i);

            ipScan = GetFirstItemProperty(oItem);
            while (GetIsItemPropertyValid(ipScan))
            {
                if (GetItemPropertyDurationType(ipScan) == DURATION_TYPE_PERMANENT
                    && GetItemPropertyType(ipScan) == iExclusiveType)
                {
                    return TRUE;
                }
                ipScan = GetNextItemProperty(oItem);
            }
        }
    }
    return FALSE;
}



beware, several (most) of the ipScans in TCC ignore DURATION_TYPE_PERMANENT, yet permanent duration ip's are the only ones that really matter in Crafting.

Eg.
GetNumberItemProperties
GetPropSlotsUsed
GetNumberPropertiesOfType
ClearSetProperties

conversely, this does respect dur permanent:
GetPropertyIsUpgrade
 

  • up
    50%
  • down
    50%
kevL's

in 'gui_tcc_constructset'

local_object "tcc_enchanter" isn't cleaned off the PC-object.

  • up
    50%
  • down
    50%
kevL's

check that multiple IPs get added when "EFFECTS" say they should. In mine they aren't/weren't

suggestion: ditch anything that deals exclusively with Recipe Creation. isObsolete=TRUE

  • up
    50%
  • down
    50%
kevL's

okay, looks like I got a basically correct 'ginc_crafting' now

i say "basically" because it doesn't support multi-EFFECTS yet. would like to put that in but it's *a lot* of extra loops.

When you get back, Vandervecken, I should slide it over to you. Be warned, however, that it's a re-work of all the crafting scripts, not just ginc_crafting.


a question: It looks like enchantments that apply to TAGS type "any" explicitly excludes type "melee" -- can you think of a reason for that or should this seemingly arbitrary restriction be removed?

  • up
    50%
  • down
    50%
kevL's

the more i look at what it takes to handle multiple EFFECTS, the more leary it is ...

for example,
there's a test for "isUpgrade" ... okay so one of the ip's is an upgrade and another isn't (easier said than done)

then there's the fact that there's a limit to the # of enchantments that oItem can have applied, by a caster of epic/non-epic level ...
so if the limit is reached, what decides which ip gets applied and which go to dev-nul ? (tempted to say that if a single slot is available on oItem that all ip's go on as one -- but there's also a further maximum limit of ip's on a given item-type: generally it's 8)

And how should multiple ips's be tested: If any one is an upgrade (free slot) then they're all upgrades? Or if any one is not an upgrade then none are an upgrade (charge slot cost)


All up and down the line in DoMagicCrafting(), multiple EFFECTS should be tested repeatedly.
for example,

test each against (a) has any of the ip's been excluded by Crafting.2da "EXCLUDE" (b) are any of the ip's illegal for the item-type, per ItemProps.2da


etc.

Plus, all of that needs to be duplicated in the pre-test for Imbue Item /gah

/rant

  • up
    50%
  • down
    50%
kevL's

earlier i said

  • geez, i just spent the last 2-1/2 hrs fighting with the Gui Okay-btn callback ...
  • [ note: ended up having to break isSpellId() out of 'ginc_crafting' and putting a copy of it directly into 'gui_trigger_spell_set' as isSpellIdGUI() ]
  • no idea why -- it was working fine -- but it likely has something to do with the fact gui_* scripts compile on-the-fly.
  • - just a warning there!


found out what it was. The on-the-fly Gui-script compiler (internal to NwN2 executable/server) was confused by this construction that I had in 'ginc_item':

const int ITEM_CATEGORY_ANY             = -1;
const int ITEM_CATEGORY_NONE            =  0;
const int ITEM_CATEGORY_WEAPON          =  1;
const int ITEM_CATEGORY_ARMOR_SHIELD    =  2;
const int ITEM_CATEGORY_BOW             =  3;
const int ITEM_CATEGORY_XBOW            =  4;
const int ITEM_CATEGORY_SLING           =  5;
// etc.

const int STATE_TIE     = 1;
const int STATE_FAIL    = 2;
const int STATE_WIN     = 3;


Duplicating the literals, apparently, not right. Is fixed and went back to using isSpellId() in 'ginc_crafting' -- the Gui-script parser isn't very sophisticated, i'd say.

  • up
    50%
  • down
    50%
kevL's

ah, multi-EFFECTS, that call functions like GetAreAllEncodedEffectsAnUpgrade(), are part of SoZ crafting ( 'kinc_trade_crafting' )

  • up
    50%
  • down
    50%
kevL's

have been looking at "Property Sets" -- think, Nasher's wardrobe in the OC.

haven't quite got it figured yet.

But i'm thinking it's not such a good idea -- its modus operandi is to change the tags of all items in the set so that they fire two generic scripts, OnEquip and OnUnequip. And while it's possible to flag an item as disallowed for sets, noone's going to do that ... which can then break a game if a plot-device relies on the tag of an equippable item. And it will break the item if that item already relies on tag-based scripts.

Besides, I can't see any advantage to making an item part of a set atm -- atm it seems like it's purely a restriction that a player might subject his/her equipment to.

any thoughts?

  • up
    50%
  • down
    50%
Kaldor Silverwand

I fixed Nasher's set. The fix is part of the OC Makeover and can also be downloaded separately.  I also created additional sets.  And D&D has a history of such sets, for example, the Hammer of Thunderbolts.  The Nasher set is an OC bug though, not an MotB bug.

I'm not sure I get your concern.  Are you concerned that people tag items as they wish and that somehow someone else may create a plot item with the same tag?  There is no solution to that. People who create items need to use something in the tags to make them unlikely to be accidentally duplicated by someone else.

In the fixed Nasher set there are unique tags for each item. Conflicts with community content seem unlikely.

i_x2_nash_boot

i_x2_nash_cloak

i_x2_nash_glove

i_x2_nash_ring

Regards

--------------------------------------------

Blog: bbellina.blogspot.com

  • up
    50%
  • down
    50%
kevL's

hey Kaldor,
no there's no problems with your Nasher set. Here's what i'm on about: The Complete Craftsman, for example (perhaps the only example..) allows a crafter to turn several items into a set. This is done generically, so ... in order to get them to 'trigger' with tag-based scripting, variables are stored on each item and all items are given a generic tag "tcc_setitem" -- ie, pre-existing items are re-tagged. And included with TCC are generic OnEquip/Unequip tag-based scripts "i_tcc_setitem_eq/ue".

i could take Nasher's boots, eg, and plonk it into a workbench with unrelated items, and turn them all into a set. The boots, along with the other items, would get re-tagged and then the boots would no longer work as part of Nasher's set. That's the problem -- in addition to the fact that any of the items could have been designed as a plot-device that would be checked for a specific tag, such as iirc the Silver Sword opening the betrayer's gate in MotB.

Overall, I'm pretty much going to simply remove the feature from my version of TCC. i don't believe it even gives any advantages but is just more of an "oh cool" thing. I mean, am sure there's a way to enhance it into an "oh Cool!" thing but atm i don't see it ...

  • up
    50%
  • down
    50%
Vandervecken

KevL,

I just got back and haven't had time to parse most of the activity on this thread, but I wanted to say that what you just posted seems wrong.  Specifically: "I'm pretty much going to simply remove the feature from my version of TCC"

It seems like the way that a person gets ino trouble here is by overwriting a plot item (or otherwise tagged item) by making it part of a TCC set. This kind of shooting yourself in the foot isn't a good idea, but it's no reason to remove a feature.

Are you planning to distribute another alternative version of TCC? If so, I don't think that kind of fragmentation is wise.  If not, what's the point in removing the feature from 'your version'?  Just don't use it, right?

  • up
    50%
  • down
    50%
kevL's

hi Vandervecken,

- i hope Iceland was refreshing

I have no plans to distribute TCC. You're free to use anything i have at your discretion. (as in free beer and [mostly] free politics)

regarding TCC Property Sets: I wouldn't delete the code -- it's there but i'd just comment out its hook points, so to speak. One way it could be more useful is if it could be tied in with actual enchanting to lower the crafter's level requirements (say by 1 level per item in the set).


i should get back on the cloud, any recommendations? ( Google Drive, i guess )

all my scripts have been reworked line by line from top to bottom. All functions are ordered and categorized. All the Recipe Creation stuff has been removed (moved out to its own #include file -- am pretty sure it's obsolete beyond the stock game). Things are still complicated, ginc_crafting is close to 3000 lines, but I believe it can be read from top to bottom without too many awkward moments.

I *have* changed some functions names in 'x0_i0_stringlib' so the calls'd have to be reverted for distribution.

- will make a coffee and look into GD. in case you want to look through it, its current functionality should replace all crafting scripts.

 

Vandervecken said

  • This kind of shooting yourself in the foot isn't a good idea, but it's no reason to remove a feature.

you & me know about that, but the re-tagging is done opaquely. There's no warning to the player and if he/she doesn't know how the Property Set subsystem works, I figure it's just asking for a big ~oops.

  • up
    50%
  • down
    50%
kevL's

test Google Drive (download link in upper right corner, i hope)

edit: [link removed, obsolete] moved to Github.

those scripts won't compile on your system without changes, but have a browse through & see if you like them.


EDIT:

bug - i_smithhammer_ac
        if (GetPlotFlag(oTarget))

should be
        if (!GetPlotFlag(oTarget))

  • up
    50%
  • down
    50%
kevL's

here's a way around re-tagging Property Set items -- ie. to NOT retag them and still have them fire the module's OnEquip/Unequip events.

The reality is that those events fire regardless of whether an item has been "tag-based" or not. So, inject code into 'x2_mod_def_equ/unequ' that tests the current item for its TCC_VAR_SET_LABEL, and if it exists call ExecuteScript() for the TCC generic OnEquip/Unequip scripts -- and they'll figure out if all parts have been equipped/unequipped.

personally i'll likely do that in my kPrC Pack module-scripts (which i hook up to any module i play anyway).


And to make Property Sets beneficial, could give the reagents that are required for the final enchant back to oCrafter.

All that will ofc be a major divergence from TCC, regarding the Set subsystem.

  • up
    50%
  • down
    50%
Kaldor Silverwand

Yes, that is a much better solution than changing the tag of items in game.  I haven't looked at the TCC but if it is relying on item variables, then do those variables remain on the item if it is sold and then repurchased from a store? Seems to me that there was a store bug that caused variables to be lost on sold items.  If so, then maybe TCC should also make the items unsellable since they rely on those variables.

  • up
    50%
  • down
    50%
kevL's

interesting ...

- tests showed no issues when selling and re-buying unstackable items (even after reload)

- variables on stackable items may go a bit haywire ... i imagine the vars are still there on one of the items in the stack but getting at it is another thing.


I don't think the vars disappear, Kaldor. On the other hand, the 'ga_open_store' script calls RecreateItemsFromBlueprint(), which seems to clear and refresh all items in a store every time it's opened -- all vars lost, and if there was a sold unique item it disappears.

Basically it's an issue with the stock 'ga_open_store' script, i'd guess.

.. marking the Property Set items plot should be a consideration regardless, but I don't think it's strictly necessary atm.

 

okay i see the problem: if the store has an infinite supply, and a sold item has the same name (&tc?) it goes into the infinite stack and is lost. Fortunately the Property Set creation routine prepends a user-defined string to the item's name. I'll make sure that's not a blank string ... I might just "plot" things anyway ... it'd likely skirt several potential issues (eg. renaming with the smith's hammer)

tks!

  • up
    50%
  • down
    50%
Vandervecken

There's so much to catch up on here, so I could use some clarification, probably as I work. kevL: do you suppose we could make IM contact, so I can talk in real time when I work on it? Where are you in the world? I am in PST timezone.

 

  • up
    50%
  • down
    50%
kevL's

PM sent

  • up
    50%
  • down
    50%
kamal
The Vault irc channel is also a handy place for talking.
  • up
    50%
  • down
    50%
kevL's

Vandervecken & i are shuffling notes in PM -- although if he really wants irc I can swing it.

and ... woohoo, first successful test of rewritten Property Sets without re-tagging! It's quite something to watch the Character Sheet suddenly light up when all items are equipped.

  • up
    50%
  • down
    50%
kevL's

i haven't heard from the V. for a while ... regardless the code has been revamped from top to bottom and I keep poking at the .2da's (checking instructions for ~2000 recipes isn't an appealing task -- but i want it :)

  • up
    50%
  • down
    50%
Vandervecken

Folks, I'm still here. Just lots of things in my way. I was all set to settle down into it last night but my mother called and I ended up on the phone to her for 2 hours. So it goes.

Kev: If you're poking at 2DAs, I recommend using the 2das in my released version as a start, they have a lot of bugfixes in them already.

I'll get there, I promise!

  • up
    50%
  • down
    50%
kevL's

hey V.
I would do that, except i've already done lots to my .2da's over the years (like increasing max bonus to +8). The scripting ought work with any well-formed Crafting/Index 2da that follows the rules:

- spelltriggers have to be first
- reagent tags have to be alpha-numerically sorted, case sensitive (caps before lowercase)

- Property Set recipes require special consideration, which we can get to when we get to it.

- and I'd like to see "SKILL" always have a definite value, because many have been left with "0" (ie, feat: Alertness) instead of "-1" or perhaps "****" (preferably -1). This is how my scripting is currently configured:

// Crafting.2da "SKILL" for magical Crafting
// -2  : no feat required
// -1  : check feat based on TCC-type of _oEnchantable
//  0+ : check corresponding feat in Feat.2da

--
but that's not in the current git TccScripts, which still handle SKILL the old way

tbh, the new implementation of Property Sets has brought stuff so far past that it's almost mind-boggling :)

however, I hesitate to upload them because they rely on stuff that's not unique to Crafting. ( for example, i have a file called 'constants_inc' which contains any constants not defined in NwScript.nss )

ofc they can be adapted ...

  • up
    50%
  • down
    50%
Vandervecken

Folks, after several months of collaboration between myself and KevL, I present to you:

The Complete Craftsman Reboot!

https://neverwintervault.org/project/nwn2/hakpak/original-hakpak/complet...

Forum thread here:
https://neverwintervault.org/forums/neverwinter-nights-2/nwn2-custom-con...

  • up
    50%
  • down
    50%

Pages