Add Starting Songs Option#2221
Conversation
|
One workaround for this, would be to do it the same way the other starting items, like Kokiri Sword and Deku Shield are and just write each song name as "Start with Song Name" and then just set the submenu back to false. The only issue with this would be that in the in game menu, every song would be written like that, which isn't a huge deal, but a little redundant and cluttered, since it's under the "Starting Songs" column. |
| for (std::vector<Option*>* menu : startingInventoryOptions) { | ||
| for (size_t i = 0; i < menu->size(); ++i) { | ||
| const auto setting = menu->at(i); | ||
| // Starting Songs | ||
| if (setting->GetName() == "Zelda's Lullaby" || | ||
| setting->GetName() == "Epona's Song" || | ||
| setting->GetName() == "Saria's Song" || | ||
| setting->GetName() == "Sun's Song" || | ||
| setting->GetName() == "Song of Time" || | ||
| setting->GetName() == "Song of Storms" || | ||
| setting->GetName() == "Minuet of Forest" || | ||
| setting->GetName() == "Bolero of Fire" || | ||
| setting->GetName() == "Serenade of Water" || | ||
| setting->GetName() == "Requiem of Spirit" || | ||
| setting->GetName() == "Nocturne of Shadow" || | ||
| setting->GetName() == "Prelude of Light") { | ||
| jsonData["settings"][setting->GetName()] = setting->GetSelectedOptionText(); | ||
| } |
There was a problem hiding this comment.
How come this loop is made seperately from the loop below?
| setting->GetName() == "Gold Skulltula Tokens" || | ||
| setting->GetName() == "Start with Fairy Ocarina" || | ||
| setting->GetName() == "Start with Kokiri Sword" || | ||
| setting->GetName() == "Start with Kokiri Sword" || |
There was a problem hiding this comment.
nitpick: should probably trim the space here
|
|
||
| Menu startingItems = Menu::SubMenu("Items", &startingItemsOptions, false); | ||
| Menu startingSongs = Menu::SubMenu("Ocarina Songs", &startingSongsOptions, false); | ||
| Menu startingSongs = Menu::SubMenu("Starting Songs", &startingSongsOptions, true); |
There was a problem hiding this comment.
This menu isn't necessarily for us, but rather for 3d rando to display the options available for the player. (You already exposed the options through our ImGui implementation.)
To answer your question in the OP, you're printing twice since the true here refers whether to print to the spoiler log.
There was a problem hiding this comment.
Yeah, I realize that, but without using their method, it either writes all the songs unorganized in the spoiler log, or I have to change all the names to "start with 'song name'" Which, either way is fine, but I'm just not sure what would be preferred. Using their sub menu makes it cleaner everywhere, minus the fact that it results in a double entry
There was a problem hiding this comment.
Personally, I would find it consistent with all the other options if starting songs began with "Start With". I wonder if doing this and removing the loop I mentioned in this review will a) have a single set of song options and b) organize the result.
There was a problem hiding this comment.
I did it before, and if I remember correctly it did organize it in the spoiler log. not sure what you mean by single set of song options though
There was a problem hiding this comment.
You mentioned a double entry somewhere? Just wanted to make sure we were on the same page.
There was a problem hiding this comment.
Oh yes, my bad. Yeah, it'll definitely do both those things.
There was a problem hiding this comment.
That's what the menu looks like now with the changes and only one entry in the spoiler log for each song.
Archez
left a comment
There was a problem hiding this comment.
Can you add the new CVars you made into the randomizerCvars list in soh/soh/Enhancements/presets.h, sorted alphabetically. This makes it so applying the "Default" preset will clear out the starting songs.
Clears out song when "Default" preset is applied
|
@Archez Done. And thank you for being detailed in you comment, really made it easy for a noob like me! |
This reverts commit 38d5d32.
05bd4a3 to
ba13e6b
Compare
This reverts commit a96d1a0.
|
@Patrick12115 Looks like something went wrong with one of the merges from develop branch. Your |
|
@Archez Yeah, I'm not sure what happened there, but I THINK, I got it all fixed |
Adds starting songs to the randomizer menu
Currently it is created two entries in the spoiler log, one under the submenu "Starting Songs" and one with just the song name.
If anyone could point me in the direction of excluding the ones that are just song names, I would very much appreciate it. This is my first PR ever, so please let me know if I did anything wrong, still learning and trying to get better.
Build Artifacts