From d2fbd89a47fb7a616b709f6fa8ea2d570ed89dcf Mon Sep 17 00:00:00 2001
From: Phar <zach@alliware.com>
Date: Mon, 13 Nov 2023 09:12:42 -0600
Subject: [PATCH] Additional edge cases, variable name changes, and some tweaks
 to UI.

---
 WebHostLib/options.py                    | 29 ++++++++++--
 WebHostLib/templates/player-options.html |  3 +-
 docs/world api.md                        |  8 ++--
 test/webhost/test_option_presets.py      | 59 ++++++++++++++++++++----
 worlds/rogue_legacy/Presets.py           | 17 +++----
 worlds/rogue_legacy/__init__.py          |  4 +-
 6 files changed, 93 insertions(+), 27 deletions(-)

diff --git a/WebHostLib/options.py b/WebHostLib/options.py
index fb1a637111d1..49146c9ddb34 100644
--- a/WebHostLib/options.py
+++ b/WebHostLib/options.py
@@ -119,17 +119,40 @@ def get_html_doc(option_type: type(Options.Option)) -> str:
         player_options["gameOptions"] = game_options
 
         player_options["presetOptions"] = {}
-        for preset_name, presets in world.web.options_presets.items():
+        for preset_name, preset in world.web.options_presets.items():
             player_options["presetOptions"][preset_name] = {}
-            for option_name, option_value in presets.items():
+            for option_name, option_value in preset.items():
+                # Random range type settings are not valid.
+                assert (option_value not in ["random-low", "random-high", "random-middle"] and
+                        not str(option_value).startswith("random-")), \
+                    f"Invalid preset value '{option_value}' for '{option_name}' in '{preset_name}'. Special random " \
+                    f"values are not supported for presets."
+
+                # Normal random is supported, but needs to be handled explicitly.
                 if option_value == "random":
                     player_options["presetOptions"][preset_name][option_name] = option_value
                     continue
 
                 option = world.options_dataclass.type_hints[option_name].from_any(option_value)
-                if issubclass(option.__class__, Options.Range):
+                if isinstance(option, Options.SpecialRange) and isinstance(option_value, str):
+                    assert option_value in option.special_range_names, \
+                        f"Invalid preset value '{option_value}' for '{option_name}' in '{preset_name}'. " \
+                        f"Expected {option.special_range_names.keys()} or {option.range_start}-{option.range_end}."
+
+                    # Still use the true value for the option, not the name.
+                    player_options["presetOptions"][preset_name][option_name] = option.value
+                elif isinstance(option, Options.Range):
                     player_options["presetOptions"][preset_name][option_name] = option.value
+                elif isinstance(option_value, str):
+                    # For Choice and Toggle options, the value should be the name of the option. This is to prevent
+                    # setting a preset for an option with an overridden from_text method that would normally be okay,
+                    # but would not be okay for the webhost's current implementation of player options UI.
+                    assert option.name_lookup[option.value] == option_value, \
+                        f"Invalid option value '{option_value}' for '{option_name}' in preset '{preset_name}'. " \
+                        f"Values must not be resolved to a different option via option.from_text (or an alias)."
+                    player_options["presetOptions"][preset_name][option_name] = option.current_key
                 else:
+                    # int and bool values are fine, just resolve them to the current key for webhost.
                     player_options["presetOptions"][preset_name][option_name] = option.current_key
 
         os.makedirs(os.path.join(target_folder, 'player-options'), exist_ok=True)
diff --git a/WebHostLib/templates/player-options.html b/WebHostLib/templates/player-options.html
index 7c35241ecbb4..4c749752882a 100644
--- a/WebHostLib/templates/player-options.html
+++ b/WebHostLib/templates/player-options.html
@@ -28,7 +28,6 @@ <h1><span id="game-name">Player</span> Options</h1>
             <a href="/static/generated/configs/{{ game }}.yaml">template file for this game</a>.
         </p>
 
-        <h2>Meta Options</h2>
         <div id="meta-options">
             <div>
                 <label for="player-name">
@@ -38,7 +37,7 @@ <h2>Meta Options</h2>
             </div>
             <div>
                 <label for="game-options-preset">
