Core: APWorld manifest #4516
Core: APWorld manifest #4516
Conversation
Co-authored-by: Doug Hoskisson <[email protected]>
github-actions
bot
added
affects: core
This import was done automatically by PyCharm, so I blame it personally.
Berserker66
added
is: enhancement
| apworld = APWorldContainer(str(zip_path)) | ||
| apworld.minimum_ap_version = version | ||
| apworld.maximum_ap_version = version | ||
| apworld.game = worldtype.game |
There was a problem hiding this comment.
probably should have some way to report world_version for frozen worlds as well to actually handle the beta apworld issues
worlds that want to keep their world version the same as ap's version could report their world_version as current ap + 1 and set their maximum_ap_version to current ap to make it always load over current ap but never overwrite any future release, so a default of ap's version would be fine too
but i imagine there would be some merged world devs that would want to manage their own version numbers as well
| def read_contents(self, opened_zipfile: zipfile.ZipFile) -> Dict[str, Any]: | ||
| manifest = super().read_contents(opened_zipfile) | ||
| self.player = manifest["player"] | ||
| self.server = manifest["server"] | ||
| self.player_name = manifest["player_name"] | ||
| return manifest |
There was a problem hiding this comment.
Isn't modifying self and returning something at the same time a bit ambiguous, especially without doc string explaining that?
There was a problem hiding this comment.
That's already how patch files work.
There was a problem hiding this comment.
Can we have a doc string?
"""
Reads a manifest from an opened zipfile, updates the APPlayerContainer object and returns the full manifest.
"""maybe?
There was a problem hiding this comment.
That's already how patch files work.
This looks wrong to me. In the main branch right now, none of the read_contents return anything.
There was a problem hiding this comment.
https://github.com/ArchipelagoMW/Archipelago/pull/4331/files#r1921592831
Since this is where it came in, I don't know if it's better to have the discussion there.
| compression_level: int = 9 | ||
| compression_method: int = zipfile.ZIP_DEFLATED | ||
| game: Optional[str] = None | ||
| """A zipfile containing at least archipelago.json, which contains a manifest json payload.""" |
There was a problem hiding this comment.
Something like "subclasses extend the contents of the manifest" would be good here imo.
|
|
||
|
|
||
| class APPlayerContainer(APContainer): | ||
| """A zipfile containing at least archipelago.json meant for a player""" |
Based on #4331, which I was really hoping to merge before working on this, but that's not how the cookie crumbled.
What is this fixing or adding?
Adds support for a manifest file (archipelago.json) inside an .apworld file. It tells AP the game, minimum core version (optional field), maximum core version (optional field), its own version (used to determine which file to prefer to load only currently)
The file itself is marked as required starting with core 0.7.0, prior, just a warning is printed, with error trace.
How was this tested?
a variety of local functional testing, such as building frozen AP on windows and loading launcher or joinking shapez and setting its minimum version to 0.7.0, by injecting:
and a bunch of others. However, this is something that can't be tested enough, so I'm putting it out now.
This could use some docs, which I hope someone is willing to contribute. Though, we should first cement if this is the API and behaviour we want.