Full boss shuffle's documentation does not match its behavior

[othertom]

The current description of "Full" boss shuffle is:

Same as Simple except the 3 bosses which appear twice are chosen randomly.

For reference, the current description of "Simple" is:

All original bosses (except both Agahnim’s and Ganon) are randomized including the 3 Ganon’s Tower refights. Therefore the shuffle includes two sets of Armos Knights, Lanmolas’ and Moldorm. This means Ganon’s Tower can contain 3 random bosses!

The way the description of "Full" is worded implies that three bosses from the initial set of 10 unique bosses are chosen to "appear twice", but what's actually happening in the Enemizer code is that three random bosses are sampled from the set of all bosses independently and added to the pool. The difference is that the actual behavior can cause a single boss to appear three (or maybe four?) times if more than one of the random samplings hit the same boss.

In the code linked below, BossPool with no prefix is the class used for what the ALTTPR site calls "Simple" shuffle and NormalBossPool is the class used for "Full" shuffle. This is verifiable by tracing the options set in Enemizer.php through the corresponding parsing code. For the links below I've referred to the 2MBRom branch of the tcprescott/Enimizer repo because that appears to be what the ALTTPR site currently depends on, but the code is unchanged since 2017.

Code references: https://github.com/tcprescott/Enimizer/blob/16271e13b1f376f12bdcd192b71fb620025f0190/EnemizerLibrary/BossRandomizer/Pool/BossPool.cs#L19

This method establishes that the pool of possible bosses is built from the the Base pool and the GT pool, with the Base pool being defined immediately below for "Simple" shuffle and reused in the subclass that's used for "Full" shuffle.

https://github.com/tcprescott/Enimizer/blob/16271e13b1f376f12bdcd192b71fb620025f0190/EnemizerLibrary/BossRandomizer/Pool/NormalBossPool.cs#L17

This method overrides the GT pool for "Full" shuffle to instead add three randomly-chosen bosses. The bosses are chosen completely independently with no effective constraints, so there's no mechanism to enforce that a particular boss can only appear twice.

Assuming that the current behavior is desired, I would suggest changing the description for "Full" boss shuffle to something like the following:

Same as Simple, except the bosses that would be duplicated in the Simple boss pool are instead replaced by copies of randomly-chosen bosses.

I'm not sure if the possibility of triple/quadruple bosses is worth explicitly mentioning, but I think it would be an improvement to have a description that avoids referring specifically to bosses appearing twice.

Open to re-filing this on the Enemizer repo if changes there to match the site documentation would be preferred.

[compiling]

The boss shuffle happens outside the enemizer codebase now.

For item rando, the shuffle happens here: https://github.com/sporchia/alttp_vt_randomizer/blob/master/app/Randomizer.php#L399-L410 This appears to behave according to the website's description of the shuffle.

For entrance, it happens here: https://github.com/KevinCathcart/ALttPEntranceRandomizer/blob/Dev/Bosses.py#L185-L209 This behaves like enemizer's codebase (it can pick the same boss multiple times). random.sample would pick 3 bosses without duplicates.

[othertom]

Thanks, somehow missed that the responsibility had shifted. The comment that prompted this was about a mystery seed, and while they didn't mention entrance shuffle being enabled, guessing that must have been the case.

Switching to random.sample for entrance seems desirable for consistency, would a pull request to do that be welcome?

[sporchia]

This should be properly handled by the randomizer. closing for now