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

Adds Mortal Spin and Population Bomb #3178

Merged
merged 4 commits into from
Aug 7, 2023

Conversation

AlexOn1ine
Copy link
Collaborator

@AlexOn1ine AlexOn1ine commented Jul 26, 2023

Closes #2497 (Credit: Lunos)
Closes #2493

Copy link
Collaborator

@DizzyEggg DizzyEggg left a comment

Choose a reason for hiding this comment

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

There were some changes to Triple Kick and multi hit moves in general. I would feel more comfortable merging this PR if we got tests for Triple Kick and moves like Double Kick, Fury Swipes, etc. Also their interactions with Skill Link, when they can miss, when the can't miss, etc.

@AlexOn1ine
Copy link
Collaborator Author

AlexOn1ine commented Jul 28, 2023

Added Skill Link Tests for EFFECT_MULTI_HIT and some tiny logic for Loaded Dice.

I wasn't able to write tests for Triple Kick and Population Bomb because it doesn't seem you can check their accuracy correctly so @AsparagusEduardo suggested to just add the TO_DO_BATTLE_TEST

@AlexOn1ine
Copy link
Collaborator Author

The tests on github don't pass


- Tests FAILED :       2    Add TESTS='X' to run tests with the defined prefix.
  - Sand Veil increases evasion during sandstorm (1/?).
  - Snow Cloak increases evasion during hail (1/?).

but they pass for me 🤔

[0] Sand Veil prevents damage from sandstorm: PASS
[0] Sand Veil increases evasion during sandstorm: PASS
- Tests PASSED:        2
- Tests TOTAL:         2
- ```

@mrgriffin
Copy link
Collaborator

mrgriffin commented Jul 29, 2023

The tests on github don't pass


- Tests FAILED :       2    Add TESTS='X' to run tests with the defined prefix.
  - Sand Veil increases evasion during sandstorm (1/?).
  - Snow Cloak increases evasion during hail (1/?).

but they pass for me thinking

[0] Sand Veil prevents damage from sandstorm: PASS
[0] Sand Veil increases evasion during sandstorm: PASS
- Tests PASSED:        2
- Tests TOTAL:         2
- ```

If you run with make check -j2 do they fail for you? (Yay #3132)
You should get the same output as on CI. (Including the [0] and [1] things)

@AlexOn1ine
Copy link
Collaborator Author

The tests on github don't pass


- Tests FAILED :       2    Add TESTS='X' to run tests with the defined prefix.
  - Sand Veil increases evasion during sandstorm (1/?).
  - Snow Cloak increases evasion during hail (1/?).

but they pass for me thinking

[0] Sand Veil prevents damage from sandstorm: PASS
[0] Sand Veil increases evasion during sandstorm: PASS
- Tests PASSED:        2
- Tests TOTAL:         2
- ```

If you run with make check -j2 do they fail for you? (Yay #3132) You should get the same output as on CI. (Including the [0] and [1] things)

All good

- Tests PASSED:        480
- Tests KNOWN_FAILING: 11
- Tests TO_DO:         40
- Tests TOTAL:         531

fix for small bug introduced in the latest commit
DizzyEggg
DizzyEggg previously approved these changes Aug 7, 2023
@AsparagusEduardo AsparagusEduardo merged commit fc66a8c into rh-hideout:upcoming Aug 7, 2023
1 check passed
This was referenced Aug 7, 2023
@AlexOn1ine AlexOn1ine deleted the gen9_moves_part2 branch August 31, 2023 08:49
@AsparagusEduardo AsparagusEduardo mentioned this pull request Sep 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants