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

[Job Utils] Bluemage job abilities #5919

Open
wants to merge 1 commit into
base: base
Choose a base branch
from

Conversation

andeluvian
Copy link
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?

This PR contains the jobutils for Blue Mage, jobabilities have been moved not to jobutils

Steps to test these changes

Create a new player with blue mage job either 75 or 99 and each ability should work as they did before the change.

@zach2good zach2good added the squash Reminder to squash commits on merge label Jun 15, 2024
@zach2good
Copy link
Contributor

As a general rule: your commit messages should say what your piece of work is doing in relation to the codebase.

If you introduce some code in an early commit, and then change the formatting (etc.) in a later commit, that's got no relevance to the codebase, it's specific to your code - so squash it please.

Commits as atomic pieces of work, etc.

@zach2good
Copy link
Contributor

While you're doing these PRs, the ability use binding should always have the action param, even if it isn't used:

abilityObject.onUseAbility = function(player, target, ability, action)
    return xi.job_utils.rune_fencer.useBattuta(player, target, ability, action)
end

etc.

It can safely be omitted, because Lua, but Core is piping it in, so it should be shown in case people want/need it later

…_utils

removed unnecessary require

bluemage jobutils refactoring

initial blue mage commit
@andeluvian andeluvian marked this pull request as ready for review July 7, 2024 10:29
Copy link
Contributor

@zach2good zach2good left a comment

Choose a reason for hiding this comment

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

Seems fine to me, just needs work so CI doesn't complain

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
squash Reminder to squash commits on merge
2 participants