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

[Enhancement] Add Fee Waiving Overrides #2266

Merged
merged 1 commit into from
Jul 30, 2024

Conversation

xsn34kzx
Copy link
Collaborator

What are the changes?

Adds two new boolean overrides: WAIVE_SHOP_FEES_OVERRIDE and WAIVE_REROLL_FEE_OVERRIDE.

Why am I doing these changes?

Whenever I'm testing item weights for really rare items and making sure that they actually appear in a "normal" reward fetching context outside of STARTING_HELD_ITEMS_OVERRIDE and ITEM_REWARD_OVERRIDE, I end up having to either use an all shiny team to bolster luck or set STARTING_MONEY_OVERRIDE to a really large number. This usually works if luck comes through, but because reroll prices exponentially grow, you end up running out of money pretty fast and then having to restart to get the STARTING_MONEY_OVERRIDE to kick in again. The reroll override directly addresses this by keeping the reroll price at 0 forever, which was the primary motivation.

Alongside the reroll override, I figured I would bundle this with a shop fee override since it also pertains to test runs in general, making it easier to do a full run for testing purposes without having to necessarily play a real run of Classic. In a similar sense, it helps gauge what items can be realistically be obtained in a mostly normal run and has applications in unit tests, removing the necessity to coordinate money and shop prices with waves or pad values to account for swings in RNG.

What did change?

Along with the two override variables, checks and ternaries for these variables were added to the SelectModifierPhase at the appropriate points to make sure that cost is 0 for the corresponding things. For reroll pricing, this involves directly placing the override in getRerollCost and, for visual purposes, surrounding the animation and cost subtraction code in an if. For shop pricing though, a lot of logic already relies on cost being 0 or null; that is, anything with a cost of 0 or null is treated as a modifier roll, which makes picking up a truly free shop item turn into picking up a modifier and moving immediately into the next wave. As a result, cost isn't mutated at all. Instead, the cost subtraction is wrapped in an if and the updateCostText is changed to display 0 rather than the actual cost of the shop item. Both of these things give the effect of the shop item cost being 0 but doesn't actually change the shop item value internally.

Screenshots/Videos

Reroll and Shop Prices - Before

rerollBefore

Reroll and Shop Prices - After (w/ Overrides set to true)

waiveFeesDemo.mov

How to test the changes?

All that's needed to test is to set either of the new override variables, WAIVE_SHOP_FEES_OVERRIDE or WAIVE_REROLL_FEE_OVERRIDE, to true in src/overrides.ts, going into an existing or new run, winning a wave to enter the SelectModifierPhase, and try buying any item or rerolling.

Checklist

  • There is no overlap with another PR?
  • The PR is self-contained and cannot be split into smaller PRs?
  • Have I provided a clear explanation of the changes?
  • Have I tested the changes (manually)?
    • Are all unit tests still passing? (npm run test)
  • Are the changes visual?
    • Have I provided screenshots/videos of the changes?

@Tempo-anon Tempo-anon added the Enhancement New feature or request label Jun 16, 2024
@f-fsantos f-fsantos changed the base branch from main to beta July 10, 2024 17:47
Copy link
Collaborator

@CodeTappert CodeTappert left a comment

Choose a reason for hiding this comment

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

Looks good to me

@@ -5221,7 +5223,7 @@ export class SelectModifierPhase extends BattlePhase {
break;
}

if (cost && this.scene.money < cost) {
if (cost && (this.scene.money < cost) && !Overrides.WAIVE_ROLL_FEE_OVERRIDE) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NIT: what's the benefit of the brackets around

(this.scene.money < cost)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I forgot when, but I ran into an issue a while ago where, instead of the boolean operation being evaluated on the right-hand side, the variable (for example, this.scene.money) was checked as falsy or not directly. I didn't happen here (as far as I know), but I'm pretty sure I did this out of habit to make every boolean in the condition more explicit / avoid the issue from before.

@Tempo-anon Tempo-anon merged commit 2900f22 into pagefaultgames:beta Jul 30, 2024
4 checks passed
@xsn34kzx xsn34kzx deleted the waive_fees branch August 8, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants