DS3: Added a few new items and locations

[Marechal-L]

trafficstars

What is this fixing or adding?

It adds new items and locations according to the players feedbacks on Discord

How was this tested?

Tested by me during several runs

[black-sliver]

This looks like it shifts IDs of existing locations and items? That results in wrong text/hint messages if you run a 0.3.5 seed on 0.3.6, also you may want to set required_client_version and make sure the new client (mod) correctly handles 0.3.5 locations/items.

[alwaysintreble]

This looks like it shifts IDs of existing locations and items? That results in wrong text/hint messages if you run a 0.3.5 seed on 0.3.6.

+1 this. this will be completely incompatible with 0.3.5 seeds so make sure that your client rejects 0.3.5 seeds and you add a required client version here so the server rejects 0.3.5 versioned client. in the future you can set base offsets per item and location section so that you have a gap in between each game so this doesn't happen. here's an example from my world https://github.com/alwaysintreble/Archipelago/blob/BatBoy/worlds/batboy/Locations.py

[Marechal-L]

Thanks for the review, I forgot to perform some update tests !

[Marechal-L]

Unfortunately I didn't manage to keep backward compatibility on this update. So :

• I set up required_client_version to 0.3.6 and updates the new client accordingly

• I added an offset between my items and locations as suggested to handle future updates

[black-sliver]

I have not looked at the new client, but if possible, you want to change the new client to either detect and work with old seeds, or detect and reject old seeds, telling the user to downgrade to finish the seed. That way nothing breaks whatever the user tries to do. (Considering that we have a running async with people playing DS3)

[Marechal-L]

The client relies entirely on the ids and data from the current seed and data_package so everything will work fine with the following combination -> old seed / old AP server and new seed / new AP server. There's no need to downgrade the client for the moment.

So I am not sure to understand your suggestion as I don't think telling to downgrade the AP server to finish a run would make sense for the player. And in this case updating the server will definitly break the current async :/

[alwaysintreble]

Because the data itself lives within the multidata you will have old seed / new AP server with every single AP update that happens as we support asynchronous play. Your client must absolutely support this or explicitly reject it.

[Marechal-L]

Oh yes sure! I will change my client to explicitly reject it

[Marechal-L]

Actually, as I also changed the way the client loads the seed (see https://github.com/ArchipelagoMW/Archipelago/pull/1155 ) the client became not compatible with old seeds.

[black-sliver]

Unless we want to merge it together with 1155 ?

[Marechal-L]

I think we should merge the features one by one. So let's merge this one first and I will set the other one as ready for review