Core: Add settings API ("auto settings") for host.yaml #1871

black-sliver · 2023-06-17T12:29:16Z

What is this fixing or adding?

Instead of relying on Utils.default_options and a hand-written host.yaml, we generate the host.yaml now from type hints in settings.py (global) and each individual world.
This way APWorlds have proper access to the host.yaml and having typed settings allows for cleaner code with automatic type-checking, like self.settings.rom_path. When saving/loading it will lazily load the type information from worlds and downcast/upcast accordingly.

In addition to that, the new API also automatically opens a file-browser and auto-saves changes when a file is requested that does not exist.

Backwards compat is provided by implementing most of the dict API in settings.Group as well as keeping Utils.get_options as an alternative name.

How was this tested?

mypy
pytest
reading the automatically generated host.yaml
rolling and hosting a few games.
FactorioClient - only on linux

What was not tested?

Most of the worlds and adjusters were not tested. Relying on backwards compat for them.
The installer - testing still on my todo list.

What is left to do?

Updating adjusters, worlds, etc - all access to Utils.get_options() should be rewritten to use the new API, but because this PR is quite big already and I can't test everything, I decided against doing that as part of this PR.
Some cleanup - a lot of untouched code does Utils.user_path(Utils.get_options()[...]), which is not required anymore. Accessing a UserPath path will automatically resolve to be a user path.
Removing the ROM Selection from the installer.

Generate.py

Utils.py

worlds/lufia2ac/__init__.py

settings.py

worlds/AutoWorld.py

worlds/lufia2ac/__init__.py

docs/settings api.md

Co-authored-by: el-u <109771707+el-u@users.noreply.github.com>

Berserker66

LGTM, also did some testing of the LttP Adjuster and I didn't find any breakage.

t3hf1gm3nt

Successfully generated a new host.yaml and made sure nothing was broken in regards to TLOZ, so gets an OK from me.

alwaysintreble

Tested on a new venv with both 3.10 and 3.11 on windows 11 and packaged the host.yaml correctly. There's still a lot of mixing of terms. I really think it should just all be renamed to settings where possible, and the use of the word options for any of these completely dropped. {world_folder_name}_options for example should be {world_folder_name}_settings if possible.

alwaysintreble · 2023-07-04T09:56:37Z

docs/settings api.md

+which either points to the installation directory, if writable, or to %home%/Archipelago otherwise.
+
+**Examples:**
+* C:\Program Data\Archipelago\options.yaml


It was already brought up to unify the nomenclature and I agree. This feature isn't documented anywhere currently so this seems like a good idea to change. Instead of options.yaml it can be settings.yaml, and just support options.yaml until we drop the old API

I don't touch filenames in this PR because migrating from one filename to another is gonna be easier once that's in and breaking someone's options.yaml will make sad faces.

docs/settings api.md

alwaysintreble · 2023-07-04T10:07:32Z

docs/settings api.md

+## World Settings
+
+Worlds can define the top level key to use by defining `settings_key: ClassVar[str]` in their World class.
+It defaults to `{folder_name}_options` if undefined, i.e. `worlds/factorio/...` defaults to `factorio_options`.


comma after i.e.

comma after i.e. (or e.g. for that matter) is usually optional and depend on which "fork" of English you use. They all agree one should precede it though, so I'm less concerned about this one.

A quick search says that's a difference between American and British english. We had mostly BE in school, so not my fault :-D

alwaysintreble · 2023-07-04T10:09:19Z

docs/settings api.md

