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

Battle Item Tests + critical fixes + new test macro #2940

Merged
merged 32 commits into from
May 7, 2023

Conversation

u8-Salem
Copy link

@u8-Salem u8-Salem commented Apr 21, 2023

Test for all the Battle items, categorized by their battle effects.
contributions are welcome.
Double checking the tests for plausability is also welcomed and probably needed.

Also add several fixes and new additions in relation to these tests:

  • Introduces a new test macro MoveWithPP that allows to specify the starting PP for a move.
    Example use: PLAYER(SPECIES_WOBBUFFET) { MovesWithPP({MOVE_TACKLE, 1}, {MOVE_CONFUSION, 20}); }
  • Includes a fix to make sure gChosenMovePos is correctly assigned for items that target a move
  • Includes critical fixes Fix USE_ITEM explicit targets #2933 and several fixes to battle items #2939

#2939 Description:

Fixes Heal Powder not using the correct battleUsage
Fixes Full Restore not using the correct BattleScript command to cure status

#2933 Description:

     if (ctx.explicitPartyIndex)
         gBattleStruct->itemPartyIndex[battlerId] = ctx.partyIndex;
     if (ctx.explicitMove)
         gBattleStruct->itemPartyIndex[battlerId] = i;

These are setting variables that get overwritten with PARTY_SIZE at the start of the turn. &gPlayerParty[6] == &gEnemyParty[0].

I have replaced them with writing to the battle record in the tests, and reading from it in the battle controllers.

We should really make the controllers emit the itemPartyIndex rather than writing to it directly. As-is, any recorded battles with an item usage or any link battles with an item usage will go horribly wrong.

TO-DO

  • HP Restoring Items
  • Status Curing Items
  • HP Restoring and Status Curing Items
  • Stat Increasing Items
  • Mist Setting Items
  • Focus Energy Setting Items
  • Reviving Items
  • PP Restoring Items
  • All Stats Increasing Items (note: dont have their own file. lumped together with stat increasing items)

Current Issues

  1. Full Heal curing confusion is broken but not detected in the test (should be fixed with several fixes to battle items #2939)
  2. Hyper Potion check passes when it shouldnt -> other hp restoring tests probably not correct aswell.
  3. Dire Hit test fails when "Wobbuffet vs Wobbuffet" because "Foe Wobbuffet" is displayed. Doesnt happen when "Wobbuffet vs Anything else" more context on discord
  4. Due to missing sanity checks in the replay/test system, the actual revive property of revive items can not be fully tested. the property "fainted" is not checked for which leads to revive items being practically the same as healing items (see discord)(vice versa healing items can "revive" mons).

Relevant GIFS

Dire Hit issue:
direhit direhit2

Issue(s) that this PR fixes

Fixes #2935
Fixes #2938

Discord contact info

Salem#3258

@u8-Salem u8-Salem changed the title [WIP] Battle Item Tests [WIP] Battle Item Tests + critical fixes Apr 24, 2023
@u8-Salem u8-Salem changed the title [WIP] Battle Item Tests + critical fixes [WIP] Battle Item Tests + critical fixes + new test macro Apr 25, 2023
@u8-Salem u8-Salem marked this pull request as ready for review April 25, 2023 21:22
@u8-Salem u8-Salem marked this pull request as draft April 25, 2023 21:23
thanks griffin <3

Co-Authored-By: Martin Griffin <838573+mrgriffin@users.noreply.github.com>
@u8-Salem u8-Salem marked this pull request as ready for review April 26, 2023 12:52
@u8-Salem
Copy link
Author

Ready for review. Any input on issue 3. would be highly appreciated as I dont have a the time to deep dive on this one rn.

@mrgriffin
Copy link
Collaborator

mrgriffin commented Apr 26, 2023

Ready for review. Any input on issue 3. would be highly appreciated as I dont have a the time to deep dive on this one rn.

idk if this is the problem, but:

static const u8 sText_PkmnUsedXToGetPumped[] = _("{B_SCR_ACTIVE_NAME_WITH_PREFIX} used\n{B_LAST_ITEM} to get pumped!");
void HandleAction_UseItem(void)
{
    gActiveBattler = gBattlerAttacker = gBattlerByTurnOrder[gCurrentTurnActionNumber];

We don't set gBattleScripting.battler, so we probably shouldn't expect any consistent behavior. But without reading the code more carefully I'm not sure if the fix here is to set that variable, or to create a new string that references e.g. the attacker, or what.

@AsparagusEduardo AsparagusEduardo marked this pull request as draft April 30, 2023 01:53
@AsparagusEduardo AsparagusEduardo marked this pull request as ready for review April 30, 2023 15:39
@u8-Salem u8-Salem changed the title [WIP] Battle Item Tests + critical fixes + new test macro Battle Item Tests + critical fixes + new test macro Apr 30, 2023
@Jaizu
Copy link

Jaizu commented Apr 30, 2023

Idk if tests support that but there are plenty of double battle bugs related to items, it would be nice to add tests for those too, I'm pretty sure some will fail based on opened issues.

@u8-Salem
Copy link
Author

Idk if tests support that but there are plenty of double battle bugs related to items, it would be nice to add tests for those too, I'm pretty sure some will fail based on opened issues.

Could you provide some examples of those issues so I can write some targeted tests? :)

@Jaizu
Copy link

Jaizu commented Apr 30, 2023

#2967
In that issue there are 2 issues listed as well

@u8-Salem
Copy link
Author

#2967
In that issue there are 2 issues listed as well

Thanks I will look into it

@u8-Salem
Copy link
Author

u8-Salem commented May 1, 2023

issue 3 fixed, pls review :)

@AsparagusEduardo AsparagusEduardo merged commit 4244551 into rh-hideout:upcoming May 7, 2023
@u8-Salem u8-Salem deleted the battleItemTests branch May 8, 2023 07:33
@AsparagusEduardo AsparagusEduardo mentioned this pull request May 31, 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.

Full Restore doesnt heal confusion Heal Powder is not working
4 participants