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

[lua] Fix Casket Drop Rate Prowess #6406

Merged
merged 1 commit into from
Nov 7, 2024

Conversation

UmeboshiXI
Copy link
Contributor

@UmeboshiXI UmeboshiXI commented Nov 4, 2024

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 treasure casket drop rate bonus from GoV Prowess effects. The bonus being based on effect power was being fed into
rand < utils.clamp(xi.settings.main.CASKET_DROP_RATE + kupowersMMBPower + prowessCasketsPower, 0, 1)
without any adjustments to convert it into a float. This caused the casket drop rate to be capped out because the value would always be more than 1(The clamp for the float).

This PR divides the effect power by 100 so that the final bonus value reflects the proper percent bonus to be added to the base casket drop rate. An effect power of 4 will now be 0.04 which would equate to a 4% increase.

Steps to test these changes

  1. Go to a GoV book zone, complete regime until you get casket rate increase or use !exec player:addStatusEffect(xi.effect.PROWESS_CASKET_RATE, 4, 0, 0) to give yourself 1 rank of Prowess Casket Rate bonus(4%).
  2. Kill mobs and see that the drop rate is not 100%.
    2b. Use a print in the above function to verify the actual drop rate is correct.

@@ -185,7 +185,7 @@ local function dropChance(player)
--end

if player:hasStatusEffect(xi.effect.PROWESS_CASKET_RATE) then
prowessCasketsPower = casketProwessEffect:getPower()
prowessCasketsPower = casketProwessEffect:getPower() / 100
Copy link
Contributor

Choose a reason for hiding this comment

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

While you're here and you understand the code, can you change the local rand = math.random() below this to be math.random(1, 100) (like we're trying to use in the rest of the code?

Would that remove the need for this /100 divisor?

Copy link
Contributor Author

@UmeboshiXI UmeboshiXI Nov 4, 2024

Choose a reason for hiding this comment

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

I can change/update it. May need to make some adjustments. I could remove /100 divisor but something similar will still be needed depending on which option you prefer.

xi.settings.main.CASKET_DROP_RATE = 0.10 Default Setting.

kupowersMMBPower and prowessCasketsPower are integers (effect:getPower())

Options:

  1. I could change the casket droprate in settings to use 1-100 instead of a 0.0-1.0 float.

  2. I could multiply the settings drop rate by 100 to convert it into an integer. (This is the code block right below the original commit that adds/clamps everything)

    if math.random(1, 100) < utils.clamp((xi.settings.main.CASKET_DROP_RATE * 100) + kupowersMMBPower + prowessCasketsPower, 0, 100) then return true end

Whichever style/option you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's a much wider change, then leave it as it is 👍

@zach2good zach2good merged commit b0a30c0 into LandSandBoat:base Nov 7, 2024
13 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