Skip to content

Conversation

@mharradon
Copy link

@mharradon mharradon commented Jul 15, 2021

PR for #36.

This PR converts backslash separated filenames in MPQs into directory trees rather than concatenated long filenames (I believe as intended by the MPQ format). In doing so it makes the library and its extraction results compatible with Windows.

There is a small interface change to share the file writing code across extracting "all files" and "some files". This does not affect command line usage.

Added minigame and test for extraction to disk for CollectMineralShards from PySC2. License from PySC2 included.

Tests confirmed passing on:

Mac: Python2.7 and 3.7
Windows: Python3.7

Michael Harradon added 6 commits July 15, 2021 15:07
…code reuse in extract_to_disk, add test with GatherMinerals.SC2Map and license. Should work on Python2.7, tested working on 3.7
Copy link
Owner

@eagleflo eagleflo left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Aside some minor nitpicks, I don't think target_dir should be a **kwargs style dictionary here. I'd propose just making it a normal parameter with a default argument of os.getcwd().

(I'll take the liberty to fixup your later commits into the main one to clean up git history.)

"""Extract all the files inside the MPQ archive in memory."""
if self.files:
return dict((f, self.read_file(f)) for f in self.files)
files = filenames if len(filenames)>0 else self.files
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: missing spaces around comparison operator

for filename in filenames:
data = self.read_file(filename)
f = open(filename, 'wb')
create_dir = os.path.join(target_dir, archive_name)
Copy link
Owner

Choose a reason for hiding this comment

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

create_dir could maybe be renamed to archive_dir to better reflect which directory it represents.


def extract(self):
def extract(self, *filenames):
"""Extract all the files inside the MPQ archive in memory."""
Copy link
Owner

Choose a reason for hiding this comment

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

This docstring is no longer necessarily true after the addition of *filenames.


def extract_to_disk(self):
def extract_to_disk(self, *filepaths, **target_dir):
"""Extract all files and write them to disk."""
Copy link
Owner

Choose a reason for hiding this comment

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

This docstring is no longer necessarily true after the addition of *filenames.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants