From 42fecc749150ae2ca47a8a4b651b95e794afe68f Mon Sep 17 00:00:00 2001
From: black-sliver <59490463+black-sliver@users.noreply.github.com>
Date: Fri, 8 Apr 2022 11:16:36 +0200
Subject: [PATCH] Core: change how required versions work, deprecate IgnoreGame
 (#426)

`AutoWorld.World`s can set required_server_version and required_client_version properties. Drop `get_required_client_version()`.
`MultiServer` will set an absolute minimum client version based on its capability (protocol level).
`IgnoreVersion` tag is replaced by using `Tracker` or `TextOnly` with empty or null `game`.
Ignoring game will also ignore game's required_client_version (and fall back to server capability).
---
 CommonClient.py                 |  2 +-
 Main.py                         |  6 ++++--
 MultiServer.py                  |  9 ++++++---
 docs/api.md                     | 10 ++++------
 docs/network protocol.md        | 14 +++++++-------
 worlds/AutoWorld.py             | 20 +++++++++++++++++---
 worlds/alttp/__init__.py        |  4 +---
 worlds/factorio/__init__.py     |  4 +---
 worlds/meritous/__init__.py     |  7 +++----
 worlds/raft/__init__.py         |  4 +---
 worlds/rogue-legacy/__init__.py |  4 +---
 worlds/sm/__init__.py           |  9 ++++-----
 worlds/smz3/__init__.py         |  7 +++----
 worlds/soe/__init__.py          |  5 +----
 worlds/subnautica/__init__.py   |  5 +----
 15 files changed, 55 insertions(+), 55 deletions(-)

diff --git a/CommonClient.py b/CommonClient.py
index cc92e3f5..1603c87f 100644
--- a/CommonClient.py
+++ b/CommonClient.py
@@ -600,7 +600,7 @@ if __name__ == '__main__':
 
     class TextContext(CommonContext):
         tags = {"AP", "IgnoreGame", "TextOnly"}
-        game = "Archipelago"
+        game = ""  # empty matches any game since 0.3.2
         items_handling = 0  # don't receive any NetworkItems
 
         async def server_auth(self, password_requested: bool = False):
diff --git a/Main.py b/Main.py
index f02c3f4d..dcef4392 100644
--- a/Main.py
+++ b/Main.py
@@ -326,11 +326,13 @@ def main(args, seed=None, baked_server_options: Optional[Dict[str, object]] = No
                 slot_data = {}
                 client_versions = {}
                 games = {}
-                minimum_versions = {"server": (0, 2, 4), "clients": client_versions}
+                minimum_versions = {"server": AutoWorld.World.required_server_version, "clients": client_versions}
                 slot_info = {}
                 names = [[name for player, name in sorted(world.player_name.items())]]
                 for slot in world.player_ids:
-                    client_versions[slot] = world.worlds[slot].get_required_client_version()
+                    player_world: AutoWorld.World = world.worlds[slot]
+                    minimum_versions["server"] = max(minimum_versions["server"], player_world.required_server_version)
+                    client_versions[slot] = player_world.required_client_version
                     games[slot] = world.game[slot]
                     slot_info[slot] = NetUtils.NetworkSlot(names[0][slot - 1], world.game[slot],
                                                            world.player_types[slot])
diff --git a/MultiServer.py b/MultiServer.py
index dccb959f..537b6d8b 100644
--- a/MultiServer.py
+++ b/MultiServer.py
@@ -37,6 +37,7 @@ from Utils import get_item_name_from_id, get_location_name_from_id, \
 from NetUtils import Endpoint, ClientStatus, NetworkItem, decode, encode, NetworkPlayer, Permission, NetworkSlot, \
     SlotType
 
+min_client_version = Version(0, 1, 6)
 colorama.init()
 
 # functions callable on storable data on the server by clients
@@ -301,7 +302,7 @@ class Context:
         clients_ver = decoded_obj["minimum_versions"].get("clients", {})
         self.minimum_client_versions = {}
         for player, version in clients_ver.items():
-            self.minimum_client_versions[player] = Utils.Version(*version)
+            self.minimum_client_versions[player] = max(Utils.Version(*version), min_client_version)
 
         self.clients = {}
         for team, names in enumerate(decoded_obj['names']):
@@ -1388,9 +1389,11 @@ async def process_client_cmd(ctx: Context, client: Client, args: dict):
         else:
             team, slot = ctx.connect_names[args['name']]
             game = ctx.games[slot]
-            if "IgnoreGame" not in args["tags"] and args['game'] != game:
+            ignore_game = "IgnoreGame" in args["tags"] or (  # IgnoreGame is deprecated. TODO: remove after 0.3.3?
+                          ("TextOnly" in args["tags"] or "Tracker" in args["tags"]) and not args.get("game"))
+            if not ignore_game and args['game'] != game:
                 errors.add('InvalidGame')
-            minver = ctx.minimum_client_versions[slot]
+            minver = min_client_version if ignore_game else ctx.minimum_client_versions[slot]
             if minver > args['version']:
                 errors.add('IncompatibleVersion')
             if args.get('items_handling', None) is None:
diff --git a/docs/api.md b/docs/api.md
index 13bb14bb..2f330eb9 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -398,7 +398,7 @@ The world has to provide the following things for generation
   `self.world.get_filled_locations(self.player)` will filter for this world.
   `item.player` can be used to see if it's a local item.
 
-In addition the following methods can be implemented
+In addition, the following methods can be implemented and attributes can be set
 
 * `def generate_early(self)`
   called per player before any items or locations are created. You can set
@@ -418,11 +418,9 @@ In addition the following methods can be implemented
   before, during and after the regular fill process, before `generate_output`.
 * `fill_slot_data` and `modify_multidata` can be used to modify the data that
   will be used by the server to host the MultiWorld.
-* `def get_required_client_version(self)`
-  can return a tuple of 3 ints to make sure the client is compatible to this
-  world (e.g. item IDs) when connecting.
-  Always use `return max((x,y,z), super().get_required_client_version())`
-  to catch updates in the lower layers that break compatibility.
+* `required_client_version: Tuple(int, int, int)`
+  Client version as tuple of 3 ints to make sure the client is compatible to
+  this world (e.g. implements all required features) when connecting.
 
 #### generate_early
 
diff --git a/docs/network protocol.md b/docs/network protocol.md
index 509b8f2c..ae93e369 100644
--- a/docs/network protocol.md	
+++ b/docs/network protocol.md	
@@ -583,13 +583,13 @@ GameData is a **dict** but contains these keys and values. It's broken out into
 ### Tags
 Tags are represented as a list of strings, the common Client tags follow:
 
-| Name | Notes |
-| ----- | ---- |
-| AP | Signifies that this client is a reference client, its usefulness is mostly in debugging to compare client behaviours more easily. |
-| IgnoreGame | Tells the server to ignore the "game" attribute in the [Connect](#Connect) packet. |
-| DeathLink | Client participates in the DeathLink mechanic, therefore will send and receive DeathLink bounce packets |
-| Tracker | Tells the server that this client is actually a Tracker, will refuse new locations from this client and send all items as if they were remote items. |
-| TextOnly | Tells the server that this client will not send locations and does not want to receive items. |
+| Name       | Notes                                                                                                                                                                                                              |
+|------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| AP         | Signifies that this client is a reference client, its usefulness is mostly in debugging to compare client behaviours more easily.                                                                                  |
+| IgnoreGame | Deprecated. See Tracker and TextOnly. Tells the server to ignore the "game" attribute in the [Connect](#Connect) packet.                                                                                           |
+| DeathLink  | Client participates in the DeathLink mechanic, therefore will send and receive DeathLink bounce packets                                                                                                            |
+| Tracker    | Tells the server that this client will not send locations and is actually a Tracker. When specified and used with empty or null `game` in [Connect](#connect), game and game's version validation will be skipped. |
+| TextOnly   | Tells the server that this client will not send locations and is intended for chat. When specified and used with empty or null `game` in [Connect](#connect), game and game's version validation will be skipped.  |
 
 ### DeathLink
 A special kind of Bounce packet that can be supported by any AP game. It targets the tag "DeathLink" and carries the following data:
diff --git a/worlds/AutoWorld.py b/worlds/AutoWorld.py
index f1a744c6..6e326897 100644
--- a/worlds/AutoWorld.py
+++ b/worlds/AutoWorld.py
@@ -27,6 +27,15 @@ class AutoWorldRegister(type):
         dct["location_names"] = frozenset(dct["location_name_to_id"])
         dct["all_item_and_group_names"] = frozenset(dct["item_names"] | set(dct.get("item_name_groups", {})))
 
+        # move away from get_required_client_version function
+        if "game" in dct:
+            assert "get_required_client_version" not in dct, f"{name}: required_client_version is an attribute now"
+        # set minimum required_client_version from bases
+        if "required_client_version" in dct and bases:
+            for base in bases:
+                if "required_client_version" in base.__dict__:
+                    dct["required_client_version"] = max(dct["required_client_version"], base.required_client_version)
+
         # construct class
         new_class = super().__new__(cls, name, bases, dct)
         if "game" in dct:
@@ -106,6 +115,14 @@ class World(metaclass=AutoWorldRegister):
     # retrieved by clients on every connection.
     data_version: int = 1
 
+    # override this if changes to a world break forward-compatibility of the client
+    # The base version of (0, 1, 6) is provided for backwards compatibility and does *not* need to be updated in the
+    # future. Protocol level compatibility check moved to MultiServer.min_client_version.
+    required_client_version: Tuple[int, int, int] = (0, 1, 6)
+
+    # update this if the resulting multidata breaks forward-compatibility of the server
+    required_server_version: Tuple[int, int, int] = (0, 2, 4)
+
     hint_blacklist: Set[str] = frozenset()  # any names that should not be hintable
 
     # NOTE: remote_items and remote_start_inventory are now available in the network protocol for the client to set.
@@ -191,9 +208,6 @@ class World(metaclass=AutoWorldRegister):
         """For deeper modification of server multidata."""
         pass
 
-    def get_required_client_version(self) -> Tuple[int, int, int]:
-        return 0, 1, 6
-
     # Spoiler writing is optional, these may not get called.
     def write_spoiler_header(self, spoiler_handle: TextIO):
         """Write to the spoiler header. If individual it's right at the end of that player's options,
diff --git a/worlds/alttp/__init__.py b/worlds/alttp/__init__.py
index 301dc4b3..1a3453f9 100644
--- a/worlds/alttp/__init__.py
+++ b/worlds/alttp/__init__.py
@@ -43,6 +43,7 @@ class ALTTPWorld(World):
     data_version = 8
     remote_items: bool = False
     remote_start_inventory: bool = False
+    required_client_version = (0, 3, 2)
 
     set_rules = set_rules
 
@@ -324,9 +325,6 @@ class ALTTPWorld(World):
             new_name = base64.b64encode(bytes(self.rom_name)).decode()
             multidata["connect_names"][new_name] = multidata["connect_names"][self.world.player_name[self.player]]
 
-    def get_required_client_version(self) -> tuple:
-        return max((0, 3, 2), super(ALTTPWorld, self).get_required_client_version())
-
     def create_item(self, name: str) -> Item:
         return ALttPItem(name, self.player, **as_dict_item_table[name])
 
diff --git a/worlds/factorio/__init__.py b/worlds/factorio/__init__.py
index cb41b027..02665391 100644
--- a/worlds/factorio/__init__.py
+++ b/worlds/factorio/__init__.py
@@ -42,6 +42,7 @@ class Factorio(World):
         "Progressive": set(progressive_tech_table.values()),
     }
     data_version = 5
+    required_client_version = (0, 3, 0)
 
     def __init__(self, world, player: int):
         super(Factorio, self).__init__(world, player)
@@ -179,9 +180,6 @@ class Factorio(World):
 
         return super(Factorio, self).collect_item(state, item, remove)
 
-    def get_required_client_version(self) -> tuple:
-        return max((0, 2, 6), super(Factorio, self).get_required_client_version())
-
     options = factorio_options
 
     @classmethod
diff --git a/worlds/meritous/__init__.py b/worlds/meritous/__init__.py
index e8715de6..24b9808b 100644
--- a/worlds/meritous/__init__.py
+++ b/worlds/meritous/__init__.py
@@ -32,6 +32,9 @@ class MeritousWorld(World):
     data_version = 2
     forced_auto_forfeit = False
 
+    # NOTE: Remember to change this before this game goes live
+    required_client_version = (0, 2, 4)
+
     options = meritous_options
 
     def __init__(self, world: MultiWorld, player: int):
@@ -150,10 +153,6 @@ class MeritousWorld(World):
             self.world.completion_condition[self.player] = lambda state: state.has(
                 "Full Victory", self.player)
 
-    def get_required_client_version(self) -> tuple:
-        # NOTE: Remember to change this before this game goes live
-        return max((0, 2, 4), super(MeritousWorld, self).get_required_client_version())
-
     def fill_slot_data(self) -> dict:
         return {
             "goal": self.goal,
diff --git a/worlds/raft/__init__.py b/worlds/raft/__init__.py
index 6f0dd5ef..8a4a6443 100644
--- a/worlds/raft/__init__.py
+++ b/worlds/raft/__init__.py
@@ -27,6 +27,7 @@ class RaftWorld(World):
     options = raft_options
 
     data_version = 1
+    required_client_version = (0, 2, 0)
 
     def generate_basic(self):
         minRPSpecified = self.world.minimum_resource_pack_amount[self.player].value
@@ -111,9 +112,6 @@ class RaftWorld(World):
 
         return super(RaftWorld, self).collect_item(state, item, remove)
 
-    def get_required_client_version(self) -> typing.Tuple[int, int, int]:
-        return max((0, 2, 0), super(RaftWorld, self).get_required_client_version())
-    
     def pre_fill(self):
         if self.world.island_frequency_locations[self.player] == 0:
             self.setLocationItem("Radio Tower Frequency to Vasagatan", "Vasagatan Frequency")
diff --git a/worlds/rogue-legacy/__init__.py b/worlds/rogue-legacy/__init__.py
index 381f6ea4..32348494 100644
--- a/worlds/rogue-legacy/__init__.py
+++ b/worlds/rogue-legacy/__init__.py
@@ -21,6 +21,7 @@ class LegacyWorld(World):
     options = legacy_options
     topology_present = False
     data_version = 3
+    required_client_version = (0, 2, 3)
 
     item_name_to_id = {name: data.code for name, data in item_table.items()}
     location_name_to_id = location_table
@@ -54,9 +55,6 @@ class LegacyWorld(World):
         data = item_table[name]
         return [self.create_item(name)] * data.quantity
 
-    def get_required_client_version(self) -> typing.Tuple[int, int, int]:
-        return max((0, 2, 3), super(LegacyWorld, self).get_required_client_version())
-
     def fill_slot_data(self) -> dict:
         slot_data = self._get_slot_data()
         for option_name in legacy_options:
diff --git a/worlds/sm/__init__.py b/worlds/sm/__init__.py
index d9b13753..305ac87b 100644
--- a/worlds/sm/__init__.py
+++ b/worlds/sm/__init__.py
@@ -69,6 +69,10 @@ class SMWorld(World):
     remote_items: bool = False
     remote_start_inventory: bool = False
 
+    # changes to client DeathLink handling for 0.2.1
+    # changes to client Remote Item handling for 0.2.6
+    required_client_version = (0, 2, 6)
+
     itemManager: ItemManager
 
     locations = {}
@@ -167,11 +171,6 @@ class SMWorld(World):
         create_locations(self, self.player)
         create_regions(self, self.world, self.player)
 
-    def get_required_client_version(self):
-        # changes to client DeathLink handling for 0.2.1
-        # changes to client Remote Item handling for 0.2.6
-        return max(super(SMWorld, self).get_required_client_version(), (0, 2, 6))
-
     def getWord(self, w):
         return (w & 0x00FF, (w & 0xFF00) >> 8)
     
diff --git a/worlds/smz3/__init__.py b/worlds/smz3/__init__.py
index cafde61b..6cb43878 100644
--- a/worlds/smz3/__init__.py
+++ b/worlds/smz3/__init__.py
@@ -51,6 +51,9 @@ class SMZ3World(World):
     remote_items: bool = False
     remote_start_inventory: bool = False
 
+    # first added for 0.2.6
+    required_client_version = (0, 2, 6)
+
     def __init__(self, world: MultiWorld, player: int):
         self.rom_name_available_event = threading.Event()
         self.locations = {}
@@ -135,10 +138,6 @@ class SMZ3World(World):
             startRegion.exits.append(exit)
             exit.connect(currentRegion)
 
-    def get_required_client_version(self):
-        # first added for 0.2.6
-        return max(super(SMZ3World, self).get_required_client_version(), (0, 2, 6))
-
     def apply_sm_custom_sprite(self):
         itemSprites = ["off_world_prog_item.bin", "off_world_item.bin"]
         itemSpritesAddress = [0xF800, 0xF900]
diff --git a/worlds/soe/__init__.py b/worlds/soe/__init__.py
index c614d9ad..244c6f07 100644
--- a/worlds/soe/__init__.py
+++ b/worlds/soe/__init__.py
@@ -147,6 +147,7 @@ class SoEWorld(World):
     remote_items = False
     data_version = 2
     web = SoEWebWorld()
+    required_client_version = (0, 2, 6)
 
     item_name_to_id, item_id_to_raw = _get_item_mapping()
     location_name_to_id, location_id_to_raw = _get_location_mapping()
@@ -321,10 +322,6 @@ class SoEWorld(World):
             payload = multidata["connect_names"][self.world.player_name[self.player]]
             multidata["connect_names"][self.connect_name] = payload
 
-    def get_required_client_version(self):
-        return max((0, 2, 6), super(SoEWorld, self).get_required_client_version())
-
-
 class SoEItem(Item):
     game: str = "Secret of Evermore"
 
diff --git a/worlds/subnautica/__init__.py b/worlds/subnautica/__init__.py
index 0a3c9c2e..92890521 100644
--- a/worlds/subnautica/__init__.py
+++ b/worlds/subnautica/__init__.py
@@ -28,6 +28,7 @@ class SubnauticaWorld(World):
     options = options
 
     data_version = 2
+    required_client_version = (0, 1, 9)
 
     def generate_basic(self):
         # Link regions
@@ -77,10 +78,6 @@ class SubnauticaWorld(World):
         item = lookup_name_to_item[name]
         return SubnauticaItem(name, item["progression"], item["id"], player=self.player)
 
-    def get_required_client_version(self) -> typing.Tuple[int, int, int]:
-        return max((0, 1, 9), super(SubnauticaWorld, self).get_required_client_version())
-
-
 def create_region(world: MultiWorld, player: int, name: str, locations=None, exits=None):
     ret = Region(name, None, name, player)
     ret.world = world