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

Show a notification when a critical hit occurs while using a spell. #14696

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

lstanford53
Copy link

@lstanford53 lstanford53 commented Jun 9, 2023

Fixes #8128

Changes

In this PR, a "critBonus" return value is added to the "cast" method for spells. This value can be any of the following:

  • If the spell cannot result in a critical hit, the "cast" method will return nothing and "critBonus" will be undefined.
  • If the spell can result in a critical hit but a critical hit does not occur, the method will return 1.
  • If the spell can result in a critical hit and a critical hit occurs, the method will return the value of the critical hit which will be over 1.

After calling the cast method, this PR will evaluate the "critBonus" return value and show a critical hit message if one is applicable. This has been added to all 3 of the spells that can cause a critical hit. (Brutal Smash, Burst of Flames, and Backstab)

Additionally, it seems that "package-lock.json" was added to this PR. I don't have experience with what that is but I asked ChatGPT and it said: "Including the package-lock.json file in the PR helps ensure that everyone working on the project is working with the same dependencies and reduces the likelihood of version conflicts or unexpected behavior due to inconsistent package versions. It is generally recommended to include the package-lock.json file in your version control system (e.g., Git) to ensure consistent and reproducible builds across different environments and collaborators."

That being said, if you would like me to remove that file from the PR, I can do that.


UUID: 88cf9b3e-5150-4298-ae14-b8b64e0b36a2

@CuriousMagpie
Copy link
Member

If you could remove package-lock.json from the PR, that would be great. It should almost never be included on Habitica PRs.

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

Successfully merging this pull request may close these issues.

Critical hits on skill use are not reported in notifications
3 participants