-
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?
Changes from all commits
f41a94b
49b4835
708848d
2fd5792
dcf5370
fb82f0c
b7aebef
23b6bd1
59d9d03
07e3ed1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,7 +86,7 @@ | |
| CLIFF_PLATEAU_ISLES_HIGHEST_ISLE_DUMMY_STAGE_NAME = "CliPlaH" | ||
|
|
||
| # Data storage key | ||
| AP_VISITED_STAGE_NAMES_KEY_FORMAT = "tww_visited_stages_%i" | ||
| AP_VISITED_STAGE_NAMES_KEY_FORMAT = "tww_visited_stages_%i_%i" | ||
|
|
||
|
|
||
| class TWWCommandProcessor(ClientCommandProcessor): | ||
|
|
@@ -207,13 +207,13 @@ def on_package(self, cmd: str, args: dict[str, Any]) -> None: | |
| if "death_link" in args["slot_data"]: | ||
| Utils.async_start(self.update_death_link(bool(args["slot_data"]["death_link"]))) | ||
| # Request the connected slot's dictionary (used as a set) of visited stages. | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % self.slot | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % (self.slot, self.team) | ||
| Utils.async_start(self.send_msgs([{"cmd": "Get", "keys": [visited_stages_key]}])) | ||
| elif cmd == "Retrieved": | ||
| requested_keys_dict = args["keys"] | ||
| # Read the connected slot's dictionary (used as a set) of visited stages. | ||
| if self.slot is not None: | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % self.slot | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % (self.slot, self.team) | ||
| if visited_stages_key in requested_keys_dict: | ||
| visited_stages = requested_keys_dict[visited_stages_key] | ||
| # If it has not been set before, the value in the response will be `None`. | ||
|
|
@@ -249,21 +249,38 @@ async def update_visited_stages(self, newly_visited_stage_name: str) -> None: | |
| """ | ||
| Update the server's data storage of the visited stage names to include the newly visited stage name. | ||
|
|
||
| This sends two types of messages: | ||
| 1. A dict-based update to tww_visited_stages_<slot> for PopTracker compatibility | ||
| 2. Individual SET messages for tww_<team>_<slot>_<stagename> to trigger server-side reconnection | ||
|
|
||
| :param newly_visited_stage_name: The name of the stage recently visited. | ||
| """ | ||
| if self.slot is not None: | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % self.slot | ||
| await self.send_msgs( | ||
| [ | ||
| { | ||
| "cmd": "Set", | ||
| "key": visited_stages_key, | ||
| "default": {}, | ||
| "want_reply": False, | ||
| "operations": [{"operation": "update", "value": {newly_visited_stage_name: True}}], | ||
| } | ||
| ] | ||
| ) | ||
| messages_to_send = [] | ||
|
|
||
| # Message 1: Update the visited_stages dict (for PopTracker) | ||
| visited_stages_key = AP_VISITED_STAGE_NAMES_KEY_FORMAT % (self.slot, self.team) | ||
| messages_to_send.append({ | ||
| "cmd": "Set", | ||
| "key": visited_stages_key, | ||
| "default": {}, | ||
| "want_reply": False, | ||
| "operations": [{"operation": "update", "value": {newly_visited_stage_name: True}}], | ||
| }) | ||
|
|
||
| # 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}], | ||
| }) | ||
|
Comment on lines
+271
to
+281
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating the PopTracker pack to change the datastorage key to include the team is like a 3 line change or so, it's very little work.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unless I hear otherwise, I'll presume you will go for
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ultimately, the tracker pack has to follow the client, so decide on the new datastorage key's format, and the poptracker pack will follow.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Implemented on latest commit. |
||
|
|
||
| await self.send_msgs(messages_to_send) | ||
|
|
||
| def update_salvage_locations_map(self) -> None: | ||
| """ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| """ | ||
| Universal Tracker support for The Wind Waker randomizer. | ||
|
|
||
| This module contains all Universal Tracker-specific logic including: | ||
| - Deferred entrance tracking and reconnection | ||
| - UT generation detection and handling | ||
| - Option restoration from UT slot_data | ||
| """ | ||
|
|
||
| from typing import Any, NamedTuple | ||
|
|
||
| # Universal Tracker multiworld attribute names | ||
| UT_RE_GEN_PASSTHROUGH_ATTR = "re_gen_passthrough" | ||
| UT_GENERATION_IS_FAKE_ATTR = "generation_is_fake" | ||
| UT_ENFORCE_DEFERRED_CONNECTIONS_ATTR = "enforce_deferred_connections" | ||
|
|
||
| # Wind Waker game name for UT identification | ||
| WIND_WAKER_GAME_NAME = "The Wind Waker" | ||
|
|
||
| class DatastorageParsed(NamedTuple): | ||
| """Parsed datastorage key for deferred entrance tracking.""" | ||
| team: int | ||
| player: int | ||
| stage_name: str | ||
|
|
||
| def is_ut_generation(multiworld: Any) -> bool: | ||
| """ | ||
| Check if the current generation is a Universal Tracker generation. | ||
|
|
||
| :param multiworld: The MultiWorld object. | ||
| :return: True if UT is active, False otherwise. | ||
| """ | ||
| return hasattr(multiworld, UT_GENERATION_IS_FAKE_ATTR) and getattr(multiworld, UT_GENERATION_IS_FAKE_ATTR) | ||
|
|
||
| def get_ut_slot_data(multiworld: Any, game_name: str = WIND_WAKER_GAME_NAME) -> dict[str, Any] | None: | ||
| """ | ||
| Get the slot_data from re_gen_passthrough for the given game. | ||
|
|
||
| :param multiworld: The MultiWorld object. | ||
| :param game_name: The game name to look up (defaults to "The Wind Waker"). | ||
| :return: The slot_data dict if available, None otherwise. | ||
| """ | ||
| re_gen_passthrough = getattr(multiworld, UT_RE_GEN_PASSTHROUGH_ATTR, {}) | ||
| if not re_gen_passthrough or game_name not in re_gen_passthrough: | ||
| return None | ||
| return re_gen_passthrough[game_name] | ||
|
|
||
| def should_defer_entrances(multiworld: Any, has_randomized_entrances: bool) -> bool: | ||
| """ | ||
| Check if entrances should be deferred based on UT and server settings. | ||
|
|
||
| :param multiworld: The MultiWorld object. | ||
| :param has_randomized_entrances: Whether the world has randomized entrances. | ||
| :return: True if entrances should be deferred, False otherwise. | ||
| """ | ||
| # Only defer entrances during UT regeneration | ||
| if not is_ut_generation(multiworld): | ||
| return False | ||
|
|
||
| # Check if server has deferred connections enabled | ||
| deferred_enabled = getattr( | ||
| multiworld, | ||
| UT_ENFORCE_DEFERRED_CONNECTIONS_ATTR, | ||
| None | ||
| ) in ("on", "default") | ||
|
|
||
| # Only defer if entrances are actually randomized | ||
| return deferred_enabled and has_randomized_entrances | ||
|
|
||
|
|
||
| def parse_datastorage_key(key: str) -> DatastorageParsed | None: | ||
| """ | ||
| Parse a datastorage key and return parsed components or None if invalid. | ||
|
|
||
| Expected format: tww_<team>_<player>_<stagename> | ||
|
|
||
| :param key: The datastorage key to parse. | ||
| :return: DatastorageParsed with team, player, stage_name, or None if invalid. | ||
| """ | ||
| parts = key.split("_") | ||
| min_parts = 4 | ||
| if len(parts) < min_parts or parts[0] != "tww": | ||
| return None | ||
| try: | ||
| team = int(parts[1]) | ||
| player = int(parts[2]) | ||
| stage_name = "_".join(parts[3:]) | ||
| return DatastorageParsed(team=team, player=player, stage_name=stage_name) | ||
| except (ValueError, IndexError): | ||
| return None |
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/3methods, that would normally get added ontoCollectionStatethrough theTWWLogicLogicMixin'sAutoLogicRegistermetaclass, with UT-specific versions that include theor self.has("Glitched", player, 1)checks (which can just beor self.has("Glitched", player)because1is 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
Uh oh!
There was an error while loading. Please reload this page.
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.
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.
Banjo Tooie isn't really comparable to Wind Waker because they set up their logic differently. In Banjo Tooie's case, their logic is all defined through methods on a specific class that the world creates an instance of. This means that a subclass of this class can exist for UT generation which overrides the methods that check logic difficulty to additionally check for the UT 'glitched' item. This means that generation with UT can just swap out the normal class for the UT-specific class. jjjj12212#3
Wind Waker, however, does not define its logic on a class, and the specific part that checks the logic difficulty is done through a LogicMixin, which is a special class that has its methods 'mixed into' the CollectionState class of Core AP. The actual class itself isn't used during generation, so subclassing it is useless.
What I think can be done for Wind Waker with Universal Tracker, is to define a new LogicMixin class when generation with Universal Tracker starts, to replace some of the methods in Wind Waker's normal LogicMixin to versions that check for UT's 'glitched' item. The important thing is that this UT-specific LogicMixin class must never be defined unless it is known that generation is occurring with Universal Tracker.
An alternative to defining a new LogicMixin class at runtime could be manually setting replacement methods onto the CollectionState class (basically manually implementing the behaviour of defining a LogicMixin class), but I feel like that would be slightly more prone to breaking if AP were to change how LogicMixins work in the future.
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 think I understand what you are saying here. I may as well confirm something. Are you allowing a refactor of the rules to go to a fully class approach here that may remove LogicMixins, or do you still wish to keep the LogicMixin approach? I don't want to jump the gun again with programming.
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.
Huh? I'm not saying any refactor should be done. Banjo Tooie and TWW set up their logic differently to one another, and that doesn't need to change.
I'm saying that when generation is occurring under Universal Tracker, I think it should work for the TWW world to define a new, UT-specific,
LogicMixinsubclass that causes the_tww_obscure/precise_#methods to be replaced with new methods that additionally check for the UT 'glitched' item.In
generate_early, or maybe even__init__if it is possible to detect that generation is occurring under UT at that point, the TWW world would define a new subclass ofLogicMixinwith each of the_tww_obscure/precise_#methods that include checking the UT 'glitched' item, causing the pre-existing mixed-in methods with the same names from TWW's normalLogicMixinsubclass to be replaced with the new methods.That way, TWW would use the methods from its normal
LogicMixinin normal generation, and only when generating with Universal Tracker would the Universal Tracker versions of_tww_obscure/precise_#get defined and replace the normal versions of those methods.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.
Ah, damn, I looked at LogicMixin's metaclass and it will error when attempting to define a new LogicMixin with some of the same methods defined, so I guess the replacing of the mixed-in methods on the CollectionState would have to be done manually.
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 haven't tested it, but my thought was something like this. The main problem I can think of is that your test that enables the glitched item won't work any more, and I wouldn't recommend calling
mix_in_universal_tracker_logic()from within tests because it makes permanent changes to theCollectionStateclass for the currently running Python process.A small test that the glitched logic is working could probably instead be added within
mix_in_universal_tracker_logic(), ifmix_in_universal_tracker_logic()is updated to take theTWWWorldinstance as an argument, so that aCollectionStatecan be created and tested against. Thelogic_obscure/precise_1/2/3attributes on theTWWWorldclass would probably also need to be changed to default toFalsefor a test to work because they are currently only defined on the instance at the end ofgenerate_early(), so testing against the mixed-in methods would raise anAttributeErrordue to the attributes not being defined on theTWWWorldinstance yet.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.
Implemented.