Core: Remove Universally Unique ID Requirements (Per-Game Data Packages)

[ThePhar]

What is this fixing or adding?

Note: This is a breaking change that may require some clients (and libraries) to handle potential ID conflicts.

The goal of this PR is to remove the requirement that item and location IDs must be unique across all worlds. IDs must still remain unique to their own world, but other worlds may now utilize that ID space for their implementations. This means it's expected for a client to check which game an ID is relevant for, prior to lookup.

This will allow any world developer to use more sensible IDs without need of a base_id_offset-type addition when building item and location tables, without worry of an ID conflict with another hypothetical world.

Clients that rely on looking up names for location and item IDs will most likely need to change how they perform these lookups to support this paradigm, otherwise, at best, names could be incorrect, and at worst, games will not function. The only changes to the Data Package itself will be the removal of version as this update also drops data_version support. If a client or library relies on version for data package caching, it will also stop working.

For library and client developers, the recommended way to perform name lookup for an ID is to check the relevant player's NetworkSlot data for their world, then lookup from within the corresponding game's GamePackage.

An example lookup scenario:

1. You connect and authenticate to the Archipelago server and the server responds with a Connected packet.

2. You locally cache the slot_info data from Connected packet for later lookup.

3. Eventually, you need to scout information in a location, so you send out a LocationScouts packet for location: 100.

4. Server responds with a LocationInfo packet with the following payload:

   [{
       "cmd": "LocationInfo",
       "locations": [{ 
           "item": 1, 
           "location": 100, 
           "player": 4, 
           "flags": 1
       }]
   }] 
   

5. You check player 4 in the slot_info data you locally cached on connection and find out they're playing Blique.

6. You can now do your lookup inside the Blique game package and get the name for item 1.

This change also increments the version of Archipelago to 0.5.0 considering the level of breaking change this contains.

How was this tested?

It hasn't, it's still in development.

If this makes graphical changes, please attach screenshots.

N/A

[black-sliver]

Other things that should probably be part of this PR

• [x] update docs and protocol
• [ ] define and require min client version
• [ ] reject upload of non-checksum seeds in webhost
• [ ] either upgrade the DB or disable spinning up of non-checksum seeds - upgrade could be done on demand or as a click. If we decide to disable old seeds instead, we should probably clean out the database at some point.
• [ ] remove code path from custom server and/or MultiServer that loads the Datapackage from worlds/

[ThePhar]

I know Berserker and I were talking about potentially deleting old seeds. @Berserker66 would probably have a better idea on the feasibility/practicality of upgrading the DB.

[ScipioWright]

The Noita mod/client now supports this change. It has a cache for the room to link up the slot number for each player with the game they are playing, and uses that as a reference for item/location names.

Edit: okay rereading this, this comment isn't super clear. I'm just following the "ideal way" as mentioned above with no real variation.

[el-u]

I don't disagree with the import and string changes in general, but including so many unrelated changes in this already large PR unnecessarily makes it less clear to read.

[ThePhar]

I don't disagree with the import and string changes in general, but including so many unrelated changes in this already large PR unnecessarily makes it less clear to read.

This PR is going to be renamed later. Part 1 of tracker clean up goes hand in hand with this PR. Not going to hackily make tracker.py work and then redo all my work again afterwards.

[ThePhar]

I'll be breaking the tracker updates into their own PR after work. Then once those are good, I can apply this on top since I know tracker will be fine. Should help narrow this scope.

[ThePhar]

Moving back to draft for a moment to make a change to the CommonContext.{type}_names interface.

[ThePhar]

Marking PR back as ready for review. There were changes to the CommonContext API.

[beauxq]

I'm getting this crash on this branch:

Traceback (most recent call last):
  File "Archipelago/CommonClient.py", line 729, in server_loop
    await process_server_cmd(ctx, msg)
  File "Archipelago/CommonClient.py", line 934, in process_server_cmd
    ctx.ui.update_hints()
  File "Archipelago/kvui.py", line 607, in update_hints
    self.log_panels["Hints"].refresh_hints(hints)
  File "Archipelago/kvui.py", line 686, in refresh_hints
    "item": {"text": self.parser.handle_node(
                     ^^^^^^^^^^^^^^^^^^^^^^^^
  File "Archipelago/NetUtils.py", line 213, in handle_node
    return handler(node)
           ^^^^^^^^^^^^^
  File "Archipelago/NetUtils.py", line 250, in _handle_item_id
    node["text"] = self.ctx.item_names.lookup_in_slot(item_id, node["player"])
                                                               ~~~~^^^^^^^^^^
KeyError: 'player'

It looks like it might be relevant that one of the games I'm testing with makes use of starting hints.

I don't have time right now to dig too deep.

[ThePhar]

I'll see if I can replicate shortly.

[ThePhar]

Yep it was hints, just had to pass along player info in the node. Seems to work for me now.