Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Item enchantment spells #556

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
Draft

Conversation

Sagenlicht
Copy link
Contributor

This bunch contains two Players Handbooks spells:

Spell Enum Spell Name
039 Bless Weapon
1160 Align Weapon

and 11 Item Enchantment spells from the Spell Compendium

Spell Enum Spell Name
1075 Sonic Weapon
1103 Dolorous Blow
1115 Deafening Clang
1130 Shield of Warding
1134 Undead Bane Weapon
1141 Lawful Sword
1149 Brambles
1154 Align Weapon, Mass
1155 Bless Weapon, Swift
1168 Spikes
1170 Weapon of Energy

This is not ready to merge

Of special note:

  1. The spells don't use the target mode item, but touch, multi or self (depending on the spell). Due to this to have a proper spell ending, the orignal spell target gets removed in the spell script and the enchanted item is added in the ET_OnConditionAdd Hook.
  2. Deafening Clang adds a helper condition to removed deafness after 1 min

Known Issues:
No visual feedback off the duration of the spells at all
Spell particles need to be added
Shield of Warding does not expire properly but ends in a permanent condition. This is bad :(

Missing Spells:
I have not redone the armor enchantments so far, TODO

@dolio
Copy link
Contributor

dolio commented Jul 30, 2021

One thing I've been mulling over...

Presumably with the python UI stuff, there will be spells that pop open their own UI. How hard would it be to make it so that spells could request that the game do existing picker (or similar) UIs? Like, if all the item affecting spells could be range touch or whatever, and then the spell script could say, "open an inventory picker for the targeted critter and give me the object," that would cover what is needed for these spells, right?

That would mean less pressure to expand the (integer-based) targeting modes. It could also more easily accommodate spells that have options for how to target things. Like, the option for Grease to affect either an area or an object could be implemented via a radial option that controls how the subsequent targeting works.

I think the downside would be that you need to commit to casting the spell before finding out that there's a targeting problem, so maybe this way is not ideal.

@dolio
Copy link
Contributor

dolio commented Aug 18, 2021

Also...

Bane weapons are a standard thing that might be nice to have. It's exactly the same effect as Undead Bane Weapon, but parameterized by creature (sub)type. So maybe you could factor out the item_condition part and use slots 0 and 1 to indicate the type and subtype expected. Probably storing 1+E is better for the subtype indicated by 2^E, since the standard bane weapons only target one subtype if any, with 0 used for cases where there are no subtypes (plant, undead, ...).

However, these shouldn't stack, I think. If you have a weapon that is bane against chaotic outsiders and bane against evil outsiders, it shouldn't be double bane against chaotic evil outsiders. So a non-stacking bonus type should be reserved if this is done.

Do the dice stack, too, though? That might be a problem.

@Sagenlicht
Copy link
Contributor Author

Sorry for my late reply, kids holidays :)

There is a vanilla bane weapon condition, which can't be used as it only stores 3 values, so you can't remove it via item_condition_remove.
While I fully agree it would be nice to have a general bane weapon enchantment, maybe carefully replacing all existing weapon enchantments to use 5 args would be a good idea anyways?

@dolio
Copy link
Contributor

dolio commented Aug 27, 2021

Oh. I thought there wasn't an existing enhancement because it isn't in the crafting options.

Mainly I was thinking that the IWD conversion might want a bane enhancement, because there are a lot of bane weapons in IE games. If it already exists for permanently enhanced items, it's not as urgent. The only class I can think of that creates all flavors of bane weapons temporarily is Artificer.

@DudeMcDude
Copy link
Contributor

Oh. I thought there wasn't an existing enhancement because it isn't in the crafting options.

Mainly I was thinking that the IWD conversion might want a bane enhancement, because there are a lot of bane weapons in IE games. If it already exists for permanently enhanced items, it's not as urgent. The only class I can think of that creates all flavors of bane weapons temporarily is Artificer.

Oh, I hadn't even realized that. I was aware of Bane, but always thought it was vs. undead for some reason (probably got it mixed up with Disruption).
I think IWD has more specific vs. creature weapons, but I suppose it's close enough.

@dolio
Copy link
Contributor

dolio commented Aug 30, 2021

I don't know what IWD has, but BG has some weapons that are like "+1, +3 vs. Shapeshifters" that I assume are what got codified into "bane" in 3E (although I don't know that 'shapeshifter' is a 3.5 category). I think it also has some weirder ones, like "+1, +3 vs. X, +4 vs Y", but bane might be close enough for those, too.

@Sagenlicht
Copy link
Contributor Author

Updated Sonic Weapon to test my idea of handling buff icons for the item enchantment spells. Removing the icon works perfectly, re-adding is currently done at OnBeginRound as I didn't find a better way. Though instantanous would be much better.