+
+Worlds define the layout of their config section using type annotation of the variable `settings` in the class.
+The type has to inherit from `settings.Group`. Each value in the config can have a comment by subclassing a builtin
+type. Some helper types are defined `settings.py`, see [Types](#Types) for a list.


Suggested change

type. Some helper types are defined `settings.py`, see [Types](#Types) for a list.

The type has to inherit from `settings.Group`. Each value in the config can have a comment by subclassing a built-in

type. Some helper types are defined in `settings.py`, see [Types](#Types) for a list.

I was referring to python builtins. The doc page for builtins calls it "built-in objects", so we should probably follow that doc style? Your suggestion did not select the correct lines, by the way.

docs/world api.md

ThePhar

In my opinion, can worry about renaming the root filename later because that can happen without worrying about breaking existing worlds. This does open a lot of possibilities for also auto-generating some of the innosetup aspects, so I'm very excited for that.

Confirmed working on Windows 3.10 and 3.11 (with exception of OOT, but that should be fixed by #1936, as it's unrelated.)

settings.py

docs/settings api.md

Co-authored-by: Zach Parks <zach@alliware.com>

…#1871) * Add settings API ("auto settings") for host.yaml * settings: no BOM when saving * settings: fix saving / groups resetting themselves * settings: fix AutoWorldRegister import Co-authored-by: el-u <109771707+el-u@users.noreply.github.com> * Lufia2: settings: clean up imports * settings: more consistent class naming * Docs: update world api for settings api refactor * settings: fix access from World instance * settings: update migration timeline * Docs: Apply suggestions from code review Co-authored-by: Zach Parks <zach@alliware.com> * Settings: correctly resolve .exe in UserPath and LocalPath --------- Co-authored-by: el-u <109771707+el-u@users.noreply.github.com> Co-authored-by: Zach Parks <zach@alliware.com>

ThePhar added is: enhancement Issues requesting new features or pull requests implementing new features. is: refactor/cleanup Improvements to code/output readability or organizization. affects: core Issues/PRs that touch core and may need additional validation. labels Jun 19, 2023

ThePhar changed the title ~~Add settings API ("auto settings") for host.yaml~~ Core: Add settings API ("auto settings") for host.yaml Jun 19, 2023

el-u reviewed Jun 22, 2023

View reviewed changes

black-sliver force-pushed the auto-settings branch from 17cc91c to 8d971f8 Compare July 2, 2023 13:25

black-sliver and others added 9 commits July 2, 2023 15:51

Add settings API ("auto settings") for host.yaml

88f6a8b

settings: no BOM when saving

1230564

settings: fix saving / groups resetting themselves

0227543

settings: fix AutoWorldRegister import

be62e5e

Co-authored-by: el-u <109771707+el-u@users.noreply.github.com>

Lufia2: settings: clean up imports

b61ecef

settings: more consistent class naming

d7a8344

Docs: update world api for settings api refactor

36c9dcc

settings: fix access from World instance

6a65dd6

settings: update migration timeline

2ae1d6c

black-sliver force-pushed the auto-settings branch from 8d971f8 to 2ae1d6c Compare July 2, 2023 13:52

Berserker66 approved these changes Jul 2, 2023

View reviewed changes

t3hf1gm3nt approved these changes Jul 3, 2023

View reviewed changes

alwaysintreble reviewed Jul 4, 2023

View reviewed changes

ThePhar approved these changes Jul 5, 2023

View reviewed changes

ThePhar reviewed Jul 5, 2023

View reviewed changes

settings.py Show resolved Hide resolved

ThePhar added the is: documentation Improvements or additions to documentation. label Jul 5, 2023

black-sliver commented Jul 5, 2023

View reviewed changes

docs/settings api.md Outdated Show resolved Hide resolved

black-sliver and others added 2 commits July 5, 2023 09:29

Docs: Apply suggestions from code review

5c4eda5

Co-authored-by: Zach Parks <zach@alliware.com>

Settings: correctly resolve .exe in UserPath and LocalPath

6cfbfbd

black-sliver merged commit 827444f into ArchipelagoMW:main Jul 5, 2023
14 checks passed

black-sliver deleted the auto-settings branch July 5, 2023 20:39

Silvris mentioned this pull request Sep 4, 2023

Yoshi's Island: Implement New Game #2141

Merged

el-u mentioned this pull request Oct 24, 2023

Core: deprecate Utils.get_options and remove Utils.get_default_options #2352

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core: Add settings API ("auto settings") for host.yaml #1871

Core: Add settings API ("auto settings") for host.yaml #1871

black-sliver commented Jun 17, 2023 •

edited

Berserker66 left a comment

t3hf1gm3nt left a comment

alwaysintreble left a comment

alwaysintreble Jul 4, 2023

black-sliver Jul 5, 2023

alwaysintreble Jul 4, 2023

ThePhar Jul 5, 2023

black-sliver Jul 5, 2023

alwaysintreble Jul 4, 2023

ThePhar Jul 5, 2023

black-sliver Jul 5, 2023

ThePhar left a comment

	type. Some helper types are defined `settings.py`, see [Types](#Types) for a list.
	The type has to inherit from `settings.Group`. Each value in the config can have a comment by subclassing a built-in
	type. Some helper types are defined in `settings.py`, see [Types](#Types) for a list.

Core: Add settings API ("auto settings") for host.yaml #1871

Core: Add settings API ("auto settings") for host.yaml #1871

Conversation

black-sliver commented Jun 17, 2023 • edited

What is this fixing or adding?

How was this tested?

What was not tested?

What is left to do?

Berserker66 left a comment

Choose a reason for hiding this comment

t3hf1gm3nt left a comment

Choose a reason for hiding this comment

alwaysintreble left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThePhar left a comment

Choose a reason for hiding this comment

black-sliver commented Jun 17, 2023 •

edited