LttP: Fixes patching on a fresh AP install (#2118)

LttPAdjuster.py

@@ -25,7 +25,7 @@ ModuleUpdate.update()
 
 from worlds.alttp.Rom import Sprite, LocalRom, apply_rom_settings, get_base_rom_bytes
 from Utils import output_path, local_path, user_path, open_file, get_cert_none_ssl_context, persistent_store, \
-    get_adjuster_settings, tkinter_center_window, init_logging
+    get_adjuster_settings, get_adjuster_settings_no_defaults, tkinter_center_window, init_logging
 
 
 GAME_ALTTP = "A Link to the Past"
@@ -43,6 +43,47 @@ class ArgumentDefaultsHelpFormatter(argparse.RawTextHelpFormatter):
     def _get_help_string(self, action):
         return textwrap.dedent(action.help)
 
+# See argparse.BooleanOptionalAction
+class BooleanOptionalActionWithDisable(argparse.Action):
+    def __init__(self,
+                 option_strings,
+                 dest,
+                 default=None,
+                 type=None,
+                 choices=None,
+                 required=False,
+                 help=None,
+                 metavar=None):
+
+        _option_strings = []
+        for option_string in option_strings:
+            _option_strings.append(option_string)
+
+            if option_string.startswith('--'):
+                option_string = '--disable' + option_string[2:]
+                _option_strings.append(option_string)
+
+        if help is not None and default is not None:
+            help += " (default: %(default)s)"
+
+        super().__init__(
+            option_strings=_option_strings,
+            dest=dest,
+            nargs=0,
+            default=default,
+            type=type,
+            choices=choices,
+            required=required,
+            help=help,
+            metavar=metavar)
+
+    def __call__(self, parser, namespace, values, option_string=None):
+        if option_string in self.option_strings:
+            setattr(namespace, self.dest, not option_string.startswith('--disable'))
+
+    def format_usage(self):
+        return ' | '.join(self.option_strings)
+
 
 def get_argparser() -> argparse.ArgumentParser:
     parser = argparse.ArgumentParser(formatter_class=ArgumentDefaultsHelpFormatter)
@@ -52,6 +93,8 @@ def get_argparser() -> argparse.ArgumentParser:
                         help='Path to an ALttP Japan(1.0) rom to use as a base.')
     parser.add_argument('--loglevel', default='info', const='info', nargs='?',
                         choices=['error', 'info', 'warning', 'debug'], help='Select level of logging for output.')
