The Wind Waker: Implement New Game

[tanjo3]

What is this fixing or adding?

This PR adds The Legend of Zelda: The Wind Waker as a supported game in Archipelago. The game uses LagoLunatic's randomizer as its base (regarding logic, options, etc.) and builds from there. Work on TWW started with Dev5ter in 2023, and I took over the project in February 2024.

The game requires an external program to randomize the ISO. This program is a modified version of Lago's randomizer that takes an APTWW file as input, which is the game's output. The program has the additional benefit of already supporting custom models, which are popular in the community. The program download can be found here: https://github.com/tanjo3/wwrando/releases/tag/ap_2.5.0. Generating multiworlds does not require this program or the vanilla ISO.

How was this tested?

Over the last year, I have produced numerous public releases of the TWW APWorld, making modifications to implement features from the base randomizer, address issues, and implement a few features unique to AP. The Wind Waker thread has many followers and recently hit the follower limit. The latest release, v2.6.1, has been out since December 31, 2024. The previous release, v2.5.2, was released in August 2024. That release was very stable, and all issues raised between that release and the current version have been fixed. I want to thank the community for their support, patience, and feedback over the last year. Without them, this would not have been possible.

If this makes graphical changes, please attach screenshots.

Adds The Wind Waker client to the AP Launcher with a custom icon.

[ScipioWright]

The separate_pools option for Mix Entrances frequently fails. It usually fails on assert len(nonprogress_entrances) == len(nonprogress_exits) from split_nonprogress_entrances_and_exits in Entrances.py.

This is with the different entrance randomization options set on, and most other options set to random.

[tanjo3]

The randomizer build (2.5.0 pre-release) has been updated to support this latest change (ae6fed7).

[Mysteryem]

The bigger issues in this batch of reviews:

• Nondeterministic generation for Required Bosses
• Setting item rules in pre_fill is too late
• I forgot to update fill_slot_data when removing the intermediate entrances and entrance regions

[Mysteryem]

I think that beyond this PR, it is going to be beneficial to rework some of the location and region logic to include additional inner/sub-regions and use rules on entrances between those regions to break up logic.

Running generation with cProfile reveals many dungeon and ganon access rules are taking up more time/calls than others.
In Generate.py, find multiworld = ERMain(erargs, seed) and replace it with:

    import cProfile
    from pstats import Stats, SortKey
    with cProfile.Profile() as pf:
        multiworld = ERmain(erargs, seed)
    # print first 50 lines sorted by cumulative duration
    Stats(pf).sort_stats(SortKey.CUMULATIVE).print_stats(50)

Note that cProfile is less accurate for very quick functions, since it incurs some overhead. It's a useful tool, but take that into account when using it.

---

The "Defeat Ganondorf" location has once of more complicated access rules in TWW, but it is added to the "The Great Sea" region, which is the starting region, meaning that it will constantly be checked for being reachable.

Many of the dungeon locations closer to the end of dungeons have similar issues in that they include longer, chained rules for reaching that point of the dungeon.

For example, there are 4 locations that check can_reach_earth_temple_many_mirrors_room() in their access rule.
can_reach_earth_temple_many_mirrors_room(), first checks can_reach_earth_temple_tall_vine_room(),
which first checks can_reach_earth_temple_third_crypt(),
which first checks can_reach_earth_temple_redead_hub_room(),
which first checks can_reach_earth_temple_basement(),
which first checks can_reach_earth_temple_sun_statue_room(),
which first checks can_play_command_melody().
can_play_command_melody() is finally the first part of the can_reach_earth_temple_many_mirrors_room() rule that actually checks the state.

These 4 locations are in the Earth Temple region, so when the Earth Temple region is accessible, the access rules of those locations will be checked.

It would likely be beneficial to create sub-regions within dungeons to avoid checking such long chained rules. Changing the order that rules are checked within each function, so that more complex/chained rules are checked last, and simpler rules are checked first may also help.

In the future, I could imagine there being Hyrule and Ganon's Tower regions.
The Great Sea would connect to Hyrule using the has_all_8_triforce_shards() rule. Miniboss Entrance in Hyrule Castle would be placed in the Hyrule region.
Hyrule would connect to Ganon's Tower using the can_access_ganons_tower rule, but with can_access_hyrule removed from can_get_past_hyrule_barrier because that part of the logic will now be controlled by the The Great Sea -> Hyrule entrance. The Ganon's Tower locations would be placed in the Ganon's Tower region.

