fixing item proc sources and messages

[gmriggs]

this pr resolves #1922

the code in this pr got a little hairy, in particular the 'bool equipping' that EnchantmentManager seems to require, and the way it needs to be passed through many of the magic functions

i think that maybe these -1 duration spells from equipping items could all be the SpellType.Enchantment, but with the way spells are first passed to Creature/Item/Life Magic, and each one handling SpellType.Enchantment and further passing to CreateEnchantment, required passing this 'equip' bool through many of these functions unfortunately.

the logic in EnchantmentManager that determines if a duration should be -1 used to be a sticking point in the past, but it really seems to boil down to something simple: if i am equipping an item that casts a spell, that spell should most likely be duration -1 to my current understanding. if there are any exceptions to this, please let me know!

Aetheria is an example of how this gets tricky -- it casts item set spells on the wielder when equipped, which are -1 duration. But it also procs spells on both the wielder and target. These procs have a duration. So the logic could be something tricky like a combination of checks like it was performing previously, but to do it the safest and clearest way, i think it really just boils down to this 'equipping' bool

CreateItemSpell() is the function that gets called when an item is casting a spell from equipping it (and only for this case), so this is the only place equip bool gets set to true. Everywhere else in the system, it defaults to false (duration), so hopefully this should limit the types of 'infinite duration' errors that arose in the system awhile ago, and also make the logic much clearer

ResistSpell() has been changed up very slightly, and is now being passed the additional 'caster' parameter that is common in many of the other magic methods. This is done for a few reasons, but now follows a consistent pattern as the other methods: when the current player has an item that casts a spell, instead of calling item.CastSpell() through the pipeline, player.CastSpell(itemSource) is instead used, which remains consistent with the current implementation of the rest of the magic system. The references to both the item caster and wielder are useful here, so it's useful to have references to them both. This could probably eventually be refactored / cleaned up further, but for now having ResistSpell() similar to the other magic methods is useful for consistency.

so for the Phials (thrown weapons w/ item procs), throughout the magic casting pipeline, wielder.TryCastSpell(target, item) is the method used (last param updated), which then calls wielder.TryResistSpell(target, item). Item is being passed for the 'item caster' param, which is used for various operations if not null. For example, in ResistSpell(), it determines the caster from either the item caster or the wielder, and now has the appropriate logic to get magicSkill. If an item caster was passed in, it tries to use its ItemSpellcraft (this part was already there). One part that was missing, if an item caster's ItemSpellcraft is null, it checks if the item is being wielded by a creature, and if so, uses the wielder's magic skill for the spell.

A clear distinction between player and item caster is also useful for determining all of the proper messages:

Source:

Target resists your spell
Pyreal Phial of Imperil casts Imperil Other V on Target

Target:

You resist the spell cast by Source
Pyreal Phial of Imperil cast Imperil Other V on you

For resists, I think the player names are always used, but for the successful cast messages, i think the item names are used there. Haven't confirmed the resist messages yet, but using item names for resists could result in some odd messages

Since these changes touched slightly on a lot of different methods, I went through and re-tested almost every possible magic casting setup, to ensure all the existing functionality was left intact. I tested all the Phials with both monster and player targets, Aetheria item set spells and both types of procs (self and target), item set spells for rare armor, casting all of the different types of magic school spells, a bunch of different wielded items with built-in spells, and also various spell traps. Everything appears to be functioning as expected, but careful review and feedback and additional playtesting is always a good thing