personally im okay with keeping the delayed deletes in (even if they fail for whatever reason) … just this →
if (iMeta == METAMAGIC_EMPOWER || iMeta == METAMAGIC_MAXIMIZE)
{
// store Metamagic on the weapon
SetLocalInt(oTarget, sMETA, iMeta);
AssignCommand(GetModule(), DelayCommand(fDur, DeleteLocalInt(oTarget, sMETA)));
}
else
DeleteLocalInt(oTarget, sMETA);
Can I just check with you … If I did not have the check …
if (iMeta == METAMAGIC_EMPOWER || iMeta == METAMAGIC_MAXIMIZE)
Would it matter?
As far as I can see, the meta is double checked in the On Hit script anyway. I just want to make sure I have not missed something about the way I had done it. Thanks!
what scripts are you using … ?
I was working from your two scripts above (latest). < I assume only updated once so far … Let me check …
If you’re wondering if it’s okay to set the metamagic value no matter what, yes it is. The default case will be used for anything other than Empowered and Maximized.
Good, That is what I thought it would do… 
I don’t mind leaving the odd var on the item (to be updated if required), as long as it does not get used erroneously. In my head, it’s a case of I’d rather just update when required than have a delay that may fail to be deleted even after doing some of the countdown. 
Sometimes, I wish I knew how to understand what does and does not contribute towards efficiency.
it becomes a guessing game at some pt.
g’nite Lance, off to watch some utubezzzzzzz
if you’re editing Dialog.Tlk you might want to delete “non-magical” from the description. It’s confusing – the spell affects both magical and non-magical weapons …
Ah … That’s good to know … That adds an extra layer of checking for me then, as I may need it not to be added to weapons with properties (magikal) already.
Maybe that TV will have to wait … as I’d like to get this done. It should only require a test for an added property I guess.
Thanks for the heads up … BTW, it was a custom tlk string added for me.
This works for me …
if (GetIsObjectValid(oTarget) && !GetIsItemPropertyValid(GetFirstItemProperty(oTarget)))
It even stops a second Darkfire being cast until the first has ended, which is good for me.
you could write a funct that checks against specific IPs …
int GetIsMagical(object oTarget) // or GetIsDarkfireTarget()
{
itemproperty ip = GetFirstItemProperty(oTarget);
while (GetIsItemPropertyValid(ip))
{
switch (GetItemPropertyType(ip))
{
case ITEM_PROPERTY_ENHANCEMENT_BONUS:
case ITEM_PROPERTY_ONHITCASTSPELL:
// etc etc etc
return TRUE;
}
ip = GetNextItemProperty(oTarget);
}
return FALSE;
}
and allow some IPs by keeping them off the list, eg. ITEM_PROPERTY_USE_LIMITATION_CLASS
and, if the check fails, send an appropriate failed-strref
“Darkfire failed : The item is magical.” (eg)
ps. you could get fancy and consider only GetItemPropertyDurationType() → DURATION_TYPE_PERMANENT as magikal … /your call but i believe it’s something to consider
correct :)
“cost” (behind the scenes like this) is better thought of as “power”
I like this. 
I also realized later that the item ‘material’ will likely return a false positive.
Looks like I’ll have to work on a function like you have outlined above to ensure I don’t make the deciding factor so hard and fast.
EDIT: I remember I already have a function that ensures “slots” for material types are not counted towards property counts that can be added when crafting (my crafting system). I need to amalgamate that in with my system and the new function. That needed a rewrite anyway!
UPDATE: OK, I rewrote my “GetMaterialProp” function and called it via a version of the function you suggested, and I believe this should cover most aspects. Just running it past you just in case I missed anything … I decided to allow a property to be added if a weapon only otherwise had negative properties.
Original spell updated to …
if (GetIsObjectValid(oTarget) && !GetIsMagikal(oTarget))
The two functions used …
///////////////////////////////////////////////////////////////////////////////////////////////////
// RETURNS TRUE IF PROPERTY IS DUE TO ITS MATERIAL TYPE
// ADDED NEGATIVE PROPERTIES AS ONES TO IGNORE AS WELL NOW I.E. ANYTHING WITH ZERO SLOT USE IN ITEMPROPDEF
///////////////////////////////////////////////////////////////////////////////////////////////////
int GetMaterialProp(object oItem, itemproperty ipLoop);
int GetMaterialProp(object oItem, itemproperty ipLoop)
{
string sTag = GetTag(oItem);
int iMATERIAL = GetItemBaseMaterialType(oItem);
int iPROP = GetItemPropertyType(ipLoop);
int iRANGED = GetWeaponRanged(oItem);
// ADAMANTINE SECURES ATTACK BONUS & DAMAGE BONUS (2 PROPERTIES)
if(iMATERIAL == 1 && (iPROP == ITEM_PROPERTY_ATTACK_BONUS || iPROP == ITEM_PROPERTY_DAMAGE_BONUS)){return TRUE;}
// DARK STEEL SECURES DAMAGE BONUS (1 PROPERTY)
else if(iMATERIAL == 3 && iPROP == ITEM_PROPERTY_DAMAGE_BONUS){return TRUE;}
// MITHRAL & DUSKWOOD SECURES WEIGHT REDUCTION (1 PROPERTY)
else if((iMATERIAL == 5 || iMATERIAL == 7) && iPROP == ITEM_PROPERTY_BASE_ITEM_WEIGHT_REDUCTION){return TRUE;}
// ZALANTAR SECURES ATTACK BONUS (RANGED) & WEIGHT REDUCTION (MELEE) (1 PROPERTY)
else if(iMATERIAL == 8 &&
(!iRANGED && iPROP == ITEM_PROPERTY_BASE_ITEM_WEIGHT_REDUCTION) ||
(iRANGED && iPROP == ITEM_PROPERTY_ATTACK_BONUS)){return TRUE;}
// SHEDERRAN CROSSBOWS SECURES ATTACK BONUS (1 PROPERTY)
else if(iPROP == ITEM_PROPERTY_ATTACK_BONUS &&
(FindSubString(sTag, "mwr_bwxh_shd_3") != -1 || FindSubString(sTag, "mwr_bwxl_shd_3") != -1)){return TRUE;}
// SHEDERRAN BOWS SECURES MIGHTY (1 PROPERTY)
else if(iPROP == ITEM_PROPERTY_MIGHTY &&
(FindSubString(sTag, "mwr_bwln_shd_3") != -1 || FindSubString(sTag, "mwr_bwsh_shd_3") != -1)){return TRUE;}
// NEGATIVE PROPERTIES DO NOT COUNT TOWARDS SLOT USE
else if(Get2DAString("itempropdef", "Slots", iPROP) == "0"){return TRUE;}
// DO NOT REMOVE ANY POISON ADDED THIS WAY
else if(iPROP == ITEM_PROPERTY_ON_MONSTER_HIT){return TRUE;} // NX1 SYSTEM
else if(iPROP == ITEM_PROPERTY_ON_HIT_PROPERTIES){return TRUE;} // NX2 SYSTEM
return FALSE;
}
////////////////////////////////////////////////////////////////////////////////////////////
// RETURN TRUE IF THE ITEM HAS ANY *PERMANENT* 'MAGIK' (CRAFTED) OR TEMP 'MAGIC' (SPELLS) ON IT.
// NB: MATERIAL BONUSES DO NOT COUNT AS PERMANENT 'MAGIK', AS IT IS A PROPERTY OF THE MATERIAL.
// ONCE A SPELL IS CAST ON THE ITEM THAT ENHANCES IT IN SOME WAY, THIS WILL RETURN TRUE.
// THEREFORE, A NORMAL MELEE WEAPON CAN ONLY HAVE ONE SPELL ENHANCEMENT ACTIVE AT A TIME.
////////////////////////////////////////////////////////////////////////////////////////////
int GetIsMagikal(object oTarget);
int GetIsMagikal(object oTarget)
{
itemproperty ipLoop = GetFirstItemProperty(oTarget);
while (GetIsItemPropertyValid(ipLoop))
{
if(GetMaterialProp(oTarget, ipLoop) == FALSE)
{
return TRUE;
}
ipLoop = GetNextItemProperty(oTarget);
}
return FALSE;
}
technote: When an if() statement returns a value and is followed by another if() statement, the second statement does not need an else clause …
ie. else is redundant throughout GetMaterialProp()
also I believe this construction is incorrect for what you want →
The OR’d condition will be evaluated on its own, without regard to the value of iMATERIAL. Instead →
if (iMATERIAL == 8
&& ((!iRANGED && iPROP == ITEM_PROPERTY_BASE_ITEM_WEIGHT_REDUCTION)
|| (iRANGED && iPROP == ITEM_PROPERTY_ATTACK_BONUS)))
{
return TRUE;
}for your perusal …
////////////////////////////////////////////////////////////////////////////////
// RETURN TRUE IF THE ITEM HAS ANY *PERMANENT* 'MAGIK' (CRAFTED) OR TEMP 'MAGIC'
// (SPELLS) ON IT.
// NB: MATERIAL BONUSES DO NOT COUNT AS PERMANENT 'MAGIK' AS IT IS A PROPERTY OF
// THE MATERIAL.
// ONCE A SPELL IS CAST ON THE ITEM THAT ENHANCES IT IN SOME WAY THIS WILL
// RETURN TRUE.
// THEREFORE A NORMAL MELEE WEAPON CAN ONLY HAVE ONE SPELL ENHANCEMENT ACTIVE AT
// A TIME.
////////////////////////////////////////////////////////////////////////////////
int GetIsMagikal(object oItem);
////////////////////////////////////////////////////////////////////////////////
// RETURNS TRUE IF PROPERTY DOES NOT COST AN ENCHANTMENT SLOT.
////////////////////////////////////////////////////////////////////////////////
int GetIsFreeIp(object oItem, itemproperty ip);
//
int GetIsMagikal(object oItem)
{
itemproperty ip = GetFirstItemProperty(oItem);
while (GetIsItemPropertyValid(ip))
{
if (!GetIsFreeIp(oItem, ip))
return TRUE;
ip = GetNextItemProperty(oItem);
}
return FALSE;
}
//
int GetIsFreeIp(object oItem, itemproperty ip)
{
int iType = GetItemPropertyType(ip);
switch (iType)
{
default:
if (Get2DAString("itempropdef", "Slots", iType) == "0")
return TRUE;
break;
case ITEM_PROPERTY_ON_HIT_PROPERTIES:
case ITEM_PROPERTY_ON_MONSTER_HIT:
return TRUE;
}
if (GetItemPropertyDurationType(ip) == DURATION_TYPE_PERMANENT)
{
switch (GetItemBaseMaterialType(oItem))
{
default:
if (FindSubString(GetTag(oItem), "_shd_") != -1) // shederran (is not defined per GMATERIAL_ types)
{
switch (GetBaseItemType(oItem))
{
case BASE_ITEM_HEAVYCROSSBOW:
case BASE_ITEM_LIGHTCROSSBOW:
if (iType == ITEM_PROPERTY_ATTACK_BONUS)
return TRUE;
break;
case BASE_ITEM_LONGBOW:
case BASE_ITEM_SHORTBOW:
if (iType == ITEM_PROPERTY_MIGHTY)
return TRUE;
break;
}
}
break;
case GMATERIAL_METAL_ADAMANTINE:
if (iType == ITEM_PROPERTY_ATTACK_BONUS || iType == ITEM_PROPERTY_DAMAGE_BONUS)
return TRUE;
break;
case GMATERIAL_METAL_DARKSTEEL:
if (iType == ITEM_PROPERTY_DAMAGE_BONUS)
return TRUE;
break;
case GMATERIAL_METAL_MITHRAL:
case GMATERIAL_WOOD_DUSKWOOD:
if (iType == ITEM_PROPERTY_BASE_ITEM_WEIGHT_REDUCTION)
return TRUE;
break;
case GMATERIAL_WOOD_DARKWOOD: // zalantar
if (GetWeaponRanged(oItem))
{
if (iType == ITEM_PROPERTY_ATTACK_BONUS)
return TRUE;
}
else if (iType == ITEM_PROPERTY_BASE_ITEM_WEIGHT_REDUCTION)
return TRUE;
break;
}
}
return FALSE;
}
(don’t some of those materials like Mithral have more than one ‘secured’ ip? Or is it just Adamantine …)
ps. possible breakage →
If a player crafts a permanent attack bonus onto an Adamantine weapon, for example, then a subsequent check for GetIsFreeIp() will count that crafted bonus as a free slot … (maybe you want that, idk)
Thanks for checking the script. I was having a bad day and can see the errors you point out now. (Suffering from some seasonal sore throat, which is messing with me. Sleepless nights again.) It’s original old format was even worse before I posted it here. 
Although, I probably broke the or logic when quickly tidying it up for posting.
I’ll take a look at your script fully in the morning. It’ll probably answer some points I needed sorting. …
Iirc, only Adamantine required two.
Idea is to ensure free slots remain for any magikal enhancement for crafting if other ‘enhancements’ are already present due to material properties. IE An Adamantine weapon has as many crafting slots remaining as a normal weapon, and is not restricted to two due to two already present due to its material.
you’re probly right, I might be thinking of mithral’s ACP (?) reduction, but that’s handled by armorrulestats iirc (and so wouldn’t affect enchantment-slots available)
Actually, I also use slots (allowing for material types) for armour too. Hopefully, the function returns an allowance of the mithral light weight property correctly here too. It should be a single allowance for the property.
I managed to look at your script (not easy on my phone ) and that looks better than my old one, so I’ll grab that later. Thanks!
As for additional similar properties like the Adamantine issue you highlighted, I don’t mind an improved enhancement of the same type overriding any existing. Therefore, a PC can improve a material’s existing quality with crafting without using an additional slot.
I’ll try to get some more sleep again now. Again, thanks for addressing the script, which I’ll grab and use for sure later.
+