-                    Options Preset: <span class="interactive" data-tooltip="Select a developer-curated preset of game options!">(?)</span>
+                    Options Preset: <span class="interactive" data-tooltip="Select from a list of developer-curated presets (if any) or reset all options to their defaults.">(?)</span>
                 </label>
                 <select id="game-options-preset">
                     <option value="__default">Defaults</option>
diff --git a/docs/world api.md b/docs/world api.md
index 635c73e1cc20..c2bd37339660 100644
--- a/docs/world api.md	
+++ b/docs/world api.md	
@@ -77,15 +77,17 @@ prefixed with the same string as defined here. Default already has 'en'.
 are the options to be set for that preset. The options are defined as a `Dict[str, Any]` where the keys are the names of
 the options and the values are the values to be set for that option. These presets will be available for users to select from on the game's options page.
 
-Note: The values must be valid for the option type and can only include the following option types:
+Note: The values must be a non-aliased value for the option type and can only include the following option types:
 
   - If you have a `Range`/`SpecialRange` option, the value should be an `int` between the `range_start` and `range_end`
-    values. 
+    values.
+    - If you have a `SpecialRange` option, the value can alternatively be a `str` that is one of the 
+      `special_range_names` keys.
   - If you have a `Choice` option, the value should be a `str` that is one of the `option_<name>` values. 
   - If you have a `Toggle`/`DefaultOnToggle` option, the value should be a `bool`.
   - `random` is also a valid value for any of these option types.
 
-`OptionDict`, `OptionList`, `OptionSet`, and `FreeText` are not supported. 
+`OptionDict`, `OptionList`, `OptionSet`, and `FreeText` are not supported for presets on the webhost.
 
 Here is an example of a defined preset:
 ```python
diff --git a/test/webhost/test_option_presets.py b/test/webhost/test_option_presets.py
index 52180b515fb0..968b8dcf9949 100644
--- a/test/webhost/test_option_presets.py
+++ b/test/webhost/test_option_presets.py
@@ -1,10 +1,7 @@
 import unittest
-from typing import TYPE_CHECKING
 
 from worlds import AutoWorldRegister
-
-if TYPE_CHECKING:
-    from Options import Option
+from Options import Choice, SpecialRange, Toggle, Range
 
 
 class TestOptionPresets(unittest.TestCase):
@@ -12,12 +9,56 @@ def test_option_presets_have_valid_options(self):
         """Test that all predefined option presets are valid options."""
         for game_name, world_type in AutoWorldRegister.world_types.items():
             presets = world_type.web.options_presets
-            for preset_name, presets in presets.items():
-                with self.subTest(game=game_name, presets=preset_name):
-                    for option_name, option_value in presets.items():
+            for preset_name, preset in presets.items():
+                with self.subTest(game=game_name, preset=preset_name):
+                    for option_name, option_value in preset.items():
                         try:
-                            # We don't need to assign the return, as we're only interested if this raises an error.
-                            world_type.options_dataclass.type_hints[option_name].from_any(option_value)
+                            option = world_type.options_dataclass.type_hints[option_name].from_any(option_value)
+                            supported_types = [Choice, Toggle, Range, SpecialRange]
+                            if not any([issubclass(option.__class__, t) for t in supported_types]):
+                                self.fail(f"'{option_name}' in preset '{preset_name}' for game '{game_name}' "
+                                          f"is not a supported type for webhost. "
+                                          f"Supported types: {', '.join([t.__name__ for t in supported_types])}")
                         except AssertionError as ex:
                             self.fail(f"Option '{option_name}': '{option_value}' in preset '{preset_name}' for game "
                                       f"'{game_name}' is not valid. Error: {ex}")
+                        except KeyError as ex:
+                            self.fail(f"Option '{option_name}' in preset '{preset_name}' for game '{game_name}' is "
+                                      f"not a defined option. Error: {ex}")
+
+    def test_option_preset_values_are_explicitly_defined(self):
+        """Test that option preset values are not a special flavor of 'random' or use from_text to resolve another
+        value.
+        """
+        for game_name, world_type in AutoWorldRegister.world_types.items():
+            presets = world_type.web.options_presets
+            for preset_name, preset in presets.items():
+                with self.subTest(game=game_name, preset=preset_name):
+                    for option_name, option_value in preset.items():
+                        # Check for non-standard random values.
+                        self.assertTrue(
+                            option_value not in ["random-low", "random-high", "random-middle"] and
+                            not str(option_value).startswith("random-"),
+                            f"'{option_name}': '{option_value}' in preset '{preset_name}' for game '{game_name}' "
+                            f"is not supported for webhost. Special random values are not supported for presets."
+                        )
+
+                        option = world_type.options_dataclass.type_hints[option_name].from_any(option_value)
+
+                        # Check for from_text resolving to a different value. ("random" is allowed though.)
+                        if option_value != "random" and isinstance(option_value, str):
+                            # Allow special named values for SpecialRange option presets.
+                            if isinstance(option, SpecialRange):
+                                self.assertTrue(
+                                    option_value in option.special_range_names,
+                                    f"Invalid preset '{option_name}': '{option_value}' in preset '{preset_name}' "
+                                    f"for game '{game_name}'. Expected {option.special_range_names.keys()} or "
+                                    f"{option.range_start}-{option.range_end}."
+                                )
+                            else:
+                                self.assertTrue(
+                                    option.name_lookup[option.value] == option_value,
+                                    f"'{option_name}': '{option_value}' in preset '{preset_name}' for game "
+                                    f"'{game_name}' is not supported for webhost. Values must not be resolved to a "
+                                    f"different option via option.from_text (or an alias)."
+                                )
diff --git a/worlds/rogue_legacy/Presets.py b/worlds/rogue_legacy/Presets.py
index 0bc22fc09891..a4284e9f7d34 100644
--- a/worlds/rogue_legacy/Presets.py
+++ b/worlds/rogue_legacy/Presets.py
@@ -1,8 +1,9 @@
 from typing import Any, Dict
 
+from .Options import Architect, GoldGainMultiplier, Vendors
 
-rl_option_presets: Dict[str, Dict[str, Any]] = {
-    # Example preset.
+rl_options_presets: Dict[str, Dict[str, Any]] = {
+    # Example preset using only literal values.
     "Unknown Fate": {
         "progression_balancing":    "random",
         "accessibility":            "random",
@@ -34,18 +35,18 @@
         "equip_pool":               "random",
         "crit_chance_pool":         "random",
         "crit_damage_pool":         "random",
-        "allow_default_names":      1,
+        "allow_default_names":      False,
         "death_link":               "random",
     },
-    # A preset I actually use.
+    # A preset I actually use, using some literal values and some from the option itself.
     "Limited Potential": {
-        "progression_balancing":    0,
+        "progression_balancing":    "disabled",
         "fairy_chests_per_zone":    2,
         "starting_class":           "random",
         "chests_per_zone":          30,
-        "vendors":                  "normal",
-        "architect":                "disabled",
-        "gold_gain_multiplier":     "half",
+        "vendors":                  Vendors.option_normal,
+        "architect":                Architect.option_disabled,
+        "gold_gain_multiplier":     GoldGainMultiplier.option_half,
         "number_of_children":       2,
         "free_diary_on_generation": False,
         "health_pool":              10,
diff --git a/worlds/rogue_legacy/__init__.py b/worlds/rogue_legacy/__init__.py
index e4ad8cf4d233..c5a8d71b5d63 100644
--- a/worlds/rogue_legacy/__init__.py
+++ b/worlds/rogue_legacy/__init__.py
@@ -5,7 +5,7 @@
 from .Items import RLItem, RLItemData, event_item_table, get_items_by_category, item_table
 from .Locations import RLLocation, location_table
 from .Options import rl_options
-from .Presets import rl_option_presets
+from .Presets import rl_options_presets
 from .Regions import create_regions
 from .Rules import set_rules
 
@@ -23,7 +23,7 @@ class RLWeb(WebWorld):
     )]
     bug_report_page = "https://github.com/ThePhar/RogueLegacyRandomizer/issues/new?assignees=&labels=bug&template=" \
                       "report-an-issue---.md&title=%5BIssue%5D"
-    options_presets = rl_option_presets
+    options_presets = rl_options_presets
 
 
 class RLWorld(World):