-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Bug][ME] Allow Part Timer Rewards to Account for Amulet Coin #5343
base: beta
Are you sure you want to change the base?
[Bug][ME] Allow Part Timer Rewards to Account for Amulet Coin #5343
Conversation
…ccount-for-amulet-coin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably just me, but imo everything testable should have an automated test, especially stuff related to bug reports -- this is helpful for regression testing.
I'm not sure it's necessarily your responsibility to write the test, though. I'll approve it, because I'm writing a huge test suite anyway.
Ah ok noted! I'll happily write some unit tests for this! This might be somewhere in the docs, but should I create a new part-timer test for this, or should I update the existing |
Oh, you should use the existing file. It's a bit different to write tests for mystery encounters (compared to, say, moves and abilities) so if you get confused don't hesitate to reach out, whether here or on the discord! |
What are the changes the user will see?
Background
There was a reported bug where if a player holds the amulet coin and completes a job for the part-timer mystery encounter, they will not receive the bonus. In other words, the rewards given from the mystery encounter are unaffected by the amulet coins benefit.
Why am I making these changes?
This PR addresses this issue by ensuring that after the money multiplier is applied, we'll also give a bonus for the amulet coin.
Fixes #5285
What are the changes from a developer perspective?
The only changes in this PR resides in part-time-encounter.ts where after the money multiplier has been determined, we then give the player a bonus for the amulet coin.
Screenshots/Videos
Note: I did not write unit tests for this change since this seems to be a minor bug fix and not game breaking
Set up
Reproducing the bug (before this commit)
Notice that the amulet coin has the same money reward
Without the amulet coin
With the amulet coin
Bug fix
Option 1
Without amulet coin
With amulet coin
Option 2
Without amulet coin
With amulet coin
Option 3
Without amulet coin
Note, I replaced this to require tackle in the moveset for the sake of testing.
With amulet coin
How to test the changes?
The testing description outlines how you can test this along with the issues.
To summarize,
Checklist
beta
as my base branchnpm run test
)npm run create-test
) or updated existing tests related to the PR's changes?Are there any localization additions or changes? If so: