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

[Bug][Challenge][UI/UX] Exclude invalid starters when combining challenges #5509

Open
wants to merge 3 commits into
base: beta
Choose a base branch
from

Conversation

Wlowscha
Copy link
Contributor

Fixed version of #5463

Why am I making these changes?

Until now, some edge cases were not handled properly. For example, Scyther can never be valid in a gen 2 mono-flying challenge: its base form is gen 1 although flying, while its evolution is gen 2 but not flying.

Other similar reports are on discord: here and here

With the current setup, the code first checks if Scyther or any evolutions are valid for the Gen 2 challenge, then it checks if Scyther or any evolutions are valid for the mono-flying challenge. Since the answer to both questions is "yes", Scyther is marked as a selectable starter.

What are the changes from a developer perspective?

Moving some logic from challenges.ts to the starter select screen. Instead of letting each challenge check the validity of evolutions and forms, now it is the starter select screen which checks, for each evolution and form, whether it is valid in all challenges.

This includes cutting down code in the applyStarterChoice of the mono-gen and mono-type challenges, and also removing the soft parameter altogether. This parameter decides whether the validity of the starter should include its evolutions and battle forms. Since it was only used in the starter select screen, it is now handled there.

Screenshots/Videos

Scyther does not show up in Gen 2 mono-flying
gen2_flying

Eevee does not show up in Gen 4 mono-fairy
gen4_fairy

Snorunt does not show up in Gen 3 mono-ghost
gen3ghost

Rowlet does not show up in Gen 7 mono-fighting
gen7fight

How to test the changes?

Try out various combinations of challenges.

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 tested the changes manually?
  • Are all unit tests still passing? (npm run test)
    • Have I created new automated tests (npm run create-test) or updated existing tests related to the PR's changes?
  • Have I provided screenshots/videos of the changes (if applicable)?
    • Have I made sure that any UI change works for both UI themes (default and legacy)?

@Wlowscha Wlowscha requested a review from a team as a code owner March 12, 2025 00:04
@Wlowscha Wlowscha requested review from Xavion3, DayKev and xsn34kzx March 12, 2025 00:04
@Snailman11 Snailman11 added the P2 Bug Minor. Non crashing Incorrect move/ability/interaction label Mar 12, 2025
Copy link
Collaborator

@DayKev DayKev left a comment

Choose a reason for hiding this comment

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

✅ after the tsdocs are updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Bug Minor. Non crashing Incorrect move/ability/interaction
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

3 participants