C7213457 - Hera Big Key / Fire Rod/Lamp Issue

[Dessyreqt]

Submitter: TheOkayGuy [v7]

c7213457

Unreachable fire/flute in a casual seed.

Bombos, Flute, Lamp, Firerod block each other.

[KatDevsGames]

it looks impossible to me

go to zelda, get flippers go to hobo, get hammer go to eastern, get mirror go to ice cave, get gloves Go to swamp, get Ether

go to spectacle rock cave, get mitts go to hera, get big key (BIG KEY BLOCKED BY TORCH PUZZLE) go to moldorm, get fire rod go to ice palace, get ocarina

Go to mire, get lamp

both fire items are beyond the hera torch puzzle in the progression

[ChristosOwen]

Also can't see anything wrong with Mire, Hera, Ice logic, with big keys or lamp/fire rod/bombos placements, so also not sure what the issue here is. Logic looks good.

[BlueViper85]

Similar to the hookshot issue, I created a log of the generated item order just to get an idea of what was going on.

Of the key trouble items here: Bombos, Fire Rod, Lamp, and OcarinaInactive, I noticed that it attempted to place each of these items more than once.

It seemed like, in at least some instances, when it first tried to place some of the items it was thrown off by the !handledDungeons evaluation (Randomizer.cs line 547) where it generated items for the dungeon instead of placing the item, which then forced the items to be placed anywhere. I could easily be barking up the wrong tree here, but it's something I noticed and figured better to share than not.

Here's the log for anyone to review and see how items were placed. At one point it tried putting the Hookshot at Moldorm's heart container (well before the FireRod wound up there instead).

GeneratedItemOrder - c7213457.txt

[Edit: So I just realized when comparing v6 and v7 source that v7 reworked how dungeon items are placed, and I don't think it's properly considering (at least in the case of the Fire Rod at moldorm) that you actually need the Big Key to get to Moldorm. So my theory being that it thinks you can get the fire rod and THEN go get the Big Key

I think this applies to the Hookshot one too. It placed the hookshot first, so at that point the flooded room didn't have the big key. After that, it placed the big key in the flooded room thinking you could access the Hookshot because it didn't realize the Big Key was needed to access the hookshot]

[BlueViper85]

Looking further at the logic for the TowerOfHera I noticed something.

CanDefeatTowerOfHera(have) has this:

return CanEnterTowerOfHera(have) && (!LocationHasItem("[dungeon-L3-1F] Tower of Hera - first floor", InventoryItemType.BigKey) || CanLightTorches(have));

So effectively it's first checking to make sure you can enter Hera, obvious enough. Then it checks EITHER that the big key is NOT on the first floor OR that you can light torches.

What it DOESN'T do is check to make sure that if the Key is on the first floor that you can access it. That's technically handled by the logic for that chest. HOWEVER, by changing CanDefeatTowerOfHera to check this, it's also confirming that Moldorm can't be beat unless you can access that chest (if it has the key in it).

I changed it to this: return CanEnterTowerOfHera(have) && ((LocationHasItem("[dungeon-L3-1F] Tower of Hera - first floor", InventoryItemType.BigKey) && CanLightTorches(have)) || LocationHasItem("[dungeon-L3-2F] Tower of Hera - Entrance", InventoryItemType.BigKey));

I tested this code change and it seemed to resolve the issues with this seed and the other seed.

Can anyone see any flaw in my logic or otherwise have any concerns?

Edit: I didn't add the actual code changes I made, so I added that.

For the sake of clarity, I also wanted to add that the && before the CanLightTorches(have) would only be evaluated if the Big Key IS in the 1F chest. If it's not then it won't worry about if you can light torches and then double-checks that the chest does NOT have the Big Key so that if 1F doesn't have the BigKey this function will still return True.

[ChristosOwen]

From #276 (same bug)

Comment from Radagast81:

