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

[Dev] Add overrides for alternating between single and double battles #4833

Merged
merged 8 commits into from
Nov 14, 2024

Conversation

PigeonBar
Copy link
Collaborator

What are the changes the user will see?

BATTLE_TYPE_OVERRIDE: "even-doubles", - Double battles on even-numbered waves, single battles on odd-numbered waves.
BATTLE_TYPE_OVERRIDE: "odd-doubles", - Double battles on odd-numbered waves, single battles on even-numbered waves.

Ideally, no changes if not using overrides.

Why am I making these changes?

This will help with testing transitions between single and double battles. For example, it will help with testing the newly implemented Commander (#4670).

(I am adding both "even-doubles" and "odd-doubles" so that, for example, we can force a X0 -> X1 biome change to be either a singles -> doubles or doubles -> singles transition.)

What are the changes from a developer perspective?

  1. Added code for handling the added override options.
  2. Added a test to double-check that these changes do not accidentally affect regular (non-overrides) gameplay.

Screenshots/Videos

even-doubles
20241109.Even.Doubles.mp4
odd-doubles
20241109.Odd.Doubles.mp4

How to test the changes?

Use the above overrides in overrides.ts.
npm run test double_battle.test.ts

Checklist

  • I'm using beta as my base branch
  • 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 considered writing automated tests for the issue?
  • [ ] If I have text, did I make it translatable and add a key in the English locale file(s)?
  • 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?

Copy link
Collaborator

@MokaStitcher MokaStitcher left a comment

Choose a reason for hiding this comment

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

haven't playtested but code lgtm

src/battle-scene.ts Outdated Show resolved Hide resolved
DayKev
DayKev previously approved these changes Nov 9, 2024
src/overrides.ts Outdated Show resolved Hide resolved
src/overrides.ts Outdated Show resolved Hide resolved
@PigeonBar

This comment was marked as outdated.

@torranx
Copy link
Collaborator

torranx commented Nov 11, 2024

DoubleType is now SingleOrDoubleType, hope everyone's happy now

TBH, I still prefer BattleMode or the one Moka mentioned, BattleStyle. SingleOrDoubleType to me is like naming DayOrNight for day and night types which I personally don't like

EDIT: if we decide to keep the SingleOrDoubleType, at least remove the "Type" from the name 👍

@DayKev
Copy link
Collaborator

DayKev commented Nov 11, 2024

Since BattleType is unfortunately already taken, BattleStyle seems good.

@Madmadness65 Madmadness65 added the Miscellaneous Changes that don't fit under any other label label Nov 11, 2024
@DayKev DayKev added the Tests Automated tests related label Nov 13, 2024
Copy link
Collaborator

@torranx torranx left a comment

Choose a reason for hiding this comment

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

lgtm, would love to see another pr that changes the battyleType override to battleStyle for consistency

@MokaStitcher MokaStitcher merged commit 640ac23 into pagefaultgames:beta Nov 14, 2024
13 checks passed
@PigeonBar PigeonBar deleted the battle-type-overrides branch November 15, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Miscellaneous Changes that don't fit under any other label Tests Automated tests related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants