Feature/split songs by RaccoonCloud · Pull Request #6627 · HarbourMasters/Shipwright

RaccoonCloud · 2026-05-17T19:07:44Z

Summary

Adds a randomizer option to split each ocarina song into two progressive pickups per song, using RandInf + the existing progressive item system (same pattern as child wallet / strength upgrades). Follow-up to #6561 with review feedback addressed.

Each of the 12 songs uses RG_PROGRESSIVE_* pool entries (doubled per song for softlock safety)
First pickup sets a per-song RandInf flag; second pickup resolves to the full song through normal GetItem / progressive flow
Requires Shuffle Songs: Anywhere (enforced in FinalizeSettings)
Pickup textbox icons show the correct colored 16×24 song note for progressive pickups
Item tracker shows 1/2 progress on the songs row when split songs are enabled
UTF-8 sanitization for spoiler/save JSON stability; Linux CI build fix included
Merged latest upstream develop

Test plan

Generate seeds with Split Ocarina Songs enabled (Shuffle Songs: Anywhere, Plentiful if desired)
Pick up first progressive song pickup → verify part flag / 1/2 tracker / correct textbox icon
Pick up second progressive pickup for same song → verify full song granted with correct name and icon
Try logic seeds and ice trap disguises on progressive song items — no seed-gen crash
Confirm option is disabled/cleared when Shuffle Songs is not Anywhere

Created by RaccoonCloud — inspired by MM Goron Lullaby split song.

Credit and thanks to the HM64 Team.

Special thank you for your friendship, support, and memories over the years, and for making SoH great:

Caladius, Proxysaw, aMannus, itsHeckinPat, Fredomato, Leggettc18, MoonlitxShadows, CardinalNerd, Kenix, Smiffic, SirMagicPenguin, alwayszchartergirl, Nordic Ryan, Grimey, Scorched11, AGreenSpoon, Mellar, PapChiefo, OneSaltyGinger, Pepper0ni

This introduces song part items, logic/state tracking, and UI/runtime integration so two parts unlock each full song while preserving stable randomizer behavior in Anywhere mode.

Replaces the previous copyright wording in split_songs.h and split_songs.cpp with a simple Created by RaccoonCloud header line.

Made split songs to use progressive item flow similar to Bomb bags and Strengh Upgardes etc Replaces discrete song part pool entries with progressive song pickups while preserving part-flag progression, song completion behavior, and logic/obtainability handling for split song CURRENTLY ONLY 2 PARTS!!

Reworded the option in UI in rando menu to explain split songs now use two progressive pickups per song and keeps the Anywhere requirement messaging.

Updated get-item textbox icon handling so OTR paths load and draw correctly on pickup (including custom icons). Song icons use the correct texture format, sizing, and per-song colors. Songs are fully progressive in randomizer logic; seed/shuffle paths allow more than two song pieces in the pool so duplicate parts and edge cases do not softlock. Also: sync player get-item entry for message context, resolve gift row when entry and id disagree, custom icons for blue potion and key rings, and ice trap naming for split song models.

All pickup textbox icons now work correctly for progressive songs and split song parts, with the correct note icon and per-song color for each song.

Pepper0ni

Quick initial review to call out some of the more obvious code style problems, You don't need 2 parts anymore and there's a lot of cruft that could be cut down on as it should simply be a progressive that checks a RandInf, gives an item that sets it to true if it's not, and gives the song if it already is.

Pepper0ni · 2026-05-31T18:06:11Z

    itemTable[RG_BOTTLE_WITH_RED_POTION] =              Item(RG_BOTTLE_WITH_RED_POTION,           Text{ "Bottle with Red Potion", "Bouteille avec une Potion Rouge", "Flasche mit rotem Elixier" },                                    ITEMTYPE_ITEM,              0x8C,                 true,  LOGIC_BOTTLES,                      RHT_BOTTLE_WITH_RED_POTION,            RG_BOTTLE_WITH_RED_POTION,            OBJECT_GI_LIQUID,        GID_POTION_RED,      TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER, {"a ", "une ", "eine "});
    itemTable[RG_BOTTLE_WITH_GREEN_POTION] =            Item(RG_BOTTLE_WITH_GREEN_POTION,         Text{ "Bottle with Green Potion", "Bouteille avec une Potion Verte", "Flasche mit grünem Elixier" },                                 ITEMTYPE_ITEM,              0x8D,                 true,  LOGIC_BOTTLES,                      RHT_BOTTLE_WITH_GREEN_POTION,          RG_BOTTLE_WITH_GREEN_POTION,          OBJECT_GI_LIQUID,        GID_POTION_GREEN,    TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER, {"a ", "une ", "eine "});