I reviewed this logic as well and have no concerns with your changes. Nonetheless it's now clear why these logic problems now appear and haven't before: The original version of the check returns true as long as the big key isn't put into a chest and changes to false afterwards, whereas the second logic allways gets the right solution (false) as long as the big key isn't put. So there are in generel 2 aproaches to solve this issue:

Review all the logic that contain keys and consider the temporary changes through the randomizer process (also the logic becomes more complex through this).
First place all the keys (big and small) without considering the can_access logic and afterwards place the items (then the 2 logics are equivalent and the issues will no longer accure).

I would prefere the second aproach, as the logic keeps to be simple and you don't have to worry that through the placement of a key the seed becomes unbeatable...

[BlueViper85]

Thanks for checking, Radagast! Ideally, I think you're right that number 2 is the best option and worth consideration as a somewhat more long-term goal since it would likely take a fair amount of effort to ensure it doesn't cause new issues. My proposed solution is just one option that could work as a short-term solution while more detailed work is evaluated.

Relative to number 1 though, I think it's worth mentioning that we saw this issue mainly as a result of there being chests after the big key in the Tower of Hera which is one of only a few dungeons that have chests behind the Big Key door or behind a specific item requirement.

Swamp Palace is one other one, which there's an issue with the Hookshot and Big Key placement there (to which I also have a proposed solution). Part of Swamp's issue is that it doesn't require the hookshot to enter, only to beat it and to access the last three chests.

Turtle Rock is another one, and the chests behind the big key door already can't have the big key and don't require specific items to access that aren't already required for entering the dungeon already (unlike Swamp Palace) (e.g. Somaria)

Ganon's Tower is another, but can already only be accessed when you have all the main progression items.

Otherwise the logic to access the bosses is the same as what is necessary to defeat them, and they can't hold Big Keys, and a lot of effort has gone into evaluating small key placements.

[Radagast81]

No problem. I mean the real issue is, that placing a key in a chest can invalidate a already assigned location. That (and a change of the special locations what is done prior the item distribution so no issue) are the only two possibilities how that could happen. So it would be good to distribute the keys first to get rid of this problem without having to validate already assigned locations again (or changing the normally correct can_access-conditions).

[SmashManiac]

I believe I stumbled on the same issue on v7 Casual seed C6433259:

• Tower of Hera's Big Key is in the 1F chest, which is blocked by unlit torches.
• The Lamp is in Misery Mire, which is inaccessible because the Moon Pearl is guarded by Lanmolas in a room also blocked by unlit torches.
• Sahasrahla has the Fire Rod, but is also inaccessible because the Pendant of Courage is the reward for beating the Tower of Hera.

[BlueViper85]

SmashManiac's issue is the same issue as this one, where it doesn't take into account the big key is needed to get to Moldorm.

Here's another seed where a similar issue happened. While it involves the fire rod, that's purely coincidental as this is the result of the logic not checking that the big key is accessible to get the Fire Rod.

V7 Seed C0304796

Fire Rod is not obtainable due to Big Key being in the chest in the second half of Skull Woods

Skull Woods

[dungeon-D3-B1] Skull Woods - big chest...................................................Fire Rod [dungeon-D3-B1] Skull Woods - Big Key room................................................Map [dungeon-D3-B1] Skull Woods - Compass room................................................Key [dungeon-D3-B1] Skull Woods - east of Fire Rod room.......................................Compass [dungeon-D3-B1] Skull Woods - Entrance to part 2..........................................Big Key [dungeon-D3-B1] Skull Woods - Gibdo/Stalfos room..........................................Key [dungeon-D3-B1] Skull Woods - south of Fire Rod room......................................Key Heart Container (Mothula).................................................................Piece of Heart

For a quick fix, adding the following to CanDefeatSkullWoods solves the problem, since it checks if the dungeon is beatable when placing the dungeon items. A more automated means of checking accessibility of items when placing new ones makes more sense as a long-term solution though.

                && !(LocationHasItem("[dungeon-D3-B1] Skull Woods - big chest", InventoryItemType.FireRod) 
                && LocationHasItem("[dungeon-D3-B1] Skull Woods - Entrance to part 2", InventoryItemType.BigKey));

