Skip to content

[lua] Blue magic: Fixup magic bursting - #11533

Open
WinterSolstice8 wants to merge 1 commit into
LandSandBoat:basefrom
WinterSolstice8:blu_stuff
Open

WinterSolstice8 wants to merge 1 commit into
LandSandBoat:basefrom
WinterSolstice8:blu_stuff

Conversation

@WinterSolstice8

Copy link
Copy Markdown
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Fixes up magic bursting. I might have accidentally broke some of this doing the massive skill conversion for Enfeeble/Drain/Breath because they don't get the massive default params like standard magical and physical spells.

Additionally, it looks like we were applying the MB bonus to magic accuracy even if Burst Affinity was not up in all cases.

Steps to test these changes

Magic Burst with blue magic and see it work. In particular, MP/HP drains, breaths, enfeebles. Enfeebles will present only as a messaging change (but macc bonus is fed through the appropriate channels as well.)

if skillchainCount > 0 then
finalDamage = math.floor(finalDamage * xi.spells.damage.calculateIfMagicBurst(caster, target, spellElement, skillchainCount))
finalDamage = math.floor(finalDamage * xi.spells.damage.calculateIfMagicBurstBonus(caster, target, spellId, skillType, spellElement))
if canMB and skillchainCount > 0 then

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you already use canMB to default skillchainCount to 0. skillchainCount cant ever be > 0 without canMB being true, making its check here redundant, no?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

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.

2 participants