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

Adding built-in spells to casting items #1230

Merged
merged 6 commits into from
Jan 5, 2019

Conversation

gmriggs
Copy link
Collaborator

@gmriggs gmriggs commented Jan 4, 2019

This starts to address #1225

This is not a full solution, but it does fix the crashing, and gets the spell to cast

Open questions:

  • Should there be a windup animation when casting spells built into the wand?
  • Should those spells use the 'Spellcraft' from the wand for the attack skill level, instead of the player's magic skill?
  • Should those spells use mana from the wand, instead of from the player's mana?

ghost
ghost previously approved these changes Jan 4, 2019
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

  • I don't remember there being a separate windup other than the cast animation involving pointing the staff or wand.
  • Not sure about this one; if the Spellcraft of the implement is used, that could have the possibility of limiting its usage range
  • Yes, the mana of the implement should be used for casting the built-in spell

@mcreedjr
Copy link
Contributor

mcreedjr commented Jan 4, 2019

For what it's worth, here is how I remember things:

  • From what I remember, certain wands/staves had different wind-up animations. One that comes to mind is the Focusing Stone. IIRC the wind up animation had the Focusing Stone drawn over the caster's head and a drawn-out, almost slow-motion method before casting. There were no visible spell building 'bubble' in front of the caster that I recall
  • It was my understanding that the Spellcraft was equivalent to the skill level used to cast the spell. Wielding (and using) a wand or staff with a spell did not require the user to be trained in a school in order to use the spell embedded in the staff/wand as long as the met the wield requirements. IE if a staff had a level I war spell on it, the wielder would be able to use it even if they weren't trained in War Magic as long as they met the other wield/use requirements of the staff. The spell that was cast using the wand had the 'skill' level associated with the Spellcraft on the item for the purposes of determining whether or not the spell was resisted by the recipient, etc
  • And, yes, I agree mana was drawn from the casting implement and not the caster's mana

@gmriggs
Copy link
Collaborator Author

gmriggs commented Jan 4, 2019

Updated PR to use mana from the wand/staff/orb for built-in spells

For the windup/cast animations, to my current understanding of the spell data, this would perhaps come from the spell itself? IE, if a spell had a special cast motion, this would be determined by the talisman in the spell formula, and not necessarily on the casting item.

Looking at some retail pcaps, I found some examples of players casting Priest's Curse, which I assume to be from the Imp Staff... it seemed like they were doing the regular windup/bolt cast animation there from the ones I found. Some videos here would really help match things up though.

For the item spellcraft, that does indeed make a lot of sense, especially if the player doesn't have a particular magic school trained for the spell. I left a comment for that part, haven't implemented it quite yet, because I wanted to verify the wield requirements for the casting item were being checked first... ie. didn't want level 1 players able to wield some powerful casting item and instantly get level 300 attack skill for that spell =)

@gmriggs
Copy link
Collaborator Author

gmriggs commented Jan 4, 2019

Also guessing the built-in spells wouldn't check or consume any spell components?

@mcreedjr
Copy link
Contributor

mcreedjr commented Jan 4, 2019

Also guessing the built-in spells wouldn't check or consume any spell components?

Definitely correct. Which is also dovetails into my reasoning why the caster doesn't need to have the magic school trained. None of the typical checks are performed when a spell is cast from a staff/wand/item

# Conflicts:
#	Source/ACE.Server/Managers/EnchantmentManager.cs
#	Source/ACE.Server/WorldObjects/Player_Use.cs
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

However, if the caster implement has wield requirements, those should be check, and I believe are checked, before the person can wield the item. If after the Arcane Lore or specific skill requirement checks are passed to be able to either wield the item or activate it, I don't believe that the built-in spell makes any further checks.

@gmriggs
Copy link
Collaborator Author

gmriggs commented Jan 4, 2019

Ok, I think a lot of these points should be accounted for now:

  • The casting implements with built-in spells did indeed have custom casting animations. These were found in PropertyDID.UseUserAnimation.

  • The War Magic: 270 activation requirement was not showing up in the appraisal window. This is another field that required munging in AppraiseInfo, where PropertyDID.ItemSkillLimit is converted into PropertyInt.AppraisalItemSkill for the client display

  • The activation checks were added to augment the wield checks. If the player doesn't meet the activation requirements (Arcane Lore 50, War Magic 270) for the Imp Staff, they can still wield it, but they now get the appropriate error when trying to cast Priest's Curse

  • The amount of mana consumed from casting Priest's Curse now comes from the ItemManaCost field. As per the staff description, this is reduced with Mana Conversion

  • Casting Priest's Curse now uses the Item Spellcraft as the attack skill level

Open questions:

  • Did casting Priest's Curse have any chance of fizzle?

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

I want to say that I don't ever remember built-in spells fizzling; however, I didn't use them all that much.

@gmriggs gmriggs merged commit 3974e2e into ACEmulator:master Jan 5, 2019
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.

None yet

2 participants