-
Notifications
You must be signed in to change notification settings - Fork 466
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
SFX: Support replaying the current BGM when changed #2150
SFX: Support replaying the current BGM when changed #2150
Conversation
void UpdateCurrentBGM(u16 seqKey, SeqType seqType) { | ||
if (seqType != SEQ_BGM_WORLD) { | ||
return; | ||
} | ||
|
||
u16 curSeqId = func_800FA0B4(SEQ_PLAYER_BGM_MAIN); | ||
if (curSeqId == seqKey) { | ||
ReplayCurrentBGM(); | ||
} | ||
} |
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 wonder if you really should only keep this one method and call Audio_QueueSeqCmd(0x00000000 | curSeqId);
in here.. In the two places where you do call ReplayCurrentBGM
you actually do the SEQ_BGM_WORLD
anyway.
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 do the same BGM_WORLD check, but those two areas are the reset all/randomizer all, that first do a loop to update all the options, then sets the CVARs at the end. So ReplayCurrentBGM
happens after the loop, and doesn't have the specific seqKey
to pass in.
I could move the those BGM_WORLD checks to also be in ReplayCurrentBgm
, or I could combine the two functions, but that would mean moving the function inside of that loop and having it check on each iteration, which is what I was trying to avoid. (it probably wouldn't have any performance impact, since it only happens on pressing a button)
We should wait to merge this in pending the next develop-bradley merge-in, as there is likely to a be a merge conflict here. Shouldn't be too tricky of one to resolve though. |
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.
This looks fine to me, pending the potential merge conflict around line 342 (which might not even happen now that I'm looking closer at it).
eb9721a
to
58fed1a
Compare
This makes it so that when the sequence for the "current" BGM is updated/reset, the audio player will replay the BGM to force the sfx override to take over.
Two functions where added, one that performs the replay, and one that checks when to execute it. The randomize/reset buttons for the line item and any change to the dropdown will check if that line item is the current sequence being played, and replay it. The randomizer all/reset all for the BGM world group will also do a replay.
Build Artifacts