jojobear13 / shinpokered

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

Bug: Instant text speed prevents TX_SFX from playing #259

Closed Mord5 closed 12 months ago

Mord5 commented 1 year ago

What emulator and/or hardware are you are using? Lemuroid for Android

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? When there's a text sound effect embedded in a string, it does not play if the text speed is set to instant; you only hear the text advance sound.

Expected behavior TX_SFX should be allowed to play fully before allowing the player to advance text.

Is it possible to reproduce the problem? Easy to see with Bill when he gives the SS Ticket - TX_SFX_KEY_ITEM should play as part of SSTicketReceivedText but does not.

jojobear13 commented 1 year ago

Known issue. Has to do with the fact that it is just a re-activation of the "0-frame text speed" bug from vanilla. Since this bug is so widely used by speedrunners and has been shown to be stable for many years, it was option-ized instead of being properly fixed and/or eliminated.

Currently unknown why this causes text sfx to not play.

Interesting. The text sfx is stuck on the wrong sound ID. SFX_KEY_ITEM is sound ID $94. But it's instead stuck on the TINK sfx value which is sound ID $90. It's not getting updated.

Looks like what's happening is this. Because there is zero delay between printing letters, the sound ID value that's loaded in the channel has no chance to update before the next sound ID is supposed to play. When it goes to try and play sound $94, it sees that sound $90 is already loaded in the channel and won't play. It thinks that $90 hasn't finished playing yet.

jojobear13 commented 1 year ago

fixed in commit 6b4ac8f784f4d7b958845d36dd9609f6b7802870

Mord5 commented 1 year ago

I checked it out and it seems like the fix works for the TX_SFX. Other SFX called differently are still obscured by the text-advance sfx. One that I noticed prominently was SFX_SHRINK during OakSpeech. Looks like tossing a call WaitForSoundToFinish on the line before https://github.com/jojobear13/shinpokered/blob/47cef1f8e3d478d93d0cd5282b57901c93bd1721/engine/oak_speech.asm#L197 fixes it but idk if this is the best solution.

Should I make a new issue for other obscured SFX, or are comments to this issue OK? (Or, is this getting into "not worth the effort" territory?)

jojobear13 commented 1 year ago

Let's keep this one open. I might have an idea to attack the problem at the source.

jojobear13 commented 1 year ago

Okay, let's give this solution in commit 8b4b647fda60b9c4b9b4e2f8e4a3154d5e89b064 a try. Here is how it works.

The main problem is that the TINK sfx from text boxes is still loaded and prevents new sfx from playing. Normally this is not a problem when there is a text delay because the TINK finishes before the next sfx needs to play. But when there is zero text delay, there is not enough time for the TINK to finish and unload from the sfx channels.

What I did was go into PrintLetterDelay and make it so that, if zero delay text is active, it checks to see if anything is loaded in the sfx channels. If so, it sets a flag to indicate that a sfx has played while printing text.

The next step is to go into the PlaySound function and check & reset that flag. If that flag was set, then you know you are trying to play a new sfx right after one that played during text-printing. Make a call to WaitForSoundToFinish to make sure the previous sfx has completely unloaded from the sfx channels before continuing with the next sound.

That should hopefully cover every scenario.