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

Chakra, Dodge, Focus retail functionality audit #6454

Merged
merged 2 commits into from
Nov 25, 2024

Conversation

WinterSolstice8
Copy link
Member

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?

Does what it says on the tin, updates Chakra, Dodge and Focus to match retail stuff in the following manners

  • basic functionality
  • recasts
  • duration (where applicable)
  • merit timer reductions

Steps to test these changes

Use Focus/Dodge/Chakra and have them approximately match the following wiki pages:
https://www.bg-wiki.com/ffxi/Focus
https://www.bg-wiki.com/ffxi/Dodge
https://www.bg-wiki.com/ffxi/Chakra

monkLevel = target:getMainLvl()
else
monkLevel = target:getSubLvl()
end
Copy link
Contributor

Choose a reason for hiding this comment

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

We are using this logic a lot, I think we also use it in other places.
Maybe we should just turn it to an util in the globals/utils.lua
Like...

function utils.getCurrentLevel(actor, job)
    local level = 0
    
    if actor:getMainJob() == job then
        level = actor:getMainLvl()
    elseif actor:getSubJob() == job then
        level = actor:getSubLvl()
    end

    return level
end

Probably better for a different PR, but putting it there

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, while I was looking at my diff I noticed that function already exists as utils.GetActiveJobLevel

@WinterSolstice8 WinterSolstice8 force-pushed the mnk_ability_audit branch 2 times, most recently from fd668df to cab541a Compare November 24, 2024 02:55
@zach2good zach2good merged commit a2410cd into LandSandBoat:base Nov 25, 2024
11 of 12 checks passed
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.

3 participants