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

[Move][Checkup] Present RNG Fixes #337

Merged
merged 11 commits into from
Jan 27, 2025
Merged

[Move][Checkup] Present RNG Fixes #337

merged 11 commits into from
Jan 27, 2025

Conversation

frutescens
Copy link
Contributor

@frutescens frutescens commented Jan 21, 2025

What are the changes the user will see?

Present will no longer use global RNG when it is used.
Present will also no longer damage the target for 1 HP before healing the target.
Present's outcomes were shifted by 1 before. This PR fixes that so that the chances of each of the 4 outcomes are accurate to mainline.

Why am I making these changes?

Move Checkups.
#313

What are the changes from a developer perspective?

firstHit variable renamed to isFirstHit
Present-related test removed from Triage since its unnecessary.
powerSeed generated with user.randSeedInt()
Fixed off-by-1 Present RNG

How to test the changes?

Using Present should not affect the events of the next wave.

Screen.Recording.2025-01-23.at.11.52.34.AM.mov

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:silent)
    • Have I created new automated tests (npm run create-test) or updated existing tests related to the PR's changes?

@frutescens frutescens added P2 Bug Minor bug. Used for when a move/ability/interaction is incorrect but is not game breaking. Checkup Part of an effort to ensure that moves/abilities/game features are functioning correctly. labels Jan 21, 2025
@frutescens frutescens linked an issue Jan 21, 2025 that may be closed by this pull request
8 tasks
Tempo-anon
Tempo-anon previously approved these changes Jan 21, 2025
src/data/move-attrs/present-power-attr.ts Outdated Show resolved Hide resolved
src/data/move-attrs/present-power-attr.ts Outdated Show resolved Hide resolved
src/data/move-attrs/present-power-attr.ts Show resolved Hide resolved
Tempo-anon
Tempo-anon previously approved these changes Jan 22, 2025
torranx
torranx previously approved these changes Jan 22, 2025
@frutescens frutescens dismissed stale reviews from torranx and Tempo-anon via 114fd18 January 23, 2025 00:21
@PigeonBar

This comment was marked as resolved.

@Tempo-anon
Copy link
Contributor

Probably caused because of toDmgValue making all attacks do a min of 1 damage. Does the 1 tick of damage happen before or after the healing applies?

@innerthunder
Copy link
Contributor

Probably caused because of toDmgValue making all attacks do a min of 1 damage. Does the 1 tick of damage happen before or after the healing applies?

The video seems to show it happens after (which makes sense -- the heal phase is pushed when move power is resolved and before damage is finalized/rounded up)

@PigeonBar

This comment was marked as resolved.

@PigeonBar

This comment was marked as resolved.

@innerthunder
Copy link
Contributor

I think a better way to fix it would be to somehow stop MoveEffectPhase.applyMove early if baseDamage <= 0 (Present's base power is -1 internally, and doesn't get overridden if the move ends up healing)

@frutescens frutescens mentioned this pull request Jan 23, 2025
8 tasks
flx-sta
flx-sta previously approved these changes Jan 24, 2025
@flx-sta flx-sta self-requested a review January 24, 2025 04:21
@flx-sta flx-sta dismissed their stale review January 24, 2025 04:21

See pigeons comment

Copy link
Contributor

@flx-sta flx-sta left a comment

Choose a reason for hiding this comment

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

Heal & Damage at the same time

Screen.Recording.2025-01-23.at.8.42.12.PM.mov
[APPLIED CHANGES FOR TESTING]

I had this

.attr(
        ChangeToStatusCategoryAttr,
        (user, _target, _move) => user.turnData.hitCount === user.turnData.hitsLeft && user.randSeedInt(100) < 20,
        (user, _target, move) => {
          user.turnData.hitCount = 1;
          user.turnData.hitsLeft = 1;
          move.addAttr(new HealAttr(0.25, true, false));
        },
      )

replaced with

.attr(
        ChangeToStatusCategoryAttr,
        (user, _target, _move) => user.turnData.hitCount === user.turnData.hitsLeft && user.randSeedInt(100) < 50, // ⚠️
        (user, _target, move) => {
          user.turnData.hitCount = 1;
          user.turnData.hitsLeft = 1;
          move.addAttr(new HealAttr(0.25, true, false));
        },
      )

@DayKev DayKev added the Move For move implementations/interactions label Jan 24, 2025
@frutescens
Copy link
Contributor Author

Very confused. I tested it with Singles and Doubles.

I changed
image

Screen.Recording.2025-01-23.at.9.29.09.PM.mov

src/data/move-attrs/change-to-status-category-attr.ts Outdated Show resolved Hide resolved
src/data/all-moves.ts Outdated Show resolved Hide resolved
@PigeonBar
Copy link
Contributor

PigeonBar commented Jan 25, 2025

I also recommend adding a if (power < 0) {return -1} to the function getBaseDamage().

Since the formula for base damage adds 2 damage at the end, it is possible under extreme circumstances for a move with -1 power to deal positive damage.

PigeonBar
PigeonBar previously approved these changes Jan 26, 2025
Tempo-anon
Tempo-anon previously approved these changes Jan 26, 2025
Copy link
Contributor

@Tempo-anon Tempo-anon 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, just some styling comments

src/data/move-attrs/present-power-attr.ts Outdated Show resolved Hide resolved
src/data/move-attrs/present-power-attr.ts Outdated Show resolved Hide resolved
src/field/pokemon.ts Show resolved Hide resolved
src/field/pokemon.ts Show resolved Hide resolved
@PigeonBar PigeonBar mentioned this pull request Jan 26, 2025
6 tasks
@frutescens frutescens dismissed stale reviews from Tempo-anon and PigeonBar via a3eaa7e January 26, 2025 23:59
PigeonBar
PigeonBar previously approved these changes Jan 27, 2025
torranx
torranx previously approved these changes Jan 27, 2025
@frutescens frutescens dismissed stale reviews from torranx and PigeonBar via 7b1bd77 January 27, 2025 17:13
@flx-sta flx-sta merged commit 8fdd829 into beta Jan 27, 2025
7 checks passed
@flx-sta flx-sta deleted the presentCheckup branch January 27, 2025 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Checkup Part of an effort to ensure that moves/abilities/game features are functioning correctly. Move For move implementations/interactions P2 Bug Minor bug. Used for when a move/ability/interaction is incorrect but is not game breaking.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Move][Checkup] Present
7 participants