For dungeons, it is less clear how best it would be to break up regions because I'm not sure there are locations present in every room with some logic tied to it, but otherwise, every room that has a presence in rules, like can_reach_earth_temple_<room name> could become a region with entrances connected them to other rooms.

The 4 locations that check can_reach_earth_temple_many_mirrors_room() could then go in an Earth Temple - Many Mirrors Room region for example, which has a connection from Earth Temple - Tall Vine Room etc.

[Mysteryem]

I am done looking through all the code now.

I have not looked at the documentation.

I have submitted a PR that will hopefully fix the client taking so long to close down, though it needs some testing that I have not got around to yet.

[Mysteryem]

Each if statement only affects a single type of item currently, so this function could be replaced with a dictionary instead of going through this if chain for every item created.

The current if chain could also have been an elif chain after the first if, instead of every single if condition always being checked.

Dictionary example:

# Set in __init__():
self.adjusted_item_classifications: dict[str, IC] = {}

# Populated in generate_early()
adjusted_item_classifications = self.adjusted_item_classifications
if not self.options.progression_big_octos_gunboats:
    adjusted_item_classifications["Quiver Capacity Upgrade"] = IC.useful
if self.options.sword_mode in ("swords_optional", "swordless"):
    adjusted_item_classifications["Progressive Sword"] = IC.useful
if not self.options.enable_tuner_logic:
    adjusted_item_classifications["Tingle Tuner"] = IC.useful
if not self.options.progression_dungeons:
    for key_name in (item_name_groups["Small Keys"] + item_name_groups["Big Keys"]):
        adjusted_item_classifications[key_name] = IC.filler
if not self.options.progression_dungeons:
    for item_name in ("Command Melody", "Earth God's Lyric", "Wind God's Aria"):
        adjusted_item_classifications[item_name] = IC.filler
if not self.options.progression_short_sidequests:
    for item_name in ("Maggie's Letter", "Moblin's Letter"):
        adjusted_item_classifications[item_name] = IC.filler
if (
    not (self.options.progression_short_sidequests or self.options.progression_long_sidequests)
):
    adjusted_item_classifications["Progressive Picto Box"] = IC.filler
if not self.options.progression_spoils_trading:
    adjusted_item_classifications["Spoils Bag"] = IC.filler
if not self.options.progression_triforce_charts:
    for chart_name in item_name_groups["Triforce Charts"]:
        adjusted_item_classifications[chart_name] = IC.filler
if not self.options.progression_treasure_charts:
    for chart_name in item_name_groups["Treasure Charts"]:
        adjusted_item_classifications[chart_name] = IC.filler
if not self.options.progression_misc:
    for statue_name in item_name_groups["Tingle Statues"]:
        adjusted_item_classifications[statue_name] = IC.filler

Replace previous use of self.determine_item_classification(name) with self.adjusted_item_classifications.get(name), or change self.determine_item_classification(name) to return self.adjusted_item_classifications.get(name) if you think there might be some items that will want more dynamically determined classifications in the future, which self.determine_item_classification(name) could be responsible for determining and then fall back to self.adjusted_item_classifications.get(name) for everything else.

[Mysteryem]

Something I've noticed when generating with many copies of the template yaml, is that when a world has both small keys and big keys in their own dungeons, with DRC in particular, this fill can occasionally (1/number of DRC locations) pick Dragon Roost Cavern - First Room to place DRC Big Key. This always forces the fill to go into swap after making such a placement.

Swap is slow, and made worse by the fact that all TWW players have their dungeon/any-dungeon items placed within a single fill_restrictive call, so single_player_placement=True cannot be used and there can be more items already placed in the fill_restrictive call to check for swapping, compared to if each TWW player had done their own individual fill

Changing the dungeon fill to perform one fill per player is normally not a good idea because the advantage of placing all the players' items with a single fill_restrictive call is that one item per player can be placed at the same time, making the fill faster if it does not have to swap.

So either you have a fill that is faster normally, but takes longer when swapping, or the opposite.

If it is easy to find which players have both Big Keys and Small Keys in their own dungeon and DRC enabled. You could add an extra item_rule to Dragon Roost Cavern - First Room for those players that prevents placing their DRC Big Key there, which could help with average generation times.

There is a worlds.generic.Rules.forbid_item() helper function that may be useful. It is specifically designed for forbidding placement of a specific item name belonging to a specific player.

I'm going to have a think about whether I can reorganise the dungeon fill into one fill per player somehow, while maintaining both good normal performance and better swap performance.

---

This dungeon fill in pre_fill sometimes going into swap will need to be taken into account by anyone trying to profile generation of TWW.
Generation with --skip_output and 24 template TWW yamls for me takes around 2.0s with zero swaps, but with 3 DRC Big Key swaps, the generation takes around 4.5s instead.

[tanjo3]

Would it be better instead to force DRC Small Key to Dragon Roost Cavern - First Room? If small keys are in their own dungeon, then that location must necessarily have a small key.

[Mysteryem]

Yes, manually placing a DRC Small Key there and removing it from the pool of dungeon items to place could work too. If you wait until pre_fill to place the small key, you'll need to check if the location has already been filled by item plando or by another world's pre_fill that was run earlier (AP meta-bingo can do this...). An early FillError could be raised in the case of the location already being filled.

Ideally, the manual placement of the item would occur as soon as possible, usually just after the location has been created, but create_items can work too, then it shouldn't be possible for another world or item plando to have already filled the location.

Manually placing the small key would not be strictly necessary in weird cases of things like players giving themselves extra DRC small keys in start inventory, but I think it's up to you whether you care enough about weird cases like that.

[Mysteryem]

I think items_received_2 and last_rcvd_index can be replaced with the items_received that is provided by CommonContext.

I ran the client with these changes and did not notice any issues.

[Mysteryem]

I don't think this ctx.items_received_2 is necessary. As far as I can tell, ctx.items_received already contains all received items in the order they were received. /received (ClientCommandProcessor._cmd_received()) in the client uses ctx.items_received to print all received items in order of acquisition.

Suggested change

	# Read the expected index of the player, which is the index of the latest item they've received.
	expected_idx = read_short(EXPECTED_INDEX_ADDR)
	# Loop through items to give.
	for item, idx in ctx.items_received_2:
	# If the item's index is greater than the player's expected index, give the player the item.
	if expected_idx <= idx:
	# Attempt to give the item and increment the expected index.
	while not _give_item(ctx, LOOKUP_ID_TO_NAME[item.item]):
	await asyncio.sleep(0.01)
	# Increment the expected index.
	write_short(EXPECTED_INDEX_ADDR, idx + 1)
	# Read the expected index of the player, which is the index of the next item they're expecting to receive.
	# The expected index starts at 0 for a fresh save file.
	expected_idx = read_short(EXPECTED_INDEX_ADDR)
	# Check if there are new items.
	received_items = ctx.items_received
	if len(received_items) <= expected_idx:
	# There are no new items.
	return
	# Loop through items to give.
	# Give the player all items at an index greater than or equal to the expected index.
	for idx, item in enumerate(received_items[expected_idx:], start=expected_idx):
	# Attempt to give the item and increment the expected index.
	while not _give_item(ctx, LOOKUP_ID_TO_NAME[item.item]):
	await asyncio.sleep(0.01)
	# Increment the expected index.
	write_short(EXPECTED_INDEX_ADDR, idx + 1)

[Mysteryem]

Removing items_received_2 (and last_rcvd_index)

Suggested change

	self.items_received_2 = []
	self.last_rcvd_index = -1

[Mysteryem]

Removing items_received_2 (and last_rcvd_index)

Suggested change

	elif cmd == "ReceivedItems":
	if args["index"] >= self.last_rcvd_index:
	self.last_rcvd_index = args["index"]
	for item in args["items"]:
	self.items_received_2.append((item, self.last_rcvd_index))
	self.last_rcvd_index += 1
	self.items_received_2.sort(key=lambda v: v[1])

[Mysteryem]

Removing items_received_2

Suggested change

self.items_received_2: list[tuple[NetworkItem, int]] = []

[Mysteryem]

Removing last_rcvd_index

Suggested change

self.last_rcvd_index: int = -1

[Mysteryem]

I think this change in rules is going to fall afoul of #4563

Either the rules would need to be reverted after the dungeon fill is complete (I think progression balancing won't currently balance locally placed items, but you might want to lock the filled locations anyway to make sure that no worlds or progression balancing in the future can then move the placed items to different locations outside the dungeon(s)), or these rules would need to be set in set_rules().