-    itemTable[RG_BOTTLE_WITH_BLUE_POTION] =             Item(RG_BOTTLE_WITH_BLUE_POTION,          Text{ "Bottle with Blue Potion", "Bouteille avec une Potion Bleue", "Flasche mit blauem Elixier" },                                  ITEMTYPE_ITEM,              0x8E,                 true,  LOGIC_BOTTLES,                      RHT_BOTTLE_WITH_BLUE_POTION,           RG_BOTTLE_WITH_BLUE_POTION,           OBJECT_GI_LIQUID,        GID_POTION_BLUE,     TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER, {"a ", "une ", "eine "});
+    itemTable[RG_BOTTLE_WITH_BLUE_POTION] =             Item(RG_BOTTLE_WITH_BLUE_POTION,          Text{ "Bottle with Blue Potion", "Bouteille avec une Potion Bleue", "Flasche mit blauem Elixier" },                                  ITEMTYPE_ITEM,              0x8E,                 true,  LOGIC_BOTTLES,                      RHT_BOTTLE_WITH_BLUE_POTION,           RG_BOTTLE_WITH_BLUE_POTION,           OBJECT_GI_LIQUID,        GID_POTION_BLUE,     TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER, {"a ", "une ", "eine "}).CustomIcon(gItemIconBottlePotionBlueTex);


This seems unrelated to the rest of the PR, can you explain it?

Thank you for taking the time to help and address some issues I will
Do my best to sort and correct here goes

This blue potion and forest temple boss key icon, I re added to help fix a custom icon issue I was having on Item Obtain for split song parts as some of the references for the icons were using different icons, will relook and sort

This kind of fix could be it's own PR that would merge faster>

Also, no need to thank me every post 😅

Pepper0ni · 2026-05-31T18:07:23Z

+    itemTable[RG_NOCTURNE_OF_SHADOW_PART1] =            Item(RG_NOCTURNE_OF_SHADOW_PART1,         Text{ "Nocturne of Shadow (Part 1)", "Nocturne de l'Ombre (Partie 1)", "Nocturne des Schattens (Teil 1)" },                         ITEMTYPE_SONG,              0xF5,                 true,  LOGIC_NONE,                         RHT_NOCTURNE_OF_SHADOW,                RG_NOCTURNE_OF_SHADOW_PART1,         OBJECT_GI_MELODY,        GID_SONG_NOCTURNE,   TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER);
+    itemTable[RG_NOCTURNE_OF_SHADOW_PART2] =            Item(RG_NOCTURNE_OF_SHADOW_PART2,         Text{ "Nocturne of Shadow (Part 2)", "Nocturne de l'Ombre (Partie 2)", "Nocturne des Schattens (Teil 2)" },                         ITEMTYPE_SONG,              0xF6,                 true,  LOGIC_NONE,                         RHT_NOCTURNE_OF_SHADOW,                RG_NOCTURNE_OF_SHADOW_PART2,         OBJECT_GI_MELODY,        GID_SONG_NOCTURNE,   TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER);
+    itemTable[RG_PRELUDE_OF_LIGHT_PART1] =              Item(RG_PRELUDE_OF_LIGHT_PART1,           Text{ "Prelude of Light (Part 1)", "Prélude de la Lumière (Partie 1)", "Kantate des Lichts (Teil 1)" },                             ITEMTYPE_SONG,              0xF7,                 true,  LOGIC_NONE,                         RHT_PRELUDE_OF_LIGHT,                  RG_PRELUDE_OF_LIGHT_PART1,           OBJECT_GI_MELODY,        GID_SONG_PRELUDE,    TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER);
+    itemTable[RG_PRELUDE_OF_LIGHT_PART2] =              Item(RG_PRELUDE_OF_LIGHT_PART2,           Text{ "Prelude of Light (Part 2)", "Prélude de la Lumière (Partie 2)", "Kantate des Lichts (Teil 2)" },                             ITEMTYPE_SONG,              0xF8,                 true,  LOGIC_NONE,                         RHT_PRELUDE_OF_LIGHT,                  RG_PRELUDE_OF_LIGHT_PART2,           OBJECT_GI_MELODY,        GID_SONG_PRELUDE,    TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER);


I don't see any reason to have 2 parts per song anymore, as there's 1 part, then the full song. this would make the code cleaner elsewhere.

Thank you for addressing things.

This is old code from when I originally had the items in 2 parts, I will remove this and retest, I think I forgot to remove this.

Pepper0ni · 2026-05-31T18:07:50Z

    itemTable[RG_FISHING_HOLE_KEY].SetCustomDrawFunc(Randomizer_DrawOverworldKey);
    // Key Rings
-    itemTable[RG_FOREST_TEMPLE_KEY_RING] =              Item(RG_FOREST_TEMPLE_KEY_RING,           Text{ "Forest Temple Key Ring", "Trousseau du Temple de la Forêt", "Schlüsselbund für den Waldtempel" },                             ITEMTYPE_SMALLKEY,          0xD5,                 true,  LOGIC_FOREST_TEMPLE_KEYS,           RHT_FOREST_TEMPLE_KEY_RING,            RG_FOREST_TEMPLE_KEY_RING,            OBJECT_GI_KEY,           GID_KEY_SMALL,       TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_SHORT, ITEM_CATEGORY_SMALL_KEY,MOD_RANDOMIZER, {"the ", "le ", "den "}, "%g");
+    itemTable[RG_FOREST_TEMPLE_KEY_RING] =              Item(RG_FOREST_TEMPLE_KEY_RING,           Text{ "Forest Temple Key Ring", "Trousseau du Temple de la Forêt", "Schlüsselbund für den Waldtempel" },                             ITEMTYPE_SMALLKEY,          0xD5,                 true,  LOGIC_FOREST_TEMPLE_KEYS,           RHT_FOREST_TEMPLE_KEY_RING,            RG_FOREST_TEMPLE_KEY_RING,            OBJECT_GI_KEY,           GID_KEY_SMALL,       TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_SHORT, ITEM_CATEGORY_SMALL_KEY,MOD_RANDOMIZER, {"the ", "le ", "den "}, "%g").CustomIcon(gQuestIconSmallKeyTex, ICON_SIZE_24);