This still allows for the Fire Rod to be located in the Big Chest in other seeds and the Big Key can still be in the chest in "Entrance to part 2" in other seeds.

[Radagast81]

Hi Blueviper, your quick fix involves negation, that can potential lead to changes from true to false during the distribution of item and therefore invalidate already distributed locations. My suggestion for a quick fix of these kind of problem:

1) Add a function to BaseRomLocations:

internal bool NotLocationHasItem(string locationName, InventoryItemType item)
{
    var itemAtLocation = GetItemAtLocation<InventoryItem>(Locations, locationName);
    return itemAtLocation != null && itemAtLocation.Type != InventoryItemType.Nothing && itemAtLocation.Type != item;
}

This function is the negation of LocationHasItem, but only returns true if the location is already distributed.

2) Change your logic to the following equivalent:

&& (NotLocationHasItem("[dungeon-D3-B1] Skull Woods - big chest", InventoryItemType.FireRod) 
    || NotLocationHasItem("[dungeon-D3-B1] Skull Woods - Entrance to part 2", InventoryItemType.BigKey));

As not(A and B) is the same as ((not A) or (not B)) due to basic boolean logic.

[BlueViper85]

Thanks, Radagast. Your check that a chest isn't empty is something I've been thinking is needed. While I don't know that I fully understand how mine is a problem necessarily (and I'd love to understand that better, but this isn't the best place to discuss such a thing)

Thinking about your solution further (and maybe I'm way off here, I know I'm not the strongest coder), I'm thinking your solution makes more sense as a full replacement for !LocationHasItem and then no modification needs to be made to other functions like CanDefeatSkullWoods. The bulk of the problem with the big key check is that it's often checking at at time when the chest that can't have the big key is still empty and later that chest is being filled with the big key.

Replacing all the !LocationHasItem with your NotLocationHasItem fixes that problem, and at least thinking about it doesn't seem to introduce any unexpected issues. Thoughts on that?

[Radagast81]

Yeah I've thought about the current problems and the NotLocationHasItem function is meant as a more general solution to these. There is only one thing that you have to consider:

new Location
{
    Name = "AAA",
    CanAccess =
       have => NotLocationHasItem("AAA",InventoryItemType.BigKey) 
}

Will cause problems since it will never evaluate to true. But that is a thing that you want to permit anyhow, cause that kind of recursive checks make no sense at all...

[BlueViper85]

I put a bit more thought into this and think I've figured out a better solution. I added a function to check if the Big Key has been placed in a dungeon, and I have this being checked when determining if the Big Chest can be accessed. This forces the big key to be placed before the big chest can be accessed, and allows the already-existing big chest logic to work properly since the big key will be somewhere and even if the chests the big chest logic looks for are still empty (since it knows the big key has been placed)

This is the function I threw together as a proof of concept and so far it seems to be working nicely. Needs a bit more testing, and I'm sure this can be made more efficient, but I'll look at that more tomorrow.

    internal bool RegionBigKeyPlaced(Region region)
    {
        var locations = (from Location location in Locations where (location.Region == region) select location).ToList();

        foreach (var location in locations)
        {
            if (LocationHasItem(location.Name,InventoryItemType.BigKey))
            {
                return true;
            };
        }

        return false;
    }

Then on every big chest in the game I added something to the effect of this (the specified region being appropriate for each big chest):

    && RegionBigKeyPlaced(Region.SwampPalace)

[BlueViper85]

I've got a new method for tracking this that doesn't require looping through each dungeon's chest every time it tries to see if the Big Chest is applicable. Further testing is required of this new one, but after placing a check for the big key's placement before each check for it's location seems to have helped this issue (and similar) without causing further issues.

[BlueViper85]

The fix for this issue has been applied to the base code and will be available when the next version is released (no ETA). Closing this issue since a fix has been applied.