+    parser.add_argument('--auto_apply', default='ask',
+                        choices=['ask', 'always', 'never'], help='Whether or not to apply settings automatically in the future.')
     parser.add_argument('--menuspeed', default='normal', const='normal', nargs='?',
                         choices=['normal', 'instant', 'double', 'triple', 'quadruple', 'half'],
                         help='''\
@@ -61,7 +104,7 @@ def get_argparser() -> argparse.ArgumentParser:
     parser.add_argument('--quickswap', help='Enable quick item swapping with L and R.', action='store_true')
     parser.add_argument('--deathlink', help='Enable DeathLink system.', action='store_true')
     parser.add_argument('--allowcollect', help='Allow collection of other player items', action='store_true')
-    parser.add_argument('--disablemusic', help='Disables game music.', action='store_true')
+    parser.add_argument('--music', default=True, help='Enables/Disables game music.', action=BooleanOptionalActionWithDisable)
     parser.add_argument('--triforcehud', default='hide_goal', const='hide_goal', nargs='?',
                         choices=['normal', 'hide_goal', 'hide_required', 'hide_both'],
                         help='''\
@@ -104,21 +147,23 @@ def get_argparser() -> argparse.ArgumentParser:
                              Alternatively, can be a ALttP Rom patched with a Link
                              sprite that will be extracted.
                              ''')
+    parser.add_argument('--sprite_pool', nargs='+', default=[], help='''
+                             A list of sprites to pull from.
+                        ''')
     parser.add_argument('--oof', help='''\
                              Path to a sound effect to replace Link's "oof" sound.
                              Needs to be in a .brr format and have a length of no
                              more than 2673 bytes, created from a 16-bit signed PCM
                              .wav at 12khz. https://github.com/boldowa/snesbrr
                              ''')
-    parser.add_argument('--names', default='', type=str)
     parser.add_argument('--update_sprites', action='store_true', help='Update Sprite Database, then exit.')
     return parser
 
 
 def main():
     parser = get_argparser()
-    args = parser.parse_args()
+    args = parser.parse_args(namespace=get_adjuster_settings_no_defaults(GAME_ALTTP))
-    args.music = not args.disablemusic
+    
     # set up logger
     loglevel = {'error': logging.ERROR, 'info': logging.INFO, 'warning': logging.WARNING, 'debug': logging.DEBUG}[
         args.loglevel]
@@ -530,9 +575,6 @@ class AttachTooltip(object):
 
 def get_rom_frame(parent=None):
     adjuster_settings = get_adjuster_settings(GAME_ALTTP)
-    if not adjuster_settings:
-        adjuster_settings = Namespace()
-        adjuster_settings.baserom = "Zelda no Densetsu - Kamigami no Triforce (Japan).sfc"
 
     romFrame = Frame(parent)
     baseRomLabel = Label(romFrame, text='LttP Base Rom: ')
@@ -560,33 +602,8 @@ def get_rom_frame(parent=None):
 
     return romFrame, romVar
 
-
 def get_rom_options_frame(parent=None):
     adjuster_settings = get_adjuster_settings(GAME_ALTTP)
-    defaults = {
-        "auto_apply": 'ask',
-        "music": True,
-        "reduceflashing": True,
-        "deathlink": False,
-        "sprite": None,
-        "oof": None,
-        "quickswap": True,
-        "menuspeed": 'normal',
-        "heartcolor": 'red',
-        "heartbeep": 'normal',
-        "ow_palettes": 'default',
-        "uw_palettes": 'default',
-        "hud_palettes": 'default',
-        "sword_palettes": 'default',
-        "shield_palettes": 'default',
-        "sprite_pool": [],
-        "allowcollect": False,
-    }
-    if not adjuster_settings:
-        adjuster_settings = Namespace()
-    for key, defaultvalue in defaults.items():
-        if not hasattr(adjuster_settings, key):
-            setattr(adjuster_settings, key, defaultvalue)
 
     romOptionsFrame = LabelFrame(parent, text="Rom options")
     romOptionsFrame.columnconfigure(0, weight=1)

Utils.py

@@ -14,6 +14,7 @@ import collections
 import importlib
 import logging
 
+from argparse import Namespace
 from settings import Settings, get_settings
 from typing import BinaryIO, Coroutine, Optional, Set, Dict, Any, Union
 from yaml import load, load_all, dump, SafeLoader
@@ -318,12 +319,27 @@ def store_data_package_for_checksum(game: str, data: typing.Dict[str, Any]) -> N
         except Exception as e:
             logging.debug(f"Could not store data package: {e}")
 
+def get_default_adjuster_settings(game_name: str) -> Namespace:
+    import LttPAdjuster
+    adjuster_settings = Namespace()
+    if game_name == LttPAdjuster.GAME_ALTTP:
+        return LttPAdjuster.get_argparser().parse_known_args(args=[])[0]
 
-def get_adjuster_settings(game_name: str) -> typing.Dict[str, typing.Any]:
-    adjuster_settings = persistent_load().get("adjuster", {}).get(game_name, {})
     return adjuster_settings
 
 
+def get_adjuster_settings_no_defaults(game_name: str) -> Namespace:
+    return persistent_load().get("adjuster", {}).get(game_name, Namespace())
+
+
+def get_adjuster_settings(game_name: str) -> Namespace:
+    adjuster_settings = get_adjuster_settings_no_defaults(game_name)
+    default_settings = get_default_adjuster_settings(game_name)
+
+    # Fill in any arguments from the argparser that we haven't seen before
+    return Namespace(**vars(adjuster_settings), **{k:v for k,v in vars(default_settings).items() if k not in vars(adjuster_settings)})
+
+
 @cache_argsless
 def get_unique_identifier():
     uuid = persistent_load().get("client", {}).get("uuid", None)

worlds/alttp/Client.py

@@ -581,31 +581,25 @@ class ALTTPSNIClient(SNIClient):
 def get_alttp_settings(romfile: str):
     import LttPAdjuster
 
-    last_settings = Utils.get_adjuster_settings(GAME_ALTTP)
-    base_settings = LttPAdjuster.get_argparser().parse_known_args(args=[])[0]
-    allow_list = {"music", "menuspeed", "heartbeep", "heartcolor", "ow_palettes", "quickswap",
-                  "uw_palettes", "sprite", "sword_palettes", "shield_palettes", "hud_palettes",
-                  "reduceflashing", "deathlink", "allowcollect", "oof"}
-
-    for option_name in allow_list:
-        # set new defaults since last_settings were created
-        if not hasattr(last_settings, option_name):
-            setattr(last_settings, option_name, getattr(base_settings, option_name))
-
     adjustedromfile = ''
-    if last_settings:
+    if vars(Utils.get_adjuster_settings_no_defaults(GAME_ALTTP)):
+        last_settings = Utils.get_adjuster_settings(GAME_ALTTP)
+
+        allow_list = {"music", "menuspeed", "heartbeep", "heartcolor", "ow_palettes", "quickswap",
+                    "uw_palettes", "sprite", "sword_palettes", "shield_palettes", "hud_palettes",
+                    "reduceflashing", "deathlink", "allowcollect", "oof"}
         choice = 'no'
-        if not hasattr(last_settings, 'auto_apply') or 'ask' in last_settings.auto_apply:
+        if 'ask' in last_settings.auto_apply:
             printed_options = {name: value for name, value in vars(last_settings).items() if name in allow_list}
-            if hasattr(last_settings, "sprite_pool"):
+            
-                sprite_pool = {}
+            sprite_pool = {}
-                for sprite in last_settings.sprite_pool:
+            for sprite in last_settings.sprite_pool:
-                    if sprite in sprite_pool:
+                if sprite in sprite_pool:
-                        sprite_pool[sprite] += 1
+                    sprite_pool[sprite] += 1
-                    else:
+                else:
-                        sprite_pool[sprite] = 1
+                    sprite_pool[sprite] = 1
-                    if sprite_pool:
+                if sprite_pool:
-                        printed_options["sprite_pool"] = sprite_pool
+                    printed_options["sprite_pool"] = sprite_pool
             import pprint
 
             from CommonClient import gui_enabled
@@ -685,17 +679,17 @@ def get_alttp_settings(romfile: str):
             choice = 'yes'
 
         if 'yes' in choice:
+            import LttPAdjuster
             from worlds.alttp.Rom import get_base_rom_path
             last_settings.rom = romfile
             last_settings.baserom = get_base_rom_path()
             last_settings.world = None
 
-            if hasattr(last_settings, "sprite_pool"):
+            if last_settings.sprite_pool:
                 from LttPAdjuster import AdjusterWorld
                 last_settings.world = AdjusterWorld(getattr(last_settings, "sprite_pool"))
 
             adjusted = True
-            import LttPAdjuster
             _, adjustedromfile = LttPAdjuster.adjust(last_settings)
 
             if hasattr(last_settings, "world"):