More unrelated changes, please explain.

Pepper0ni · 2026-05-31T18:13:15Z

+        "Requires Shuffle Songs: \"Anywhere\" — Song Locations / Dungeon Rewards only have 12 song slots, so split "
+        "parts are not generated there.\n"
+        "\n"
+        "When enabled with Anywhere, the item pool places two progressive song pickups per song (24 total song "


The explanation is a bit redundant as it only works when set to anywhere

Thank you for addressing this

Will update this, possibly missed when sorting our parts to progressive

Pepper0ni · 2026-05-31T18:29:59Z

    // Gameplay stats: Update the time the item was obtained
    Randomizer_GameplayStats_SetTimestamp(item);

+    if (Rando::SplitSongs::IsSongPart(item)) {


The handling of progressive songs is needlessly complex. PARTs could simply be implemented as RAND_INF's similar to child wallet or Roc's feather, and the progressive system will send the full song item when appropriate.

Thank you for addressing this.

Ahh ok I went with similar upgrades like bombchu bags but I think I must still
Be referencing parts instead of making it progressive will sort and test.

Pepper0ni · 2026-05-31T18:33:21Z

+        case RG_PROGRESSIVE_REQUIEM_OF_SPIRIT:
+        case RG_PROGRESSIVE_NOCTURNE_OF_SHADOW:
+        case RG_PROGRESSIVE_PRELUDE_OF_LIGHT:
+            actual = SplitSongs::ResolveProgressiveSongStage(randomizerGet);


if parts are implemented as RandInfs, and there's only need for 1 part, then much of these helper functions can be reduced to a Randinf check. Child wallet is a good example here.

Thank you for addressing this

Ok will re write and test this, and get this cleaned up.

Pepper0ni · 2026-05-31T18:35:35Z

+            if (!split) {
+                AddItemToPool(RG_ZELDAS_LULLABY, songAnywhere ? 2 : 1, 1, 1, 1, songAnywhere);
+            } else {
+                AddItemToPool(RG_PROGRESSIVE_ZELDAS_LULLABY, songAnywhere ? 2 : 1, 1, 1, 1, songAnywhere);


Why are you running AddItemToPool twice? Given the other changes and cut downs you could simply say 3,2,2,2

Thank you for addressing this

This system was a potential setup for there to be additional progressive parts so you can set it to have say 6 split parts of a song, CURRENTLY A WIP but left in.

Also having an extra set of parts was to help say have a 3rd obtainable part to help avoid any soft locks.

I was considering future use of this, but I think that concept would be best done alongside a deeper change in progressives to support all items without bloating the item list.

I'm not sure if there's much demand for needing more than 1 "PART" however, and it would come with exponential code and flag bloat unless converted to it's own entry in save context like bombchu bags were

Drop Part 1/Part 2 as separate items. Each song uses progressive pool entries (still doubled for softlock) and one RandInf; first pickup sets the flag, second progressive grants the full song.

Use one AddItemToPool call per song (3,2,2,2) instead of doubling calls. Drop unrelated blue potion and key ring CustomIcon tweaks. Shorten option text now that Anywhere-only behavior is enforced in settings.

Sync with HarbourMasters develop (90823fa) before PR HarbourMasters#6627 review.

Restore GameInteractor hook includes removed during the develop merge. RegisterItemMessages still uses COND_ID_HOOK macros from GameInteractor_Hooks.h.

The develop merge updated SoH sources but left libultraship on an older commit missing Fast3dGui.h and Fast::WindowBackend, breaking all platforms.

DrawSplitSongProgress must cast Ship::Gui to Fast::Fast3dGui before GetTextureByName, matching DrawSong and the rest of the item tracker.

Clear the seed-generated flag before fill so the item tracker does not read spoiler locations while the generator thread resets context. Skip song-part spoiler lookups while RandoGenerating is set. Break infinite GetGIEntry recursion for first-stage progressive song items.

Progressive song pickups no longer resolve to full-song GI entries on the first piece; part tracking completes the song on the second pickup. Re-add key ring CustomIcon assignments dropped during the develop merge.

Pepper0ni

This PR is massively overcomplicating the implementation and is including a fix/feature (the new icon code) that really should be in it's own PR, on top of other style issues.

Parts only need to be RandInf items that are checked before giving songs using the existing progressive system, with the rest of the implementation for supporting code using existing macros and frameworks to implement custom items.

While I have considered larger changes to support all items those changes will come later and should be a change to the progressive system far beyond the scope of this PR, so the focus should be on a simple implementation.

If you are unsure how to go about this I can try to find time for a simple re-implementation to put you on the right track.

Pepper0ni · 2026-06-04T15:22:26Z

-        if (!ctx->GetOption(RSK_STARTING_PRELUDE_OF_LIGHT).Get()) {
-            AddItemToPool(RG_PRELUDE_OF_LIGHT, songAnywhere ? 2 : 1, 1, 1, 1, songAnywhere);
-        }
+        const bool split = ctx->GetOption(RSK_SPLIT_OCARINA_SONGS) && songAnywhere;


Having songs anywhere silently disable split songs here is not good practice for handling settings incompatibilities, instead check for and set 1 setting or the other in Context::FinalizeSettings and then trust it has been handled properly here.

This makes the setting easier to edit later.

Pepper0ni · 2026-06-04T15:25:28Z

    lesserPool.clear();
    int reservedSlots = 0;

+    if (ctx->GetOption(RSK_SHUFFLE_SONGS).Is(RO_SONG_SHUFFLE_ANYWHERE)) {


AddItemToPool already adds items to possibleIceTrapModels if they are in the item pool, you should not need to add them manually.

You will however need to add trick names for the Progressive Songs in shop.cpp

Pepper0ni · 2026-06-04T15:33:51Z

+#include "z64player.h"
+#include "z64save.h"
 extern PlayState* gPlayState;
+void Message_LoadItemIcon(PlayState* play, u16 itemId, s16 y);


I assume this stuff is related to the blue potion and forest boss key icons? If so I would strongly prefer those were handled in a separate PR, as significant changes involving item messages need testing and reviews from people with different skill-sets to simply adding a new progressive item step that does nothing.

Pepper0ni · 2026-06-04T15:40:31Z

 f32 triforcePieceScale;

 void RandomizerOnPlayerUpdateHandler() {
+    Rando::SplitSongs::ProcessPendingFullSongGrants();


Why do we have a function in PlayerUpdate checking specifically if a song should be given and then seemingly rebuilding part of the item give queue(!?) to offer it separately?

The Progressive Item System we should be hooking into works by changing the given item at GetItem time. The first item given will be a part that does nothing but set the RandInf that we have the part, and the second will aware the song as normal. This all happens in the normal process of receiving an item, you do not need to build a separate pipeline to handle this. This only serves to lower performance and make the code less maintainable.

Pepper0ni · 2026-06-04T15:41:50Z

        randomizerQueuedChecks = std::queue<RandomizerCheck>();
        randomizerQueuedCheck = RC_UNKNOWN_CHECK;
        randomizerQueuedItemEntry = GET_ITEM_NONE;
+        Rando::SplitSongs::ClearPendingFullSongGrants();


Again, existing systems should manage the Progressive Song properly as they do a Bomb bag or Scale. no need for redundant checks.

Pepper0ni · 2026-06-04T16:15:36Z

        "Anywhere - Songs can appear at any location.";
+    mOptionDescriptions[RSK_SPLIT_OCARINA_SONGS] =
+        "Each ocarina song becomes a progressive song item (like bomb bag upgrades). The first pickup marks "
+        "progress; the second grants the full song. The pool includes extra copies per song to reduce softlocks.";


The pool includes extra copies per song to reduce softlocks. Is not part of the current implementation, but making this into a dropdown that has that as an option would be nice.

Pepper0ni · 2026-06-09T14:23:45Z

    SECTION_DISPLAY_MINIMAL_SEPARATE,
 } ItemTrackerMinimalDisplayType;

+// One icon per logical song; order matches Rando::SplitSongs::GetSongDef (split_songs.cpp kSplitSongs).


Why are we re-implementing everything? This should only need similar handling to Bronze Scale, with maybe new graphics. Please follow existing implementations where they exist, we have many structs designed to simplify the item adding procedure.

Pepper0ni · 2026-06-09T14:26:17Z

        initTrickNames = true;
    }

+    RandomizerGet rg = static_cast<RandomizerGet>(id);


You do not need special handling here, just add trick names for your new items.

Pepper0ni · 2026-06-09T14:28:47Z

    14.0f, // ?
 };

+/**


Please do not add to src directly unless you have a very good reason, use hooks instead

Pepper0ni · 2026-06-09T14:29:38Z

        } else {
            giEntry = this->getItemEntry;
        }
+        // SOH [Randomizer]: Message_OpenText / custom text hooks read player->getItemEntry. Without syncing,


as before, do stuff like this in hooks.

RaccoonCloud · 2026-06-09T15:33:59Z

This PR is massively overcomplicating the implementation and is including a fix/feature (the new icon code) that really should be in it's own PR, on top of other style issues.

Parts only need to be RandInf items that are checked before giving songs using the existing progressive system, with the rest of the implementation for supporting code using existing macros and frameworks to implement custom items.

While I have considered larger changes to support all items those changes will come later and should be a change to the progressive system far beyond the scope of this PR, so the focus should be on a simple implementation.

If you are unsure how to go about this I can try to find time for a simple re-implementation to put you on the right track.

Thank you for taking your time again to go over this.

I'm over complicating it yes I'm sorry, I was thinking is works it could be used overall, my problem is overthinking and not simplifying as you have put.

Thank you for your patience with me on this I will try and strip it right back it all works in practice and my build but again I'm forgetting it need it to slot with the whole build and follow the main tree and not my branch and build and tweaks.

This feedback I will use and attempt but when you have time and stuff please by all means inject yourself to whatever you feel or needs doing!

Revert z_message_PAL overhaul and unrelated icon/debug changes; keep minimal song pickup icons via vanilla ITEM_SONG_* in ItemMessages. Drop deferred grant queue — second pickup resolves to full song through GetGIEntry.

FinalizeSettings clears split songs when shuffle is not Anywhere. Drop manual ice trap model list. Add progressive song trick names. Song tracker shows 1/2 on the normal songs row. getItemEntry sync lives in ItemMessages, not z_player.

… text. Stop GetGIEntry recursing when the resolved stage is still the progressive item. Reword the option tooltip like Bombchu Bag, without bracketed asides.

Hook song textbox icons through ItemMessages and the existing custom icon VB hooks, with correct IA8 16x24 gSongNoteTex drawing and warp song colours from the pause quest screen. Small z_message guard so the note is not stretched into a 24x24 quad. Sanitize UTF-8 in spoiler log, hints, and save JSON to fix regen and new-game crashes. Mark seed ready after spoiler write. Second progressive pickup grants the full song through normal item give.

GCC rejects in-class template specializations that MSVC allows. Same behavior as before - strings are still passed through SanitizeUtf8 on save.

Sync with HarbourMasters develop and resolve Traps/item tracker conflicts.

Pepper0ni

Just want to hear more reasons for why you are doing certain things.

Pepper0ni · 2026-06-17T17:06:23Z

    Random_Init(seed);

    auto ctx = Rando::Context::GetInstance();
+    ctx->SetSeedGenerated(false);


Can you explain why you added these?

Thank you again for your help and patience with me (first timer — I kept messaging Cal).

Every time I ran a new seed or generated one, SoH was crashing because other things (item tracker, spoiler lookups) could read placement data while Fill() was still mid-reset — like a “seed ready” race.

I SetSeedGenerated(false) at the top of Playthrough_Init and only flip to true after SpoilerLog_Write() finishes, so nothing treats the seed as valid until the spoiler JSON is fully written. Before that, things were trying to use spoiler data that wasn’t ready yet and it’d just crash on generate. If there’s a better way I’ll relook and sort.

This showed up most with split songs because the tracker hits song progress during seed generation, but the flag lifecycle fix is general — it just enforces generate → write → consume order.

Rebuilt and tested split songs still works (first pickup = part / 1/2 on tracker, second = full song).
Pulled icon hooks and UTF-8 stuff into separate branches like you asked.
Also removed the logic scratch workaround progressive songs go through normal ApplyItemEffect now

Pepper0ni · 2026-06-17T17:09:11Z

 std::string placementtxt;
 } // namespace

+static std::string SafeJsonString(std::string value) {


This JSON stuff looks useful, but like with the icon fixes from before might be better in it's own PR unless it relates to the functionality of split songs.

Not really anything to do with the split-songs logic it seemed to be a language spelling or non-english text (curly quotes, accents, etc.) was producing invalid JSON in the spoiler log, which then broke SoH on Seed generation so SafeJsonString is a thin wrapper: SohUtils::SanitizeUtf8 on every string before it hits jsonData.

I bundled it here because I kept hitting it while testing split songs with FR/DE UI, but happy to move this and the SaveManager string overload into a PR or if you’d rather it separately? I was scared of messing up alot of preset code and stuff you all have in place and it being other languages I panicked abit

Pepper0ni · 2026-06-17T17:22:45Z

 }

 void BuildCustomItemMessage(Player* player, CustomMessage& msg) {
+    if (TryBuildSongItemMessage(player, msg)) {


Can you explain why you needed to cut your songs parts out from the normal custom text/icon flow and instead run custom code?

If it's related to the icon issues you had fixed here in the older iteration, please push those changes to a new PR and we can look at getting that bugfix in before merging this.

Progressive song pickups use custom items, not vanilla ITEM_SONG_*. Vanilla always draws the textbox icon as 32×32 RGBA but the song note texture is IA8 16×24 which was a pain to find so without rerouting you get a garbled/static icon on pickup which was so annoying and frustrating.

TryBuildSongItemMessage routes through ITEM_CUSTOM and the existing icon hooks so the note draws right (including warp song colours from the quest screen). Also picks the right name and the progressive label on first pickup, full song name on second.

Happy to peel the icon hook stuff into a separate PR if you’d rather that first like you mentioned before.

The fact it could mess with other things is precisely why it needs to be in a separate PR, it needs completely different testing from the rest of the PR and could sneak past people who handle this stuff if you put it in this PR

Pepper0ni · 2026-06-17T17:27:29Z

    auto ctx = Rando::Context::GetInstance();
    auto logic = ctx->GetLogic();
-    if (!logic->CalculatingAvailableChecks) {
+    if (SplitSongs::IsProgressiveSong(randomizerGet) && UsingLogicSimulationBuffer(logic)) {


What reason do you have for breaking the normal ApplyItemEffect workflow here?

This bit only runs during seed generation when the game is using a temporary save copy to figure out what you can get, not when you actually pick something up in-game.

If I let the normal ApplyItemEffect run there, it was giving you the full song right away, which skips the whole progressive/split idea of “first pickup = part flag, second pickup = real song”.

So ApplyProgressiveEffectToLogicScratch fakes that in the temp save: first time = set the RandInf part flag, second time = grant the song.

When you’re actually playing, it still goes through the normal progressive flow (GetGIEntry → item give). This is just so logic checks during fill don’t get it wrong.

First PR so still finding my feet — hope that makes sense.

That sounds like an issue in ApplyItemEffect. Don't apply boilerplate to workaround the issue, that leaves a ton of mess and special cases for future coders to clean up, solve it properly. Double check what RG is hitting your checks in ApplyItemEffect, and figure out why you are getting the wrong song level.

Progressives are handled in ApplyItemEffect by checking the RG and processing what level to use based on settings and the current state, if you are applying the wrong state, something is not being checked properly.

Done ok so workaround is out, using normal ApplyItemEffect path now.

Progressive songs now go through normal ApplyItemEffect (child wallet style) instead of the logic scratch workaround.

mergify · 2026-06-19T18:15:01Z

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

Queue this pull request

Pepper0ni

This is certainly getting better, but I still think the code quality isn't there for merge. Rather than constantly play PR and fix tennis where I try to second hand debug the issues that caused you to implement other changes and possible get things wrong through (more) assumptions, I made a probably working demonstration of how this should be done in #6803 which can be used as a learning exercise.

I think the most important thing to say though is that if similar code is working for other items but not yours, you should maybe ask yourself or more experienced devs what you are doing wrong before attaching a new code-path for your items. If everyone did that, the code would quickly become an unmaintainable mess of special cases, so as reviewer I have to stop it here.

Pepper0ni · 2026-06-21T16:19:23Z

              "Putput-Kapazität (prog.)" },                                        // "Capacidad progresiva de pera"
        Text{ "Progressive Nut Bag", "Sac de noix (prog.)", "Nußbeutel (prog.)" }, // "Bolsa de nueces progresiva"
        Text{ "Progressive Husk Capacity", "Capacité de noisettes (prog.)",
-              "Schalen-Kapazität (prog.)€" }, // "Mayor capacidad de castañas"


These Euro symbols are control characters that signal that the text is plural, why were these removed?

Pepper0ni · 2026-06-21T16:46:51Z

+    RAND_INF_SPLIT_REQUIEM_PART1, RAND_INF_SPLIT_NOCTURNE_PART1, RAND_INF_SPLIT_PRELUDE_PART1,
+};
+
+static constexpr SplitSongDef kSplitSongs[SPLIT_SONG_MAX] = {


Is there a reason this isn't a map to save iterating. You can remove SplitSongId completely if you make SplitSongDef a map where the progressive RG is the key and and the value is a struct containing other data. The item tracker needs to work in QuestItem, which is unfortunate for us as it's in a bad order and nothing else wants it, but another conversion map solves that relatively efficiently.

Pepper0ni · 2026-06-21T16:52:53Z

+    return &kSplitSongs[id];
+}
+
+const SplitSongDef* SplitSongs::GetSongDefFromProgressive(RandomizerGet rg) {


you can save iterating by using rg - RG_PROGRESSIVE_ZELDAS_LULLABY instead, as RG_PROGRESSIVE_ZELDAS_LULLABY - RG_PROGRESSIVE_ZELDAS_LULLABY = 0, which is SPLIT_SONG_ZELDAS_LULLABY, and both the songs and defs share an order

You can do the same in GetSongDefFromFullSong too using RG_ZELDAS_LULLABY

Pepper0ni · 2026-06-21T16:56:43Z

+};
+
+static bool IsValidSplitSongId(SplitSongId id) {
+    return id >= 0 && id < SPLIT_SONG_MAX;


You started counting SplitSongId from 0, checking from more than 0 with result in it not recognizing ZL

the compiler should be doing this validation for you, because you have the enum's file type

Pepper0ni · 2026-06-21T16:57:28Z

+    return id >= 0 && id < SPLIT_SONG_MAX;
+}
+
+static bool UsingLogicSimulationBuffer(Logic* logic) {


If this is useful, it shouldn't be here but somewhere more central, maybe in logic itself (depends on the scoping)

Really though, if you are following existing patterns, you shouldn't need to validate more than it already does.

Pepper0ni · 2026-06-21T16:59:07Z

+}
+
+bool SplitSongs::IsProgressiveSong(RandomizerGet rg) {
+    return GetSongDefFromProgressive(rg) != nullptr;


We can just do a >= and <= check on the enum here. Remember than enums are just fancy names for numbers.

Pepper0ni · 2026-06-21T17:12:56Z

    ImGui::SetCursorScreenPos(ImVec2(p.x + 6, p.y));
    ImGui::Image(std::dynamic_pointer_cast<Fast::Fast3dGui>(Ship::Context::GetRawInstance()->GetWindow()->GetGui())
-                     ->GetTextureByName(hasSong && IsValidSaveFile() ? item.name : item.nameFaded),
+                     ->GetTextureByName(showBright && IsValidSaveFile() ? item.name : item.nameFaded),


My gut feeling is that faded with 1/2 is more clear than shaded with 1/2 as it signals you still can't use the song better, but am willing to hear other opinions.

Pepper0ni · 2026-06-23T10:56:16Z

    if (giEntry != nullptr && actual == RG_NONE) {
        return giEntry;
    }
+    if (actual == randomizerGet && giEntry != nullptr) {


Editing a core function's logic like GetGIEntry is dangerous as it could break other things unexpectedly, if you think you need this, you're probably making a mistake elsewhere, if you can't find it, tell more senior devs your problem so we can give advice.

Pepper0ni · 2026-06-23T10:58:25Z

-            SetQuestItem(RandoGetToQuestItem.find(item.GetRandomizerGet())->second, state);
-            break;
+        case ITEMTYPE_SONG: {
+            RandomizerGet rg = item.GetRandomizerGet();


The handling here doesn't need to be this bespoke or nested. The PART items are just randInf items, so they can be handled with the other RandInfs, while the song item themselves are unchanged, we're only changing how we get them

Pepper0ni · 2026-06-23T11:00:07Z

    itemTable[RG_PROGRESSIVE_MAGIC_METER] =             Item(RG_PROGRESSIVE_MAGIC_METER,          Text{ "Progressive Magic Meter", "Jauge de Magie (prog.)", "Progressives Magisches Maß" },                                           ITEMTYPE_ITEM,              0x8A,                 true,  LOGIC_PROGRESSIVE_MAGIC,            RHT_PROGRESSIVE_MAGIC_METER,           ITEM_CATEGORY_MAJOR, {"a ", "une ", "ein "}, "%g", true).CustomIcon(gQuestIconMagicJarBigTex, ICON_SIZE_24);
    itemTable[RG_PROGRESSIVE_OCARINA] =                 Item(RG_PROGRESSIVE_OCARINA,              Text{ "Progressive Ocarina", "Ocarina (prog.)", "Progressive Okarina" },                                                             ITEMTYPE_ITEM,              0x8B,                 true,  LOGIC_PROGRESSIVE_OCARINA,          RHT_PROGRESSIVE_OCARINA,               ITEM_CATEGORY_MAJOR, {"a ", "un ", "eine "}, "%g", true);
    itemTable[RG_PROGRESSIVE_GORONSWORD] =              Item(RG_PROGRESSIVE_GORONSWORD,           Text{ "Progressive Goron Sword", "Épée Goron (prog.)", "Progressives Goronen-Schwert" },                                             ITEMTYPE_ITEM,              0xD4,                 true,  LOGIC_PROGRESSIVE_GIANT_KNIFE,      RHT_PROGRESSIVE_GORONSWORD,            ITEM_CATEGORY_MAJOR, {"a ", "une ", "ein "}, "%g", true);
+    itemTable[RG_PROGRESSIVE_ZELDAS_LULLABY] =          Item(RG_PROGRESSIVE_ZELDAS_LULLABY,       Text{ "Progressive Zelda's Lullaby", "Berceuse de Zelda (prog.)", "Progressives Zeldas Wiegenlied" },                               ITEMTYPE_SONG,              0xE1,                 true,  LOGIC_NONE,                         RHT_ZELDAS_LULLABY,                    RG_PROGRESSIVE_ZELDAS_LULLABY,       OBJECT_GI_MELODY,        GID_SONG_ZELDA,      TEXT_RANDOMIZER_CUSTOM_ITEM, 0x80, CHEST_ANIM_LONG,  ITEM_CATEGORY_MAJOR,  MOD_RANDOMIZER, {"a ", "une ", "ein "}, "%g", true);


OK this is probably my bad for miscommunication, but my point about removing PART items in the future meant "we do it in a future PR", and I forgot bombchu bags has funny handling for bombchus instead of using it's own RG. It would be cleanet to have 1 part per song as an RG for the time being.

RaccoonCloud added 9 commits April 24, 2026 14:56

iAdd split ocarina songs mode with part-based progression.

9397b10

This introduces song part items, logic/state tracking, and UI/runtime integration so two parts unlock each full song while preserving stable randomizer behavior in Anywhere mode.

iUpdate split songs file headers with creator attribution.

5feddec

Replaces the previous copyright wording in split_songs.h and split_songs.cpp with a simple Created by RaccoonCloud header line.

Merge develop into feature/split-songs and resolve conflicts

2bd2ea4

Apply clang-format to split songs changes

5417774

Update split songs option text for progressive behavior.

0386051

Reworded the option in UI in rando menu to explain split songs now use two progressive pickups per song and keeps the Anywhere requirement messaging.

Fix clang-format in ItemMessages.cpp

042e035

Randomizer: all song textbox icons work for progressive pickup

6a10537

All pickup textbox icons now work correctly for progressive songs and split song parts, with the correct note icon and per-song color for each song.

Pepper0ni suggested changes May 31, 2026

View reviewed changes

RaccoonCloud added 3 commits June 4, 2026 14:47

Simplify split songs to progressive-only RandInf tracking

bc2027e

Drop Part 1/Part 2 as separate items. Each song uses progressive pool entries (still doubled for softlock) and one RandInf; first pickup sets the flag, second progressive grants the full song.

Address split songs PR review feedback

72c849f

Use one AddItemToPool call per song (3,2,2,2) instead of doubling calls. Drop unrelated blue potion and key ring CustomIcon tweaks. Shorten option text now that Anywhere-only behavior is enforced in settings.

Merge upstream develop into feature/split-songs

bd13ce5

Sync with HarbourMasters develop (90823fa) before PR HarbourMasters#6627 review.

RaccoonCloud requested a review from Pepper0ni June 4, 2026 14:02

RaccoonCloud added 5 commits June 4, 2026 17:29

Fix ItemMessages build after develop merge

11a4287

Restore GameInteractor hook includes removed during the develop merge. RegisterItemMessages still uses COND_ID_HOOK macros from GameInteractor_Hooks.h.

Bump libultraship to match upstream develop after merge

f8464be

The develop merge updated SoH sources but left libultraship on an older commit missing Fast3dGui.h and Fast::WindowBackend, breaking all platforms.

Fix split song tracker texture lookup for Fast3dGui API

10f462d

DrawSplitSongProgress must cast Ship::Gui to Fast::Fast3dGui before GetTextureByName, matching DrawSong and the rest of the item tracker.

Fix split song part-1 grant and restore key ring icons

3e1aff7

Progressive song pickups no longer resolve to full-song GI entries on the first piece; part tracking completes the song on the second pickup. Re-add key ring CustomIcon assignments dropped during the develop merge.

RaccoonCloud force-pushed the feature/split-songs branch from 7e2b465 to 3e1aff7 Compare June 8, 2026 15:17

Pepper0ni suggested changes Jun 9, 2026

View reviewed changes

RaccoonCloud added 4 commits June 9, 2026 17:24

Simplify split songs to RandInf + progressive-only pattern.

2998034

Revert z_message_PAL overhaul and unrelated icon/debug changes; keep minimal song pickup icons via vanilla ITEM_SONG_* in ItemMessages. Drop deferred grant queue — second pickup resolves to full song through GetGIEntry.

Fix seed gen crash on progressive songs and clean up split songs menu…

39427cc

… text. Stop GetGIEntry recursing when the resolved stage is still the progressive item. Reword the option tooltip like Bombchu Bag, without bracketed asides.

RaccoonCloud force-pushed the feature/split-songs branch from c8761e8 to b78aac9 Compare June 15, 2026 18:53

RaccoonCloud added 2 commits June 15, 2026 21:53

Fix Linux CI: use SaveData string overload for UTF-8 sanitization.

79b6882

GCC rejects in-class template specializations that MSVC allows. Same behavior as before - strings are still passed through SanitizeUtf8 on save.

Merge upstream develop into feature/split-songs

1198703

Sync with HarbourMasters develop and resolve Traps/item tracker conflicts.

RaccoonCloud force-pushed the feature/split-songs branch from 5a1b237 to 1198703 Compare June 15, 2026 20:56

RaccoonCloud requested a review from Pepper0ni June 15, 2026 21:45

Pepper0ni reviewed Jun 17, 2026

View reviewed changes

Split PR scope per review: drop icon and UTF-8 side fixes.

7d146a6

Progressive songs now go through normal ApplyItemEffect (child wallet style) instead of the logic scratch workaround.

This was referenced Jun 18, 2026

Sanitize UTF-8 in spoiler log, hints, and save JSON. #6769

Open

Fix song pickup textbox icons for custom/progressive items. #6770

Open

RaccoonCloud requested a review from Pepper0ni June 19, 2026 19:09

Pepper0ni suggested changes Jun 23, 2026

View reviewed changes

Refactor split songs to PART items and static songData maps per review.

2edee8d

Uh oh!

Conversation

RaccoonCloud commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Pepper0ni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pepper0ni May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RaccoonCloud May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Pepper0ni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RaccoonCloud commented Jun 9, 2026

Uh oh!

Pepper0ni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RaccoonCloud Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RaccoonCloud commented May 17, 2026 •

edited

Loading

Pepper0ni May 31, 2026 •

edited

Loading

RaccoonCloud May 31, 2026 •

edited

Loading

RaccoonCloud Jun 17, 2026 •

edited

Loading

RaccoonCloud Jun 17, 2026 •

edited

Loading

RaccoonCloud Jun 17, 2026 •

edited

Loading