What are your thoughts about this buff icon idea? I really would like to give a feedback to the player, how long the item enchants last, and I don't think I can add it to the item description.

If this idea is ok, I will add it to all item enchantments

@dolio
Copy link
Contributor

dolio commented Sep 24, 2021

Why not do the indicator in the same condition as the item_condition_add? Wouldn't that make it only show up while they're holding the weapon, and that's what you want?

@Sagenlicht
Copy link
Contributor Author

Why not do the indicator in the same condition as the item_condition_add? Wouldn't that make it only show up while they're holding the weapon, and that's what you want?

I am not 100% sure I fully understand your comment dolio. I can't add the buff symbol to the weapon, I need to attach it to its wearer. And because you could unequip or even pass the weapon after the item was enchanted to a different PC, I have to make sure the buff icon follows as well (or is removed in case of a simple de-equip).

Maybe I am missing something, but I fail to see how I could handle the buff icon in the item_condition.

@dolio
Copy link
Contributor

dolio commented Sep 24, 2021

item_condition_add_with_args adds a condition that is applied to whoever is holding the item. If you add tooltip hooks to that condition, won't it show up while the weapon is wielded or whatever? All the callbacks run with the item holder as the attachee, not the item.

@Sagenlicht
Copy link
Contributor Author

Sagenlicht commented Sep 25, 2021

Ah ok ty for your help dolio! Works fine now, I updated the Sonic Weapon spell to reflect this. I added a 6th arg that holds the duration, though this arg has no real effect other than for display purpose. The end is still handled by the original condition.

If I can use this for all item enchantments, I will add this to all others as well. I updated the spell_utils for this too.


alignType = args.get_arg(1)

if alignType == 1:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO there should be particle effect only when it's really necessary, otherwise it will wear out and start to annoy each time. Essentially aligned weapon should bypass aligned damage reduction. For example Hezrou suppose to have Damage reduction 10/good. So when alignment of a weapon does no real bonus, then there should be no particle.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will look into it

@dolio
Copy link
Contributor

dolio commented Sep 25, 2021

If you're putting the duration on the item_condition_add thing, you should do some testing with removing and equipping it. I think any duration updates might only occur to the creature-applied condition, not the original item condition, so re-equipping it might reset the displayed duration.

Basically, I think the creature holding the item might get a copy of whatever you originally passed to item_condition_add every time, so it needs to work in a way that doesn't require updating state for that particular condition.

Having it query the weapon for the duration ticking on it would be the way to fix that, I think.

@Sagenlicht
Copy link
Contributor Author

Thanks again dolio.
I updated Sonic Weapon once again, It is now possible to choose main or offhand when casting the spell and I fixed the tooltip duration problem.

@dolio
Copy link
Contributor

dolio commented Oct 3, 2021

It doesn't seem like the duration query for item conditions will work if there is more than one spell on the weapon that uses that system. It will just report the duration of the most recent condition (I think).

@Sagenlicht
Copy link
Contributor Author

You are right dolio, I need to add a check to verify it's the same spell_id. Will fix it

@DudeMcDude
Copy link
Contributor

The duration tooltip should be coupled to the spell condition, not the item enhancement condition, since the latter could be used in crafting.

@dolio
Copy link
Contributor

dolio commented Oct 3, 2021

Putting a tooltip hook on the spell condition that targets the item doesn't do anything, though. It has to be on the item wield condition for it to show up on the character, even if it queries the spell condition to see if it should display.

Maybe the weapon card stuff could be given a hook. But someone on the Co8 forums was remarking that it's hard to tell if e.g. Disrupting Weapon is still active in combat, and the weapon card doesn't seem like the most accessible place for that information, unfortunately.

@DudeMcDude
Copy link
Contributor

What's the weapon card?

I guess we could expand the tooltip query for items too.

But on second thought, which of these enchantments can be permanent? It seems like it's just Undead Bane.

@dolio
Copy link
Contributor

dolio commented Oct 3, 2021

Sorry, by "weapon card" I mean the details you can see by clicking on the item appropriately in your inventory. You can see enhancement and keen and such there, but only conditions from the original game. And I'm not sure you can get actually get into that window while the item is equipped. I guess those are actually tied to the item condition, too, now that I think about it, because they display for permanent conditions, not actually based on the active spell.

@Sagenlicht
Copy link
Contributor Author

Updated Sonic Weapon

I moved the tooltip to a secondary condition, so if you only want to apply the item condition as a permanent buff (crafting) the tooltip won't be there.
Changed the query to include the spell_id, so the tooltip only activates if it matches the tooltip.

In my testings this looks actually pretty solid now. If this gets approved, I would add this method to the other spells as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants