LttP, Core: Fixes patching LttP on a fresh AP install

[zig-for]

What is this fixing or adding?

Archipelago (0.4.2) logging initialized on Windows-10-10.0.19045-SP0 running Python 3.8.10
Patch file was supplied. Creating sfc rom..
Wrote rom file to .\output\AP_07758176404715800194_P1_YourName1.sfc
Uncaught exception
Traceback (most recent call last):
  File ".\SNIClient.py", line 726, in <module>
    asyncio.run(main())
  File "D:\Python38\lib\asyncio\runners.py", line 44, in run
    return loop.run_until_complete(main)
  File "D:\Python38\lib\asyncio\base_events.py", line 616, in run_until_complete
    return future.result()
  File ".\SNIClient.py", line 698, in main
    adjustedromfile, adjusted = get_alttp_settings(romfile)
  File "D:\git\Archipelago\worlds\alttp\Client.py", line 593, in get_alttp_settings
    setattr(last_settings, option_name, getattr(base_settings, option_name))
AttributeError: 'dict' object has no attribute 'heartcolor'

Cleans up usage of lttp adjuster settings and hardcoded values that could be different than the manual calls to the adjuster. Also implements --disablemusic properly

How was this tested?

Genned. After the first patch, settings appeared to be kosher. Mucked around with the --(disable)music option a bit

If this makes graphical changes, please attach screenshots.

usage: LttPAdjuster.py [-h] [--baserom BASEROM] [--loglevel [{error,info,warning,debug}]] [--menuspeed [{normal,instant,double,triple,quadruple,half}]] [--quickswap] [--deathlink] [--allowcollect]
                       [--music | --disablemusic] [--triforcehud [{normal,hide_goal,hide_required,hide_both}]] [--enableflashing] [--heartbeep [{double,normal,half,quarter,off}]]
                       [--heartcolor [{red,blue,green,yellow,random}]] [--ow_palettes {default,random,blackout,puke,classic,grayscale,negative,dizzy,sick}]
                       [--shield_palettes {default,random,blackout,puke,classic,grayscale,negative,dizzy,sick}] [--sword_palettes {default,random,blackout,puke,classic,grayscale,negative,dizzy,sick}]
                       [--hud_palettes {default,random,blackout,puke,classic,grayscale,negative,dizzy,sick}] [--uw_palettes {default,random,blackout,puke,classic,grayscale,negative,dizzy,sick}] [--sprite SPRITE]
                       [--oof OOF] [--names NAMES] [--update_sprites]
                       [rom]

positional arguments:
  rom                   Path to an ALttP rom to adjust.
~snip~
  --music, --disablemusic
                        Enables/Disables game music. (default: True)
~snip~

[el-u]

I was able to patch an ALttP ROM using this branch.
One odd thing was that it brought up a UI claiming "Last used adjuster settings were found", even though "_persistent_storage.yaml" was not present.

[Berserker66]

Yeah, when getting the adjuster settings it now populates it with the default, so it checking for an empty dict will now always trigger, as the dict is populated and bring up the UI.

[zig-for]

I can improve the behavior. Could I get a description for what is supposed to happen? If you’ve never used the adjuster, don’t save settings and don’t show the UI? Or is the new behavior intended/desired?Sent from my iPhoneOn Aug 22, 2023, at 11:45 AM, Fabian Dill ***@***.***> wrote: Yeah, when getting the adjuster settings it now populates it with the default, so it checking for an empty dict will now always trigger, as the dict is populated and bring up the UI. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[Berserker66]

Restoring the old behaviour would be appropriate - if no custom adjuster settings exist, skip the UI

[zig-for]

Ok, I think I fixed things up now:

• Running LttpAdjuster.py will use the persisted adjustersettings as defaults (this might be new behavior, but is probably wanted)
• Added sprite_pool and auto_apply to the arguments, so that they get correctly populated with defaults
• Running the client on a fresh install will ignore the adjuster as no settings are stored
• Running the client with adjuster settings will overlay those settings on top of the defaults

Edit: this allows removing some getattr :)

[zig-for]

Possible further improvements:

• Warning/removal of arguments that don't exist

[zig-for]

Someone who's a better writer than me should come up with nicer help text. :)

[zig-for]

This list should really just go away - hardly anyone uses text mode, I would guess - and I'd much rather just hide options we don't want to show, rather than hard code a list of special args.

[zig-for]

Giving some side-eye to all this if x in y

[zig-for]

I think there was maybe a bug here if you had an empty sprite_pool before.

[zig-for]

Ditto for removing this special case. Just show the pool as it exists in the yaml

[zig-for]

Hooray, no more duplicated default args.

[el-u]

I was able to patch an ALttP ROM using this branch.
The adjuster settings UI no longer popped up when "_persistent_storage.yaml" was not present, which I think is the correct behavior.
(It did appear when "_persistent_storage.yaml" was present but auto_apply was not set, which also seems like correct behavior.)