Change `Region`'s scene string with a `SceneId` by Pepe20129 · Pull Request #5398 · HarbourMasters/Shipwright

Pepe20129 · 2025-04-12T16:42:10Z

Also deduces timePass (and areas when possible) from the scene.

Build Artifacts

No possible for `RR_CASTLE_GROUNDS` & "link's pocket" areas

Pepper0ni · 2025-04-13T20:01:36Z

Deriving time pass form the scene is problematic for psuedo-regions used to filter out access such as swim checks when you spawn in water, as it can cause the player to get time pass in a area they cannot actually survive in. What is the intention for those situations?

The colossus hands would also be a problem but apparently they were set to spirit temple anyway, which raises spirit temple key logic questions about TimePass I would have to think about.

Pepe20129 · 2025-04-13T20:17:17Z

I think for those regions we could use SCENE_ID_MAX which is what we do for other pseudo-regions.

Pepper0ni · 2025-04-13T21:00:01Z

I think for those regions we could use SCENE_ID_MAX which is what we do for other pseudo-regions.

The issue here is that they could then connect back to the original scene as they won't be caught by the check in entrance shuffle that prohibits own-scene connections

Pepper0ni

A few things I noticed going over it all, mostly side effects of moving from text to ID and mistakes in assigning timepass.

Pepper0ni · 2025-04-13T20:07:56Z

soh/soh/Enhancements/randomizer/entrance.cpp


    // Entrances shouldn't connect to their own scene, fail in this situation
-    if (entrance->GetParentRegion()->scene != "" &&
+    if (entrance->GetParentRegion()->scene != SCENE_ID_MAX &&


This needs an exception to handle OGC, Hyrule Castle and Castle Grounds (currently assigned as SCENE_ID_MAX) and make sure they are not connected to each other.

Additionally, all grottos and fairy fountains have separate scene text right now that allows them to connect to each other in decoupled + mixed pools, under this system they would all have the same scene and be unable to connect, which I doubt was your intention.

How does entrance rando currently handle going to hc/ogc as the wrong age?

Fixed the grottos/fairy fountains

It uses a filtration region called RR_CASTLE_GROUNDS, which splits the logic into the HC and OGC regions on entry, enforcing the age restriction, however leaving these 2 does not go through this region unless it's the south connection, as RR_CASTLE_GROUNDS also connects to market.

In practice, I believe what happens is just that HC and OGC are the same scene, and the difference is only determined by what age you are going in, like Market, in case that was the angle you were after.

Fixed, I think.

soh/soh/Enhancements/randomizer/location_access.cpp

soh/soh/Enhancements/randomizer/location_access/dungeons/ganons_castle.cpp

Pepper0ni · 2025-04-14T00:23:51Z

soh/soh/Enhancements/randomizer/location_access/dungeons/spirit_temple.cpp

    });

