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

Fix: SFX only save authentic sequence cvars #2216

Merged

Conversation

Archez
Copy link
Contributor

@Archez Archez commented Dec 20, 2022

The randomize all/reset all buttons in the SFX editor would save custom sequence names to CVars (specifically the first BGM, and all fanfares). This does not have an immediate impact on the game, but I'm not sure if certain characters in a sequence name could cause the json to get corrupted.

This PR prevents non-authentic sequence names from getting save to the json.

Here is an example of what the shipofharkinian.json would look like:

"gSfxEditor_Super_Mario_Bros_Deluxe_-_Flag_Fanfare": 605,
"gSfxEditor_Super_Smash_Bros": {
    "_Melee_-_Ice_Climbers_Victory_Theme": 632,
    "_Melee_-_Stage_Clear_2": 634
},
"gSfxEditor_TLoZ_A_Link_to_the_Past_-_Ganon_Transform_Fanfare": 644,
"gSfxEditor_TLoZ_A_Link_to_the_Past_-_Ocarina_": 647,
"gSfxEditor_TLoZ_A_Link_to_the_Past_-_Victory_Fanfare": 650,
"gSfxEditor_TLoZ_Majora's_Mask_-_Elegy_of_Emptiness_(SPT)": 659,
"gSfxEditor_TLoZ_Majora's_Mask_-_Goron_Lullaby_(SPT)": 663,
"gSfxEditor_TLoZ_Majora's_Mask_-_Mask_Get_(SPT)": 666,
"gSfxEditor_TLoZ_Majora's_Mask_-_New_Wave_Bossa_Nova_(SPT)": 671,
"gSfxEditor_TLoZ_Majora's_Mask_-_Oath_to_Order_(SPT)": 672,
"gSfxEditor_TLoZ_Majora's_Mask_-_Sonata_of_Awakening_(SPT)": 675,
"gSfxEditor_TLoZ_Majora's_Mask_-_Song_of_Double_Time_(SPT)": 676,
"gSfxEditor_TLoZ_Majora's_Mask_-_Song_of_Healing_(SPT)": 677,
"gSfxEditor_TLoZ_Majora's_Mask_-_Song_of_Inverted_Time_(SPT)": 678,
"gSfxEditor_TLoZ_Majora's_Mask_-_Song_of_Soaring_(SPT)": 679,
"gSfxEditor_The_Legend_of_Zelda_the_Windwaker_-_Wind's_Aria": 736,
"gSfxEditor_The_Lick_(Requiem_of_Spirit_Ver": {
    ")": 738
},
"gSfxEditor_Twilight_Princess_-_Learn_Skill": 749,

Build Artifacts

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! left one little comment with a question

@@ -208,7 +212,8 @@ void Draw_SfxTab(const std::string& tabId, const std::map<u16, std::tuple<std::s
const auto& [name, sfxKey, seqType] = seqData;
const std::string cvarKey = "gSfxEditor_" + sfxKey;
if (seqType & type) {
if (((seqType & SEQ_BGM_CUSTOM) || seqType == SEQ_FANFARE) && defaultValue > MAX_AUTHENTIC_SEQID) {
// Only save authentic sequence CVars
if (((seqType & SEQ_BGM_CUSTOM) || seqType == SEQ_FANFARE) && defaultValue >= MAX_AUTHENTIC_SEQID) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just want to confirm the old > was a mistake and >= is what we want here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is a second instance below that has the >=

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, the naming is a little confusing, it's not clear that the authentic sequence with the highest id is 109 not 110 from this

#define MAX_AUTHENTIC_SEQID 110

the only place that's documented is here
int startingSeqNum = MAX_AUTHENTIC_SEQID; // 109 is the highest vanilla sequence

@briaguya-ai briaguya-ai merged commit 6f7361e into HarbourMasters:develop-bradley Dec 20, 2022
@Archez Archez deleted the fix-sfx-cvar-save branch December 21, 2022 01:20
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.

None yet

2 participants