From a297d90bb6cff91b85e487848ebcda3db7f7dd32 Mon Sep 17 00:00:00 2001 From: Idain Date: Wed, 9 Aug 2023 22:52:46 -0400 Subject: [PATCH 1/4] Document fix about AI item pointer overflowing --- docs/bugs_and_glitches.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index 6ccc1ffe915..fb0af94d769 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -59,6 +59,7 @@ Fixes in the [multi-player battle engine](#multi-player-battle-engine) category - [AI makes a false assumption about `CheckTypeMatchup`](#ai-makes-a-false-assumption-about-checktypematchup) - [AI use of Full Heal or Full Restore does not cure Nightmare status](#ai-use-of-full-heal-or-full-restore-does-not-cure-nightmare-status) - [AI use of Full Heal does not cure confusion status](#ai-use-of-full-heal-does-not-cure-confusion-status) + - [AI might use its base reward's value as an item](#ai-might-use-its-base-rewards-value-as-an-item) - [Wild Pokémon can always Teleport regardless of level difference](#wild-pok%C3%A9mon-can-always-teleport-regardless-of-level-difference) - [`RIVAL2` has lower DVs than `RIVAL1`](#rival2-has-lower-dvs-than-rival1) - [`HELD_CATCH_CHANCE` has no effect](#held_catch_chance-has-no-effect) @@ -1455,6 +1456,33 @@ Pryce's dialog ("That BADGE will raise the SPECIAL stats of POKéMON.") implies ret ``` +### AI might use its base reward's value as an item + +The routine in `AI_TryItem` usually rotates between `wEnemyTrainerItem1` and `wEnemyTrainerItem2` to check if either one of the AI's items is in the `AI_Items` list, but if the AI has used its first item (or it's `ITEM_NONE`) and doesn't use its second item, the AI's item pointer will overflow to the next address after `wEnemyTrainerItem2` (which is `wEnemyTrainerBaseReward`) and use its value as an item constant. If the base reward's value corresponds to a valid item in the `AI_Items` list, it might use it instead. + +**Fix:** Edit `AI_TryItem` in [engine/battle/ai/items.asm](https://github.com/pret/pokecrystal/blob/master/engine/battle/ai/items.asm): + +```diff + AI_TryItem: + ... + ld a, [wTrainerClass] + dec a + ld hl, TrainerClassAttributes + TRNATTR_AI_ITEM_SWITCH + ld bc, NUM_TRAINER_ATTRIBUTES + call AddNTimes + ld b, h + ld c, l + ld hl, AI_Items +-; BUG: AI might use its base reward's value as an item (see docs/bugs_and_glitches.md) +- ld de, wEnemyTrainerItem1 + .loop ++ ld de, wEnemyTrainerItem1 + ld a, [hl] + and a + inc a + ret z +``` + ### Wild Pokémon can always Teleport regardless of level difference From 41e97cb939940d8629d9555a0ca9fd48e8d6983d Mon Sep 17 00:00:00 2001 From: Idain Date: Wed, 9 Aug 2023 22:59:09 -0400 Subject: [PATCH 2/4] Forgot to insert bug comment in the code --- engine/battle/ai/items.asm | 1 + 1 file changed, 1 insertion(+) diff --git a/engine/battle/ai/items.asm b/engine/battle/ai/items.asm index 7e5c5ef5912..00dd5b837c7 100644 --- a/engine/battle/ai/items.asm +++ b/engine/battle/ai/items.asm @@ -175,6 +175,7 @@ AI_TryItem: ld b, h ld c, l ld hl, AI_Items +; BUG: AI might use its base reward's value as an item (see docs/bugs_and_glitches.md) ld de, wEnemyTrainerItem1 .loop ld a, [hl] From 68544856baa3a822fd41a9f05684e4aacc5ee62c Mon Sep 17 00:00:00 2001 From: Idain Date: Thu, 10 Aug 2023 12:49:34 -0400 Subject: [PATCH 3/4] Update docs/bugs_and_glitches.md Co-authored-by: vulcandth --- docs/bugs_and_glitches.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index fb0af94d769..e8f0acc8147 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -1458,7 +1458,7 @@ Pryce's dialog ("That BADGE will raise the SPECIAL stats of POKéMON.") implies ### AI might use its base reward's value as an item -The routine in `AI_TryItem` usually rotates between `wEnemyTrainerItem1` and `wEnemyTrainerItem2` to check if either one of the AI's items is in the `AI_Items` list, but if the AI has used its first item (or it's `ITEM_NONE`) and doesn't use its second item, the AI's item pointer will overflow to the next address after `wEnemyTrainerItem2` (which is `wEnemyTrainerBaseReward`) and use its value as an item constant. If the base reward's value corresponds to a valid item in the `AI_Items` list, it might use it instead. +In the `AI_TryItem` routine, the code checks both `wEnemyTrainerItem1` and `wEnemyTrainerItem2` to see if the AI's items match any in the `AI_Items` list. However, if the AI has used its first item (or it's set to `ITEM_NONE`) and doesn't use its second item, the item pointer can overflow to the subsequent address, `wEnemyTrainerBaseReward`. If this address's value matches an item in the `AI_Items` list, the AI could mistakenly use it. **Fix:** Edit `AI_TryItem` in [engine/battle/ai/items.asm](https://github.com/pret/pokecrystal/blob/master/engine/battle/ai/items.asm): From 7ba3c921ae46f49c8946a3770b0646b88308214d Mon Sep 17 00:00:00 2001 From: Rangi <35663410+Rangi42@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:25:59 -0400 Subject: [PATCH 4/4] Apply suggestions from code review --- docs/bugs_and_glitches.md | 8 ++++---- engine/battle/ai/items.asm | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/bugs_and_glitches.md b/docs/bugs_and_glitches.md index e8f0acc8147..b14568bdba8 100644 --- a/docs/bugs_and_glitches.md +++ b/docs/bugs_and_glitches.md @@ -59,7 +59,7 @@ Fixes in the [multi-player battle engine](#multi-player-battle-engine) category - [AI makes a false assumption about `CheckTypeMatchup`](#ai-makes-a-false-assumption-about-checktypematchup) - [AI use of Full Heal or Full Restore does not cure Nightmare status](#ai-use-of-full-heal-or-full-restore-does-not-cure-nightmare-status) - [AI use of Full Heal does not cure confusion status](#ai-use-of-full-heal-does-not-cure-confusion-status) - - [AI might use its base reward's value as an item](#ai-might-use-its-base-rewards-value-as-an-item) + - [AI might use its base reward value as an item](#ai-might-use-its-base-reward-value-as-an-item) - [Wild Pokémon can always Teleport regardless of level difference](#wild-pok%C3%A9mon-can-always-teleport-regardless-of-level-difference) - [`RIVAL2` has lower DVs than `RIVAL1`](#rival2-has-lower-dvs-than-rival1) - [`HELD_CATCH_CHANCE` has no effect](#held_catch_chance-has-no-effect) @@ -1456,9 +1456,9 @@ Pryce's dialog ("That BADGE will raise the SPECIAL stats of POKéMON.") implies ret ``` -### AI might use its base reward's value as an item +### AI might use its base reward value as an item -In the `AI_TryItem` routine, the code checks both `wEnemyTrainerItem1` and `wEnemyTrainerItem2` to see if the AI's items match any in the `AI_Items` list. However, if the AI has used its first item (or it's set to `ITEM_NONE`) and doesn't use its second item, the item pointer can overflow to the subsequent address, `wEnemyTrainerBaseReward`. If this address's value matches an item in the `AI_Items` list, the AI could mistakenly use it. +In the `AI_TryItem` routine, an item pointer is set to `wEnemyTrainerItem1` and then increments to `wEnemyTrainerItem2` to see if either of the AI's items are in the `AI_Items` list. However, if the AI has used its first item (or its first one is `ITEM_NONE`) and hasn't used its second item, the item pointer can increment from `wEnemyTrainerItem2` to `wEnemyTrainerBaseReward`. If the value at this address then matches an item in the `AI_Items` list, the AI could mistakenly use it. **Fix:** Edit `AI_TryItem` in [engine/battle/ai/items.asm](https://github.com/pret/pokecrystal/blob/master/engine/battle/ai/items.asm): @@ -1473,7 +1473,7 @@ In the `AI_TryItem` routine, the code checks both `wEnemyTrainerItem1` and `wEne ld b, h ld c, l ld hl, AI_Items --; BUG: AI might use its base reward's value as an item (see docs/bugs_and_glitches.md) +-; BUG: AI might use its base reward value as an item (see docs/bugs_and_glitches.md) - ld de, wEnemyTrainerItem1 .loop + ld de, wEnemyTrainerItem1 diff --git a/engine/battle/ai/items.asm b/engine/battle/ai/items.asm index 00dd5b837c7..3c0adb5f281 100644 --- a/engine/battle/ai/items.asm +++ b/engine/battle/ai/items.asm @@ -175,7 +175,7 @@ AI_TryItem: ld b, h ld c, l ld hl, AI_Items -; BUG: AI might use its base reward's value as an item (see docs/bugs_and_glitches.md) +; BUG: AI might use its base reward value as an item (see docs/bugs_and_glitches.md) ld de, wEnemyTrainerItem1 .loop ld a, [hl]