-    areaTable[RR_SPIRIT_TEMPLE_OUTDOOR_HANDS] = Region("Spirit Temple Outdoor Hands", "Spirit Temple", {RA_SPIRIT_TEMPLE}, NO_DAY_NIGHT_CYCLE, {}, {
+    areaTable[RR_SPIRIT_TEMPLE_OUTDOOR_HANDS] = Region("Spirit Temple Outdoor Hands", SCENE_SPIRIT_TEMPLE, {}, {


Technically speaking, Time passes on the hands, however the complexities of spirit key logic make me unsure if I want it to count as a time pass area in the long term. I would have to think about it.

soh/soh/Enhancements/randomizer/location_access/dungeons/water_temple.cpp

Pepper0ni · 2025-04-14T00:31:50Z

soh/soh/Enhancements/randomizer/location_access/overworld/castle_grounds.cpp

    //With multi-area support {RA_CASTLE_GROUNDS} is not strictly required anymore, as any interior here could inherit both
    //{RA_HYRULE_CASTLE} and {RA_OUTSIDE_GANONS_CASTLE}, but a setting to merge the latter 2 into the former may be preferred
-    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", "Castle Grounds", {RA_CASTLE_GROUNDS}, NO_DAY_NIGHT_CYCLE, {}, {}, {
+    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", SCENE_ID_MAX, {RA_CASTLE_GROUNDS}, {}, {}, {


Similar to the boss situation, this will be able to connect to OGC and market if not accounted for, however this trio need special casing due to the way the area is handled between ages.

Fixed, I think.

soh/soh/Enhancements/randomizer/location_access.cpp

Pepper0ni · 2025-04-23T19:14:01Z

soh/soh/Enhancements/randomizer/location_access/overworld/castle_grounds.cpp

-    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", "Castle Grounds", {RA_CASTLE_GROUNDS}, NO_DAY_NIGHT_CYCLE, {}, {}, {
+    //
+    //Temporarily uses SCENE_OUTSIDE_GANONS_CASTLE to avoid self connection between ages
+    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", SCENE_OUTSIDE_GANONS_CASTLE, {RA_CASTLE_GROUNDS}, {}, {}, {


This should work for now, but at some point OGC is going to want it's own day/night setting to handle the perma-night, do you have an idea of how to separate this from the rest of OGC when that happens?

We could add an explicit check for it in AreEntrancesCompatible (checking the name or the area or the id) and change this back to SCENE_ID_MAX.

Fair enough, so long as there's a plan for when I get around to it. a hardcoded exception never feels great but it seems appropriate in this case given the special case

Pepper0ni · 2025-04-28T10:46:26Z

Deriving time pass form the scene is problematic for psuedo-regions used to filter out access such as swim checks when you spawn in water, as it can cause the player to get time pass in a area they cannot actually survive in. What is the intention for those situations?

The colossus hands would also be a problem but apparently they were set to spirit temple anyway, which raises spirit temple key logic questions about TimePass I would have to think about.

I have added right hand to the shared area, so i do not want time pass on the hands. I'm pretty sure any case where we have certain hands access we will get colossus and it's TimePass in logic anyway

Malkierian · 2025-05-02T01:59:08Z

Just so I know, what was the result of the discussion about pseudo-areas and timePass inheritance? I think it could probably go in, but that has me still concerned.

Pepper0ni · 2025-05-03T17:48:56Z

Just so I know, what was the result of the discussion about pseudo-areas and TimePass inheritance? I think it could probably go in, but that has me still concerned.

I just looked it over, and there was indeed no satisfactory response, I guess we need that fallback I mentioned immediately rather than later, so I'm unapproving until there's a manual TimePass override option or some other solution that doesn't cause entrance connection issues

Pepper0ni

Had a review, the core structure seems solid now, there's just some shortcut stuff i found issue with and some other small things

soh/soh/Enhancements/randomizer/location_access.cpp

soh/soh/Enhancements/randomizer/location_access/dungeons/dodongos_cavern.cpp

soh/soh/Enhancements/randomizer/location_access/dungeons/fire_temple.cpp

soh/soh/Enhancements/randomizer/location_access/overworld/lake_hylia.cpp

soh/soh/Enhancements/randomizer/location_access/overworld/zoras_river.cpp

Malkierian · 2025-06-18T20:31:03Z

@Pepe20129 Address Pepper0ni's latest review and the merge conflicts, and I think we can get this in. @Pepper0ni if you wouldn't mind going back through the other reviews and seeing if the remaining unresolved conversations can be marked as resolved, I'd appreciate it.

Pepe20129 · 2025-06-18T21:34:03Z

Fixed merge conflicts

Malkierian · 2025-06-18T21:57:05Z

Looks like you might be missing an include in deku_tree.

soh/soh/Enhancements/randomizer/location_access/overworld/zoras_river.cpp

soh/soh/Enhancements/randomizer/location_access/overworld/lake_hylia.cpp

Pepper0ni · 2025-06-18T22:06:29Z

soh/soh/Enhancements/randomizer/location_access/overworld/castle_grounds.cpp

-    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", "Castle Grounds", {RA_CASTLE_GROUNDS}, NO_DAY_NIGHT_CYCLE, {}, {}, {
+    //
+    //Temporarily uses SCENE_OUTSIDE_GANONS_CASTLE to avoid self connection between ages
+    areaTable[RR_CASTLE_GROUNDS] = Region("Castle Grounds", SCENE_OUTSIDE_GANONS_CASTLE, TIME_DOESNT_PASS, {RA_CASTLE_GROUNDS}, {}, {}, {


is RA_CASTLE_GROUNDS still properly preserved to cover the interior-on-the-market-exit edge case?

I think so.

Pepper0ni

LGTM

* Change region scene from string to sceneid * Deduce `timePass` from scene * Deduce `areas` from scene where possible No possible for `RR_CASTLE_GROUNDS` & "link's pocket" areas * Update zoras_fountain.cpp * Applied clang format * Address review * Address review * Clang format * Fix pseudo regions * Format * Address review * Address review

Pepe20129 added 4 commits April 7, 2025 19:15

Change region scene from string to sceneid

116291c

Deduce timePass from scene

c8726c2

Deduce areas from scene where possible

c07ccf5

No possible for `RR_CASTLE_GROUNDS` & "link's pocket" areas

Merge branch 'develop' into region_scene

0fa5ed6

Pepe20129 added 2 commits April 13, 2025 22:19

Update zoras_fountain.cpp

f2da25a

Applied clang format

2d50236

Pepper0ni suggested changes Apr 14, 2025

View reviewed changes

Pepe20129 added 3 commits April 19, 2025 22:42

Address review

ca3f704

Address review

5602161

Clang format

41fa184

Pepper0ni reviewed Apr 23, 2025

View reviewed changes

Pepper0ni approved these changes Apr 27, 2025

View reviewed changes

Pepper0ni mentioned this pull request May 26, 2025

Add a lot "Ocarina items tricks" [W.I.P] #5544

Closed

17 tasks

Pepe20129 added 3 commits May 26, 2025 20:07

Fix pseudo regions

8ff130c

Format

c81de3a

Merge branch 'develop' into region_scene

a520834

Pepe20129 requested review from Malkierian and Pepper0ni May 30, 2025 20:59

Pepper0ni suggested changes Jun 14, 2025

View reviewed changes

Pepe20129 added 2 commits June 18, 2025 22:35

Address review

6a6dfbf

Merge branch 'develop' into region_scene

2e03fe4

Pepper0ni reviewed Jun 18, 2025

View reviewed changes

soh/soh/Enhancements/randomizer/location_access/overworld/zoras_river.cpp Outdated Show resolved Hide resolved

Pepper0ni reviewed Jun 18, 2025

View reviewed changes

soh/soh/Enhancements/randomizer/location_access/overworld/lake_hylia.cpp Outdated Show resolved Hide resolved

Pepper0ni reviewed Jun 18, 2025

View reviewed changes

Address review

50af77d

Pepper0ni approved these changes Jun 19, 2025

View reviewed changes

Malkierian approved these changes Jun 19, 2025

View reviewed changes

Malkierian merged commit 87c9713 into HarbourMasters:develop Jun 19, 2025
6 checks passed

Pepe20129 deleted the region_scene branch June 20, 2025 11:17

Conversation

Pepe20129 commented Apr 12, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Artifacts

Uh oh!

Pepper0ni commented Apr 13, 2025

Uh oh!

Pepe20129 commented Apr 13, 2025

Uh oh!

Pepper0ni commented Apr 13, 2025

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!

Malkierian Apr 20, 2025 • 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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Pepper0ni commented Apr 28, 2025

Uh oh!

Malkierian commented May 2, 2025

Uh oh!

Pepper0ni commented May 3, 2025

Uh oh!

Pepper0ni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Malkierian commented Jun 18, 2025

Uh oh!

Pepe20129 commented Jun 18, 2025

Uh oh!

Malkierian commented Jun 18, 2025

Uh oh!

Uh oh!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Pepe20129 commented Apr 12, 2025 •

edited by github-actions bot

Loading

Malkierian Apr 20, 2025 •

edited

Loading