Skip to content

Fix logic placing item on saving carpenters when carpenters start free #6661

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.

Sign up for GitHub

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

Merged
serprex merged 2 commits into from
May 31, 2026
Merged

Fix logic placing item on saving carpenters when carpenters start free #6661

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1c55b7
Fix logic placing item on saving carpenters when carpenters start free
serprex May 31, 2026
09516be
more fixes
serprex May 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 10 additions & 13 deletions soh/soh/Enhancements/randomizer/3drando/fill.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,30 +81,30 @@ static void PropagateTimeTravel(GetAccessibleLocationsStruct& gals, RandomizerGe
static bool UpdateToDAccess(Entrance* entrance, Region* connection) {
StartPerformanceTimer(PT_TOD_ACCESS);

bool ageTimePropogated = false;
bool ageTimePropagated = false;
Region* parent = entrance->GetParentRegion();

if (!connection->childDay && parent->childDay && entrance->CheckConditionAtAgeTime(logic->IsChild, logic->AtDay)) {
connection->childDay = true;
ageTimePropogated = true;
ageTimePropagated = true;
}
if (!connection->childNight && parent->childNight &&
entrance->CheckConditionAtAgeTime(logic->IsChild, logic->AtNight)) {
connection->childNight = true;
ageTimePropogated = true;
ageTimePropagated = true;
}
if (!connection->adultDay && parent->adultDay && entrance->CheckConditionAtAgeTime(logic->IsAdult, logic->AtDay)) {
connection->adultDay = true;
ageTimePropogated = true;
ageTimePropagated = true;
}
if (!connection->adultNight && parent->adultNight &&
entrance->CheckConditionAtAgeTime(logic->IsAdult, logic->AtNight)) {
connection->adultNight = true;
ageTimePropogated = true;
ageTimePropagated = true;
}

StopPerformanceTimer(PT_TOD_ACCESS);
return ageTimePropogated;
return ageTimePropagated;
}

// Check if key locations in the overworld are accessable
Expand Down Expand Up @@ -569,10 +569,12 @@ void GeneratePlaythrough() {
do {
gals.InitLoop();
for (size_t i = 0; i < gals.regionPool.size(); i++) {
resetSphere:
ProcessRegion(RegionTable(gals.regionPool[i]), gals, RG_NONE, false, true);
if (gals.resetSphere) {
gals.resetSphere = false;
i = -1;
i = 0;
Copy link
Copy Markdown
Member Author

@serprex serprex May 31, 2026

Choose a reason for hiding this comment

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

this is fixing unrelated issue where size_t is unsigned so this would effectively break from loop, rendering this optimization inept

goto resetSphere;
}
}
if (gals.itemSphere.size() > 0) {
Expand Down Expand Up @@ -891,14 +893,12 @@ static void AssumedFill(const std::vector<RandomizerGet>& items, const std::vect

// retry if there are no more locations to place items
if (accessibleLocations.empty()) {

SPDLOG_DEBUG("CANNOT PLACE {}. TRYING_AGAIN...",
Rando::StaticData::RetrieveItem(item).GetName().GetEnglish());

// reset any locations that got an item
for (RandomizerCheck loc : attemptedLocations) {
ctx->GetItemLocation(loc)->SetPlacedItem(RG_NONE);
// itemsPlaced--;
}
attemptedLocations.clear();

Expand Down Expand Up @@ -1009,12 +1009,10 @@ static void RandomizeDungeonRewards() {
}

if (ctx->GetOption(RSK_SHUFFLE_DUNGEON_REWARDS).Is(RO_DUNGEON_REWARDS_END_OF_DUNGEON)) {
// Randomize dungeon rewards with assumed fill
AssumedFill(rewards, Rando::StaticData::dungeonRewardLocations);
// Then remove them from the item pool
std::erase_if(itemPool, [](const auto i) {
return Rando::StaticData::RetrieveItem(i).GetItemType() == ITEMTYPE_DUNGEONREWARD;
});
AssumedFill(rewards, Rando::StaticData::dungeonRewardLocations);
Comment on lines 1012 to +1015
Copy link
Copy Markdown
Member Author

@serprex serprex May 31, 2026

Choose a reason for hiding this comment

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

@Pepper0ni pulling this fix from #5659 where this ran into same problem where closed door of time failed to generate seeds. AssumedFill would think itemPool has stones & end up placing stones on Morpha or whatever, causing generation to fail consistently

} else if (ctx->GetOption(RSK_SHUFFLE_DUNGEON_REWARDS).Is(RO_DUNGEON_REWARDS_VANILLA)) {
for (RandomizerCheck loc : Rando::StaticData::dungeonRewardLocations) {
ctx->GetItemLocation(loc)->PlaceVanillaItem();
Expand Down Expand Up @@ -1439,7 +1437,6 @@ int Fill() {
StopPerformanceTimer(PT_PLAYTHROUGH_GENERATION);
// Successful placement, produced beatable result
if (ctx->playthroughBeatable && !placementFailure) {

SPDLOG_INFO("Calculating Playthrough...");
StartPerformanceTimer(PT_PARE_DOWN_PLAYTHROUGH);
PareDownPlaythrough();
Expand Down
6 changes: 1 addition & 5 deletions soh/soh/Enhancements/randomizer/3drando/item_pool.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ void GenerateItemPool() {
ctx->PlaceItemInLocation(RC_TH_DEAD_END_CARPENTER, RG_RECOVERY_HEART, false, true);
ctx->PlaceItemInLocation(RC_TH_DOUBLE_CELL_CARPENTER, RG_RECOVERY_HEART, false, true);
ctx->PlaceItemInLocation(RC_TH_STEEP_SLOPE_CARPENTER, RG_RECOVERY_HEART, false, true);

ctx->PlaceItemInLocation(RC_TH_FREED_CARPENTERS, RG_BLUE_RUPEE, false, true);
} else if (ctx->GetOption(RSK_GERUDO_KEYS).IsNot(RO_GERUDO_KEYS_VANILLA)) {
if (ctx->GetOption(RSK_GERUDO_FORTRESS).Is(RO_GF_CARPENTERS_FAST)) {
AddItemToPool(RG_GERUDO_FORTRESS_SMALL_KEY, 2, 1, 1, 1);
Expand All @@ -630,7 +630,6 @@ void GenerateItemPool() {
AddItemToPool(RG_GERUDO_FORTRESS_SMALL_KEY, 5, 4, 4, 4);
}
}

} else {
if (ctx->GetOption(RSK_GERUDO_FORTRESS).Is(RO_GF_CARPENTERS_FAST)) {
ctx->PlaceItemInLocation(RC_TH_1_TORCH_CARPENTER, RG_GERUDO_FORTRESS_SMALL_KEY, false, true);
Expand All @@ -648,9 +647,6 @@ void GenerateItemPool() {
// Gerudo Membership Card
if (ctx->GetOption(RSK_SHUFFLE_GERUDO_MEMBERSHIP_CARD)) {
AddItemToPool(RG_GERUDO_MEMBERSHIP_CARD, 2, 1, 1, 1);
if (ctx->GetOption(RSK_GERUDO_FORTRESS).IsNot(RO_GF_CARPENTERS_FREE)) {
Copy link
Copy Markdown
Member Author

@serprex serprex May 31, 2026

Choose a reason for hiding this comment

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

this seems like an existing bug that'd always junk freeing carpenters when card shuffled. These checks shouldn't be coupled, so moved junking freeing carpenters to when carpenters start freed

ctx->PlaceItemInLocation(RC_TH_FREED_CARPENTERS, RG_BLUE_RUPEE, false, true);
}
} else {
ctx->PlaceItemInLocation(RC_TH_FREED_CARPENTERS, RG_GERUDO_MEMBERSHIP_CARD, false, true);
}
Expand Down
4 changes: 2 additions & 2 deletions soh/soh/Enhancements/randomizer/location_access/overworld/kokiri_forest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ void RegionTable_Init_KokiriForest() {
LOCATION(RC_KF_WONDER_PLATFORMS_1, logic->IsChild),
LOCATION(RC_KF_WONDER_PLATFORMS_2, logic->IsChild),
//Technically bad logic, because we can move Mido out of logic, but then we already have KSword...
LOCATION(RC_MIDO_HINT, !ctx->GetOption(RSK_FOREST).Is(RO_CLOSED_FOREST_OFF) && logic->IsChild && logic->CanUse(RG_SPEAK_KOKIRI)),
LOCATION(RC_MIDO_HINT, !ctx->GetOption(RSK_FOREST).Is(RO_CLOSED_FOREST_OFF) && logic->IsChild && logic->HasItem(RG_SPEAK_KOKIRI)),
}, {
//Exits
ENTRANCE(RR_KF_BOULDER_LOOP, logic->CanUse(RG_CRAWL)),
Expand Down Expand Up @@ -210,7 +210,7 @@ void RegionTable_Init_KokiriForest() {
LOCATION(RC_KF_MIDOS_TOP_RIGHT_CHEST, logic->HasItem(RG_OPEN_CHEST)),
LOCATION(RC_KF_MIDOS_BOTTOM_LEFT_CHEST, logic->HasItem(RG_OPEN_CHEST)),
LOCATION(RC_KF_MIDOS_BOTTOM_RIGHT_CHEST, logic->HasItem(RG_OPEN_CHEST)),
LOCATION(RC_MIDO_HINT, logic->Get(LOGIC_SHOWED_MIDO_SWORD_AND_SHIELD) && logic->IsChild && logic->CanUse(RG_SPEAK_KOKIRI)),
LOCATION(RC_MIDO_HINT, logic->Get(LOGIC_SHOWED_MIDO_SWORD_AND_SHIELD) && logic->IsChild && logic->HasItem(RG_SPEAK_KOKIRI)),
}, {
//Exits
ENTRANCE(RR_KOKIRI_FOREST, true),
Expand Down
1 change: 1 addition & 0 deletions soh/soh/Enhancements/randomizer/savefile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@ extern "C" void Randomizer_InitSaveFile() {
gSaveContext.sceneFlags[SCENE_THIEVES_HIDEOUT].swch |= (1 << 0x11);
gSaveContext.sceneFlags[SCENE_THIEVES_HIDEOUT].collect |= (1 << 0x0C); // picked up key

Flags_SetRandomizerInf(RAND_INF_TH_ITEM_FROM_LEADER_OF_FORTRESS);
if (!Randomizer_GetSettingValue(RSK_SHUFFLE_GERUDO_MEMBERSHIP_CARD)) {
Item_Give(NULL, ITEM_GERUDO_CARD);
}
Expand Down
Loading