-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
added ability to set flags in individual tests #3781
Conversation
@@ -839,6 +841,8 @@ struct moveWithPP { | |||
#define Status1(status1) Status1_(__LINE__, status1) | |||
#define OTName(otName) do {static const u8 otName_[] = _(otName); OTName_(__LINE__, otName_);} while (0) | |||
|
|||
#define FLAG_SET(flagId) do { FlagSet(flagId); gBattleTestRunnerState->data.setFlag = flagId; } while (0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd move the implementation into test/test_runner_battle.h
, where you could take advantage of INVALID_IF
to report an error if FLAG_SET
is called twice in the same test (i.e. called with gBattleTestRunnerState->data.setFlag != 0
).
EDIT: As-is, the subsequent calls will succeed, but only the last call's flag will be reset.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could you document FLAG_SET
at the top of this file, including how it works with PARAMETRIZE
/PASSES_RANDOMLY
.
I'm not sure that how it works with PARAMETRIZE
is particularly intuitive—I think the flag stays set for all later PARAMETRIZE
s, unless they happen to use PASSES_RANDOMLY
in which case the flag is reset after every trial? It's probably worth adding flag clearing to CB2_BattleTest_NextParameter
.
if (DATA.setFlag != 0) { | ||
FlagClear(DATA.setFlag); | ||
DATA.setFlag = 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! I think TearDownBattle
doesn't always run? It seems it's only done when a test fails unexpectedly (because that's what sets STATE->tearDownBattle = TRUE
), or between PASSES_RANDOMLY
trials?
This logic probably wants to be in its own function which is called from BattleTest_TearDown
, CB2_BattleTest_NextParameter
, and CB2_BattleTest_NextTrial
?
Btw, this should go into upcoming since it's a new feature. |
Closed in favour of #3786 |
Description
Its real quick and dirty. But if its stupid and it works, its not stupid.
Just added a macro the same as the PLAYER/OPPONENT macros that lets you input a flag into FLAG_SET, like so:
The flags ID is stored in the BattleTestData struct and is accessed by the TearDownBattle function in order to actually remove it at the end of the test.
Issue(s) that this PR fixes
None currently. But if you want to change the experience share away from build flags, it might be useful.
Also good if expansion ever adds level caps for a similar reason.
We can just check way more stuff. It'll be cool. Trust me.
Discord contact info
Zatsoo