jojobear13 / shinpokered

Mostly-vanilla hack of Pokémon Red/Blue focused on bugfixes and trainer ai
209 stars 42 forks source link

Bug: Gym Leaders playing Meet Trainer song #260

Closed Mord5 closed 12 months ago

Mord5 commented 1 year ago

What emulator and/or hardware are you are using? Lemuroid for Android & bgb 1.5.9.w64

What version of this project are you using? v1.23M built from source commit https://github.com/jojobear13/shinpokered/commit/cc28c3da068a883bdfe37406eb6251d467cc40e9

What is the problem you encountered? Function PlayTrainerMusic checks the value of wGymLeaderNo before loading MUSIC_MEET_*_TRAINER. However, nearly all Gym Leaders have wGymLeaderNo set after the call to EngageMapTrainer in their respective scripts (e.g. FuchsiaGymText1). As such, MUSIC_MEET_MALE_TRAINER plays before 7/8 Gym Leader battles plus the Elite Four.

(The exception to this is Blaine, since for him wGymLeaderNo is set by CinnabarGymText1, which calls the shared script CinnabarGymScript_758b7 that initiates all battles in his gym.)

Also, because the female Gym Leaders are not included in FemaleTrainerList, MUSIC_MEET_MALE_TRAINER rather than MUSIC_MEET_FEMALE_TRAINER plays for Misty, Erika, Sabrina, Lorelei, & Agatha.

Expected behavior The order of operations in PlayTrainerMusic shows that whoever wrote that function didn't want a "Meet" theme to play before Gym Leader battles. This is corroborated by the absence of female Gym Leaders from FemaleTrainerList. As such, I believe that the intended behavior is for no Gym Leader to play a Meet theme.

To solve this issue, I think the lines setting wGymLeaderNo should be moved to before the call to EngageMapTrainer in all relevant scripts. Using FuchsiaGymText1 as an example:

    call SaveEndBattleTextPointers
+   ld a, $5
+   ld [wGymLeaderNo], a
    ld a, [H_SPRITEINDEX]
    ld [wSpriteIndex], a
    call EngageMapTrainer
    call InitBattleEnemyParameters
-   ld a, $5
-   ld [wGymLeaderNo], a
    xor a

I think it's open to question as to whether Giovanni should have MUSIC_MEET_EVIL_TRAINER play before his battle in Viridian Gym.

jojobear13 commented 1 year ago

Looks like the MUSIC_MEET_MALE_TRAINER does not play when doing kanto gym leaders in Let's Go, HG/SS, and GSC. FR/LG is the only one that preserves it. I wonder why?

Mord5 commented 1 year ago

What does FRLG do with Blaine?

jojobear13 commented 1 year ago

What does FRLG do with Blaine?

Does the meeting jingle unlike in the originals. https://youtu.be/MGYRulCAjDk?t=8215

Mord5 commented 1 year ago

As I see it there are 2 options for resolving this inconsistent behavior: 1) remove the Meet theme from all trainers that set a Gym Leader number, or 2) add the Meet theme to Blaine and add the female trainers to the female list.

The evidence in the code convinced me that in the mid-90s GF intended to implement option 1, regardless of their choices in later gens and remakes.

jojobear13 commented 1 year ago

Agreed on option 1. Done in Lite commit 5c8b654e4c10597eacb285e0c0b224ae48211a58.

I've decided that the jingle will only be allowed to play when loading into the regular trainer battle theme. This means that the jingle will still play for the elite 4 in the Lite branch. I've decided to not add Lorelei and Agatha to the female trainer jingle list because they aren't presented as "girly" like the female meeting jingle implies.

As for the Master branch, the meeting jingle will not play for the elite 4 because they load into the gym leader theme. Adjusted in commit b2186bb6820c0cdb2297f67eed6b2b453c52ca75