Include triforce pieces in major item hints

[matthewkirby]

The important_check hint type says X has N major items. where X is the location and N is the number of important items. There is a big block of code to exclude things from the count of "important items" and they mostly make sense. Things are removed from the count to not confuse the player. Unfortunately, this include triforce pieces. Original feature author stated that it "defeats the idea of a triforce hunt" but I would argue it is dramatically more confusing to omit them.

These hints have been included in RSL for quite awhile but today someone ran into the case where they had a hint that said "Shadow Temple has 7 major items" and then collected what they were interpreting as 7 major items but the same room that had the 7th major item also had an 8th major item. I think that while excluding pieces may from the hints may have merit as the original author intended, but a player running a seed will need to specifically know that triforce pieces are removed or risks confusion.

[matthewkirby]

So I actually looked at this function a little more. While this change fixes a big issue (at least imo), I think there are other problems with it. To some extent, its just the mega if statement. For instance, if a location has location.locked=True, it isn't included even if it has a major item. UNLESS that major item is the BGS or DD. The same is true for all the keys as well below. Also, the last chunk about shuffle_ganon_bosskey doesn't include the heart win con but does include the token conditions.

The issues (at least as I see them) mostly come down to decisions (some, like the one that inspired me to look at it, were somewhat arbitrary imo) that were made when the original feature was added, but also come with a bunch of hard coded exceptions. The randomizer can already track when an item is "important". Is there a flag anywhere for an item or location that indicates this? It might be easier to just simple build these hints using that flag.

[matthewkirby]

Detailing the additional changes, the first 3 remove about 10 lines of code and result in no changes.

1. Corrected the logic inside the if statement such that not location.locked always needs to be true, not just when location.item.majoritem is true.
2. The majoritem property of the Item class actually does identical checks to all of the key checks. This lets me remove the or statements for small keys/keyrings, hideout keys/keyrings, tcg keys/keyrings, boss keys, and the shuffle_ganon_bosskey options for vanilla and dungeon with 0 changes to current functionality (after considering the bug fix in bullet 1 above).
3. For shuffle_ganon_bosskey, I removed the options for stones, medallion, dungeons, and tokens (hearts was missing). When any of these options are selected, the ganon boss key does not exist in a hintable region (root), therefore we don't need to worry about these conditions. This change also has no change to current functionality of the code.
4. I removed the final setting for shuffle_ganon_bosskey, on_lacs. When this option is enabled, the player must return to the temple of time to receive their boss key. This is a normal item location in the vanilla game and the ganon boss key is an important item, so why not count it.

My entire goal looking at this was to make these hint types consistent and usable. Top RSL runners mention that they ignore these hints because they are inconsistent and are unsure what is counted and what isn't. These changes should clarify the hint type to include any shuffled non-junk item or item required to beat the game (such as hearts with heart win con).

[fenhl]

4. I removed the final setting for shuffle_ganon_bosskey, on_lacs. When this option is enabled, the player must return to the temple of time to receive their boss key. This is a normal item location in the vanilla game and the ganon boss key is an important item, so why not count it.

If Gbk is on LACS, the LACS location is locked (i.e. unshuffled). This matches the behavior of other hint types — we don't hint anything that's already known from settings.

[fenhl]

What's the purpose of this line?

[matthewkirby]

Oh, was I being silly? I thought when you said the thing about not hinting GBK on lacs you meant I should reintroduce the line to not count it. So the intent was that if the region was temple of time, the ganon boss key would not be counted as a major item for this hint type.

[matthewkirby]

I will actually run some tests later and make sure that the GBK on lacs isn't included if I remove this line.

[fenhl]

I meant that it's not going to be hinted either way so the line is not necessary. Also, the way it's written, it excludes Gbk when it's not on LACS, which I don't think we should do.

[fenhl]

I will actually run some tests later and make sure that the GBK on lacs isn't included if I remove this line.

I just checked and it isn't.