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

Add aoc directory support to botw-patcher #1

Merged
merged 1 commit into from
Sep 29, 2018
Merged

Add aoc directory support to botw-patcher #1

merged 1 commit into from
Sep 29, 2018

Conversation

bravely-beep
Copy link
Contributor

@bravely-beep bravely-beep commented Sep 21, 2018

When running botw-patcher, you can now specify an aoc directory too:

patcher  ORIGINAL_CONTENT_DIR   MOD_DIR  TARGET_DIR  --target {wiiu,switch}
         [aoc  ORIGINAL_AOC_DIR   MOD_AOC_DIR   TARGET_AOC_DIR]

It automatically handles the conditions for adding the 'Aoc/0010/' prefix, and saves the RSTB for both the content and aoc directories file correctly, into the non-aoc TARGET_DIR.

@@ -230,8 +237,16 @@ def cli_main() -> None:
parser.add_argument('target_dir', type=Path, help='Path to the target directory')
parser.add_argument('-f', '--force', action='store_true', help='Clean up the target directory if it exists')
parser.add_argument('-t', '--target', choices=['wiiu', 'switch'], help='Target platform', required=True)
subparsers = parser.add_subparsers()

aoc_parser = subparsers.add_parser('aoc', help='Use a separate add-on-content directory as well')
Copy link
Member

@leoetlino leoetlino Sep 21, 2018

Choose a reason for hiding this comment

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

Hmm, using a subparser sounds like a hack :p (it's intended to be used for subcommands, like sarc {create,update} for example, not optional arguments)
Could you replace this with --aoc-dir + --aoc-patch-dir + --aoc-target-dir? Having 6 positional arguments, separated with only aoc would also make commands too hard to read IMO.

make_loadable_layer(args.content_dir, args.patch_dir, args.target_dir,
wiiu=(args.target == 'wiiu'))

wiiu=(args.target == 'wiiu')
Copy link
Member

Choose a reason for hiding this comment

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

wiiu = args.target == 'wiiu'

table = rstb.util.read_rstb(args.content_dir / _RSTB_PATH_IN_CONTENT, be=wiiu)

make_loadable_layer(args.content_dir, args.patch_dir, args.target_dir, wiiu, table, is_aoc=False)
try:
Copy link
Member

@leoetlino leoetlino Sep 21, 2018

Choose a reason for hiding this comment

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

After removing the subparser thing, exception handling can be replaced with a simpler, less surprising if check.


ARCHIVE_EXTS = {'sarc', 'pack', 'bactorpack', 'bmodelsh', 'beventpack', 'stera', 'stats',
'ssarc', 'spack', 'sbactorpack', 'sbmodelsh', 'sbeventpack', 'sstera', 'sstats',
'blarc', 'sblarc', 'genvb', 'sgenvb'}
AOC_PREFIX = 'Aoc/0010/'
AOC_PREFIX_CONDITIONS = {'Terrain\/A\/AocField.*', 'Pack\/.*.pack', 'UI\/StaffRollDLC\/.*',
Copy link
Member

@leoetlino leoetlino Sep 21, 2018

Choose a reason for hiding this comment

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

This should be a tuple or a list; it should not be a set.

Also, the list of conditions is technically not entirely correct. This will match all dungeon resources and all voice streams for example. The dungeon number needs to be checked like how Nintendo does it. Here is a fairly accurate re-implementation of the logic.

(Using regexes for that many strings is also going to be pretty expensive when you have many resources to patch. Best to use simple string comparisons for most cases and save regex/more complex parsing for the special cases)


# File is not in any archive, so just return the path relative to the content root.
return rel_path.as_posix()
resource_path = rel_path.as_posix()
Copy link
Member

@leoetlino leoetlino Sep 21, 2018

Choose a reason for hiding this comment

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

Since you've removed the return, the execution will always reach this line, meaning the correct value of resource_path for archived files will always be overwritten.

I suggest keeping the returns and just adding get_path() around the returned strings, where get_path is a local function that checks for is_aoc and the AoC conditions (see an earlier comment) and adds the prefix if needed. (Feel free to change the name to something better)

'Physics\/StaticCompound\/CDungeon\/.*', 'Movie\/Demo/.*', 'Game\/AocField\/.*', 'NavMesh\/AocField\/.*',
'NavMesh\/MainFieldDungeon\/.*', 'NavMesh\/CDungeon\/.*', 'Physics\/TeraMeshRigidBody\/AocField\/.*',
'Voice\/.*\/Stream_Demo\/.*\/.*\.bfstm', 'System\/AocVersion\.txt'}
RE_PATTERNS = {re.compile(string) for string in AOC_PREFIX_CONDITIONS}
Copy link
Member

@leoetlino leoetlino Sep 21, 2018

Choose a reason for hiding this comment

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

This doesn't have to be a set either. A set is a specific data structure you use when you need fast inserts, removals or "is element in the set" checks. You're just iterating over the compiled regexp objects so a simple list is the best structure.

@bravely-beep
Copy link
Contributor Author

bravely-beep commented Sep 27, 2018

The issues raised should now be fixed.

if AOC_VOICE_PATTERN.match(new_path):
return AOC_PREFIX + new_path

dungeon_num_str = re.search('"Dungeon\((.+?)\)', new_path)
Copy link
Member

Choose a reason for hiding this comment

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

typo (there's an extra quote)

@leoetlino leoetlino merged commit c451215 into zeldamods:master Sep 29, 2018
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