-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Wind Waker: Support for Universal Tracker. #5818
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.
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
base: main
Are you sure you want to change the base?
Wind Waker: Support for Universal Tracker. #5818
Conversation
This adds the following features for Universal Tracker: * Sequence break support: UT now recognizes which checks you can get out of sequence without completely breaking the game. Only the checks involving Levels 1-3 of Obscure and Precision are in here: Barrier Skip is not considered a valid sequence break. A unit test demonstrating the basics of how UT interprets sequence breaks is provided. * YAML-less Generation: We now have the ability to fully parse the options from the original YAML file and fill the slots as appropriate. * Entrance Shuffle Support: You are now told what rooms have items that you can access. If you are unsure of the path, use `/get_logical_path` or `/explain` in UT. * Opt-In Deferred Entrance Support: We parse the game's memory to temporarily disconnect entrances in UT. At the bottom of the checks you know you have access to, you will be told of entrances you can access but haven't yet. Once you access them for the first time, the entrances are connected and the check lists behave as before. If you do not wish to have deferred entrances in your games, you can always set your `host.yaml` file to exclude them.
Mysteryem
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes that need to be made, a question about datastorage keys, and a thought about the LogicMixin.
I have not yet had a deeper look at the actual implementation of unrandomizing random options from slot data, or the new tests.
From running the UT fuzzer hook I didn't see any UT-related failures.
| # Message 2: Set individual stage key to trigger server-side world callback | ||
| # This matches the format expected by reconnect_found_entrances(): <GAME_ABBR>_<team>_<slot>_<stagename> | ||
| if self.team is not None: | ||
| stage_key = f"tww_{self.team}_{self.slot}_{newly_visited_stage_name}" | ||
| messages_to_send.append({ | ||
| "cmd": "Set", | ||
| "key": stage_key, | ||
| "default": 0, | ||
| "want_reply": False, | ||
| "operations": [{"operation": "replace", "value": 1}], | ||
| }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PopTracker pack can already connect entrances using just the list of visited stages, why are all these new datastorage keys needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was following the example of Waffles: https://github.com/TheLX5/Archipelago/blob/waffles/worlds/waffles/Client.py#L458
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't really answer my question. Is there some technical requirement of Universal Tracker for these new datastorage keys to be required?
Either way, the client shouldn't be duplicating the same visited stages data across both the existing list datastorage key and all the new individual datastorage keys, so one of the two should be removed, though it is not clear to me which one should be removed currently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on earlier Discord discussion, we can hopefully resolve this one if the original message sent also contains room for the (mostly unused by all except devs) team parameter. How easy is it to update poptracker to account for that?
worlds/tww/Options.py
Outdated
| "randomize_mapcompass", | ||
| "randomize_smallkeys", | ||
| "randomize_bigkeys", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not relevant to logic, so do not need to be included in slot data for UT support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possible misunderstanding here, but why would these not be relevant to logic? The location of the keys, at least, kind of determines which places you can go, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these options do is control where the keys get placed in the multiworld. The keys are real AP items and not events, so Universal Tracker discards them from its internal generation. When the key items get received from the multiworld, then Universal Tracker's logic updates to account for the keys.
worlds/tww/Options.py
Outdated
| "num_required_bosses", | ||
| "included_dungeons", | ||
| "excluded_dungeons", | ||
| "chest_type_matches_contents", | ||
| "hero_mode", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not relevant to logic, so do not need to be included in slot data UT support.
In the case of num_required_bosses, generation with UT can calculate the count based on the number of required bosses enabled that this PR is putting into slot data.
worlds/tww/Options.py
Outdated
| "mix_entrances", | ||
| "randomize_enemies", | ||
| "randomize_starting_island", | ||
| "randomize_charts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are not relevant to logic, so do not need to be included in slot data.
In the case of mix_entrances, generation with UT can just assume that entrances are always mixed, because all non-mixed ER combinations are a subset of mixed ER combinations.
In the case of randomize_charts, unless you're intending to add deferred entrance support for randomized charts, where entrances would only be connected when the player opens the chart in-game, generation with UT can just assume that charts are always randomized, because non-randomized charts is a possible randomized charts combination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I debated deferred entrance support for randomized charts, but considering that you can either easily open up the charts in a few seconds or a future option to just have them pre-opened, I didn't feel that was worth it.
Mix entrances, I get.
Randomize Starting Island, I am torn. If you don't start with the Swift Sail or Ballad of Gales, and get placed in an island with no accessible checks, doesn't that cause a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Randomize Starting Island does not affect the world's logic in any way.
Playing without the Swift Sail is not really recommended, but even if you don't have the Swift Sail enabled, you can still slowly cruise across the ocean like when you don't have the sail out.
worlds/tww/__init__.py
Outdated
| slot_data["charts"] = charts_mapping | ||
|
|
||
| # Add required bosses information. | ||
| slot_data["required_bosses"] = self.boss_reqs.required_boss_item_locations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"required_bosses" is already in use for the Required Bosses option, please use a different name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like slowing down regular generation just to add Universal Tracker support.
I'm not sure if there is a good solution to this, but it might be possible to, when generating with UT, replace the _tww_obscure/precise_1/2/3 methods, that would normally get added onto CollectionState through the TWWLogic LogicMixin's AutoLogicRegister metaclass, with UT-specific versions that include the or self.has("Glitched", player, 1) checks (which can just be or self.has("Glitched", player) because 1 is the default count).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did that pattern similar to what was done for Ship of Harkinian, and I was explicitly suggested to place it in the trick section. https://github.com/lonegraywolf2000/Archipelago-SoH/blob/oot-soh/worlds/oot_soh/LogicHelpers.py#L387
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If they want to put up with their regular generation being slower just to support UT glitched logic, then that's up to them.
Ideally, there would be a better way where UT glitched logic is only checked when generation is occurring under UT. Depending on how worlds have set up their logic, separating out the UT glitched logic functions could be easier or more difficult.
If there is a way to support UT glitched logic without slowing down regular generations, and without being highly complicated, then I think it would be a good thing to do.
This is similar to the idea of making logic rules that never needs to check what Options are enabled because different logic is set depending on what the enabled Options are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on earlier discord discussions, I may want to see how you handle things in the future with Banjo Tooie before bringing it over.
To copy/paraphrase from Mysteryem, all these do is just place where everything goes. The keys/maps/compasses are real AP items. They are NOT events. Universal Tracker can handle these with no issues.
What is this fixing or adding?
This adds the following features for Universal Tracker:
/get_logical_pathor/explainin UT.host.yamlfile to exclude them.How was this tested?
Multiple seed generations were created, with each seed being loaded into both Universal Tracker and Dolphin for extensive comparing and contrasting.
Logs were previously present to ensure the data coming through was correct, but these were removed for the production push.
The unit test for sequence break support is based off of one I wrote for the Ship of Harkinian APWorld, and that was verified to work.
If this makes graphical changes, please attach screenshots.
While I didn't make a graphical change directly, I do want to show the result of dungeon entrance shuffle in Universal Tracker.

Additional credits go here I guess.