Skip to content
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

Core: prepare worlds.Files for APWorldContainer #4331

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 40 additions & 22 deletions worlds/Files.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,15 @@ class InvalidDataError(Exception):


class APContainer:
"""A zipfile containing at least archipelago.json"""
version: int = container_version
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."""
version: ClassVar[int] = container_version
compression_level: ClassVar[int] = 9
compression_method: ClassVar[int] = zipfile.ZIP_DEFLATED

# instance attributes:
path: Optional[str]
player: Optional[int]
player_name: str
server: str

def __init__(self, path: Optional[str] = None, player: Optional[int] = None,
player_name: str = "", server: str = ""):
def __init__(self, path: Optional[str] = None):
self.path = path
self.player = player
self.player_name = player_name
self.server = server

def write(self, file: Optional[Union[str, BinaryIO]] = None) -> None:
zip_file = file if file else self.path
Expand Down Expand Up @@ -135,31 +126,58 @@ def read(self, file: Optional[Union[str, BinaryIO]] = None) -> None:
message = f"{arg0} - "
raise InvalidDataError(f"{message}This might be the incorrect world version for this file") from e

def read_contents(self, opened_zipfile: zipfile.ZipFile) -> None:
def read_contents(self, opened_zipfile: zipfile.ZipFile) -> Dict[str, Any]:
Berserker66 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something I didn't notice before.
related to #4516 (comment)

Not only is this ambiguous to both mutate and return something, this is also a breaking change.
If this base class returns something in this method, every subclass that overrides it also needs to return the same type.
That could include apworlds that are not in this repo, as well as some in this repo.

If this changes here, it needs to change in every subclass, and we need to think about the deprecation for external apworlds.

with opened_zipfile.open("archipelago.json", "r") as f:
manifest = json.load(f)
if manifest["compatible_version"] > self.version:
raise Exception(f"File (version: {manifest['compatible_version']}) too new "
f"for this handler (version: {self.version})")
return manifest

def get_manifest(self) -> Dict[str, Any]:
return {
# minimum version of patch system expected for patching to be successful
"compatible_version": 5,
"version": container_version,
}


class APPlayerContainer(APContainer):
"""A zipfile containing at least archipelago.json meant for a player"""
game: ClassVar[Optional[str]] = None

player: Optional[int]
player_name: str
server: str

def __init__(self, path: Optional[str] = None, player: Optional[int] = None,
player_name: str = "", server: str = ""):
super().__init__(path)
self.player = player
self.player_name = player_name
self.server = server

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

def get_manifest(self) -> Dict[str, Any]:
return {
manifest = super().get_manifest()
manifest.update({
"server": self.server, # allow immediate connection to server in multiworld. Empty string otherwise
"player": self.player,
"player_name": self.player_name,
"game": self.game,
# minimum version of patch system expected for patching to be successful
"compatible_version": 5,
"version": container_version,
}
})
return manifest


class APPatch(APContainer):
class APPatch(APPlayerContainer):
"""
An `APContainer` that represents a patch file.
An `APPlayerContainer` that represents a patch file.
It includes the `procedure` key in the manifest to indicate that it is a patch.

Your implementation should inherit from this if your output file
Expand Down
2 changes: 1 addition & 1 deletion worlds/factorio/Mod.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
}


class FactorioModFile(worlds.Files.APContainer):
class FactorioModFile(worlds.Files.APPlayerContainer):
game = "Factorio"
compression_method = zipfile.ZIP_DEFLATED # Factorio can't load LZMA archives
writing_tasks: List[Callable[[], Tuple[str, Union[str, bytes]]]]
Expand Down
4 changes: 2 additions & 2 deletions worlds/kh2/OpenKH.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
from .Items import item_dictionary_table
from .Locations import all_locations, SoraLevels, exclusion_table
from .XPValues import lvlStats, formExp, soraExp
from worlds.Files import APContainer
from worlds.Files import APPlayerContainer


class KH2Container(APContainer):
class KH2Container(APPlayerContainer):
game: str = 'Kingdom Hearts 2'

def __init__(self, patch_data: dict, base_path: str, output_directory: str,
Expand Down
Loading