Settings: Handle empty Groups

[qwint]

What is this fixing or adding?

alternative to #4452
issue was an apworld that defined

class mySettings(settings.Group):
    pass

and added it as their world.settings type hint to register it, causing late crashes

while the previous solution tried to crash the world early enough to not affect other worlds' settings, this pr instead aims to accept an empty Group as valid and to handle an empty dict type cache directly
export empty groups as an empty dict instead of crashing

How was this tested?

made emerald null out their settings class
on an empty file exported as

pokemon_emerald_settings:
  {}

get_settings().pokemon_emerald_settings returned the class which did not have attributes as expected
reverting emerald changes and re-saving the file populated the section as expected

pokemon_emerald_settings:
  # File name of your English Pokemon Emerald ROM
  rom_file: "Pokemon - Emerald Version (USA, Europe).gba"

If this makes graphical changes, please attach screenshots.

[black-sliver]

Not sure yet if we want this solution (or both), but It's less messy than the other solution, so maybe.

[black-sliver]

Thanks!

While I still believe it is probably a mistake to have an empty group, I can think of one legit reason, so in combination with the cleaner code, this probably the best solution until someone runs into other problems 😅

[black-sliver]

Something is failing for me on that branch. Blocking merge for now, will update once I figured out what.

[black-sliver]

Okay, I figured out what the problem is:
when upgrading the host.yaml, e.g. with python Launcher.py --update_settings and you already have a key in the Group and the new Group does not have members, then it'll write both the {} as well as the old key, i.e.

pokemon_emerald_settings:
  {}
  rom_file: "path/to/file"

--
This is expected behaviour:
When loading arbitrary values from the yaml, it'll set them as attributes and you can access them like any yaml dict. When saving that again (after the upgrade), it'll ignore the type hint and just use the object's type, but will still write the attribute.

[qwint]

not sure i understand the codepath in question, but would ideal functionality "notice" that keys are going to be added and skip the empty dict write?

[black-sliver]

This adds {} if the class is empty, but loading a yaml can add values to the instance that will then get dumped in line 289.

[qwint]

tested with an empty host.yaml with and without emerald's settings object mangled
then attempted to use --update_settings to switch between mangled and unmangled and saw things act as expected and only add the empty dict when there is no other values to print

[qwint]

some quick testing those updates seem to work well

[black-sliver]

Reran the broken case now and works. Thanks!