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.

Sign up for GitHub

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

Jump to bottom

Added missing Move Effect TODO tests - Volume E #5915

Merged
merged 5 commits into from
Jan 1, 2025
Merged

Added missing Move Effect TODO tests - Volume E #5915

merged 5 commits into from
Jan 1, 2025

Conversation

AsparagusEduardo
Copy link
Collaborator

Description

Handled all move effects that start with a E.

Other notable stuff:

  • Moved Terrain tests to move effect folders.
    • Added surge ability test files.
    • Added Terrain seeds test file.
  • Moved Mimicry tests to its own file.
  • Moved Encore/Dynamax tests to Encore file
  • Parametrized Encore tests
  • Added theorical Evasion up 2 test

Discord contact info

AsparagusEduardo

@AlexOn1ine AlexOn1ine added the category: battle-tests Related to the automated test environment label Dec 31, 2024
Bassoonian
Bassoonian reviewed Dec 31, 2024
View reviewed changes
test/battle/move_effect/evasion_up_2.c Outdated
#include "global.h"
#include "test/battle.h"

// There's no move with EFFECT_EVASION_UP_2 effect. Below is a theorical test.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// There's no move with EFFECT_EVASION_UP_2 effect. Below is a theorical test.
// There's no move with EFFECT_EVASION_UP_2 effect. Below is a theoretical test.

I'm not sure if there's a point in these though, we barely cover all non-theoretical cases and doing this sets a precedent that may make people expect us to write tests for their hypothetical new effects, too

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we remove this unused move effect then? I believe it's from vanilla

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's highly unlikely to be introduced in (vanilla) Pokémon at any point, so fine by me

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we keep it? Since it is already there, there is no harm in keeping it. It might also be an effect users used for their custom moves.

Though I think we should not introduce custom effects

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did some digging and indeed EFFECT_EVASION_UP_2 is from vanilla.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://github.com/pret/pokeemerald/blob/50d325f081a161f4a223d999497d7a65bd896194/include/constants/battle_move_effects.h#L60

@Bassoonian Bassoonian merged commit 55f0d3a into rh-hideout:master Jan 1, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexOn1ine AlexOn1ine AlexOn1ine left review comments

@Bassoonian Bassoonian Bassoonian approved these changes

Assignees
No one assigned
Labels
category: battle-tests Related to the automated test environment
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AsparagusEduardo @Bassoonian @AlexOn1ine