Archipelago
Archipelago copied to clipboard
ArchipelagoMW
Core: log warning for unknown options
Please format your title with what portion of the project this pull request is targeting and what it's changing.
ex. "MyGame4: implement new game" or "Docs: add new guide for customizing MyGame3"
What is this fixing or adding?
title
How was this tested?
rolled seeds with valid and invalid option names
If this makes graphical changes, please attach screenshots.
So this will break things like mercenary mode yamls for alttp as they use and set "invalid" keys in the yaml.
Ran this against the weights file for the big async and turned out I didn't notice that roll_triggers was called 2 separate times and would give different results based on when it was called and would find invalid keys for other games even if that game wasn't being played. Fixed this by only doing the validation step after all triggers are finished resolving and having a global set of valid_keys to check against for both trigger resolutions now. Found a bunch of typos in the big async weights file with it and it succeeded with some custom names in it.
I am not sure if i like this, like for timespinner i can basicly use the same yaml for (that includes beta options) that works on both normal and beta. those new beta options simply wont have any effect but wont break anything either
The point is to fail for options that were removed. While i don't think people should be removing options they're usually modified in some way so this solves that. When i was testing this with the async weights file there were about 3 options being rolled that no longer exist that wouldn't have been found otherwise. Supporting beta content is out of scope of this especially when it's just a minor convenience.
but does it really matter? if the option is removed well it wont do a thing right? I don't think this warrants to error out I am seriously concerned that this will be more of a annoyance every time it fails because of this over the value it adds
My proposal was to introduce hidden options, and therefore give maintainers the ability to make a hidden option that errors out when set, if they want to loudly deprecate an option. I too worry this will lead to more annoyance than it's worth, personally.
I still don't agree but it now just logs a warning for unknown options instead of crashing.