Options: support "random" and variations for OptionSet with defined valid_keys (#4418)

* seemingly works? needs testing

* attempt docs update

* move to verify resolution (keep?)

* account for no valid keys and "random" being passed

* Update advanced_settings_en.md

* Update Options.py

Co-authored-by: qwint <qwint.42@gmail.com>

* Update Options.py

Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>

* unify random handling between range and set

* Update Options.py

* Update Options.py

* Update Options.py

Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>

* super is weird

* fix item/location

* remove groups from options

* unittest

* pep8

* Update Options.py

Co-authored-by: Exempt-Medic <60412657+Exempt-Medic@users.noreply.github.com>

* Update Options.py

---------

Co-authored-by: qwint <qwint.42@gmail.com>
Co-authored-by: Doug Hoskisson <beauxq@users.noreply.github.com>
Co-authored-by: Exempt-Medic <60412657+Exempt-Medic@users.noreply.github.com>

Options.py

@@ -24,6 +24,39 @@ if typing.TYPE_CHECKING:
     import pathlib
 
 
+_RANDOM_OPTS = [
+        "random", "random-low", "random-middle", "random-high", 
+        "random-range-low-<min>-<max>", "random-range-middle-<min>-<max>",
+        "random-range-high-<min>-<max>", "random-range-<min>-<max>",
+]
+    
+    
+def triangular(lower: int, end: int, tri: float = 0.5) -> int:
+    """
+    Integer triangular distribution for `lower` inclusive to `end` inclusive.
+
+    Expects `lower <= end` and `0.0 <= tri <= 1.0`. The result of other inputs is undefined.
+    """
+    # Use the continuous range [lower, end + 1) to produce an integer result in [lower, end].
+    # random.triangular is actually [a, b] and not [a, b), so there is a very small chance of getting exactly b even
+    # when a != b, so ensure the result is never more than `end`.
+    return min(end, math.floor(random.triangular(0.0, 1.0, tri) * (end - lower + 1) + lower))
+  
+
+def random_weighted_range(text: str, range_start: int, range_end: int):
+    if text == "random-low":
+        return triangular(range_start, range_end, 0.0)
+    elif text == "random-high":
+        return triangular(range_start, range_end, 1.0)
+    elif text == "random-middle":
+        return triangular(range_start, range_end)
+    elif text == "random":
+        return random.randint(range_start, range_end)
+    else:
+            raise Exception(f"random text \"{text}\" did not resolve to a recognized pattern. "
+                            f"Acceptable values are: {', '.join(_RANDOM_OPTS)}.")
+
+        
 def roll_percentage(percentage: int | float) -> bool:
     """Roll a percentage chance.
     percentage is expected to be in range [0, 100]"""
@@ -690,12 +723,6 @@ class Range(NumericOption):
     range_start = 0
     range_end = 1
 
-    _RANDOM_OPTS = [
-        "random", "random-low", "random-middle", "random-high", 
-        "random-range-low-<min>-<max>", "random-range-middle-<min>-<max>",
-        "random-range-high-<min>-<max>", "random-range-<min>-<max>",
-    ]
-
     def __init__(self, value: int):
         if value < self.range_start:
             raise Exception(f"{value} is lower than minimum {self.range_start} for option {self.__class__.__name__}")
@@ -744,25 +771,16 @@ class Range(NumericOption):
 
     @classmethod
     def weighted_range(cls, text) -> Range:
-        if text == "random-low":
+        if text.startswith("random-range-"):
-            return cls(cls.triangular(cls.range_start, cls.range_end, 0.0))
-        elif text == "random-high":
-            return cls(cls.triangular(cls.range_start, cls.range_end, 1.0))
-        elif text == "random-middle":
-            return cls(cls.triangular(cls.range_start, cls.range_end))
-        elif text.startswith("random-range-"):
             return cls.custom_range(text)
-        elif text == "random":
-            return cls(random.randint(cls.range_start, cls.range_end))
         else:
-            raise Exception(f"random text \"{text}\" did not resolve to a recognized pattern. "
+            return cls(random_weighted_range(text, cls.range_start, cls.range_end))
-                            f"Acceptable values are: {', '.join(cls._RANDOM_OPTS)}.")
 
     @classmethod
     def custom_range(cls, text) -> Range:
         textsplit = text.split("-")
         try:
-            random_range = [int(textsplit[len(textsplit) - 2]), int(textsplit[len(textsplit) - 1])]
+            random_range = [int(textsplit[-2]), int(textsplit[-1])]
         except ValueError:
             raise ValueError(f"Invalid random range {text} for option {cls.__name__}")
         random_range.sort()
@@ -770,14 +788,9 @@ class Range(NumericOption):
             raise Exception(
                 f"{random_range[0]}-{random_range[1]} is outside allowed range "
                 f"{cls.range_start}-{cls.range_end} for option {cls.__name__}")
-        if text.startswith("random-range-low"):
+        if textsplit[2] in ("low", "middle", "high"):
-            return cls(cls.triangular(random_range[0], random_range[1], 0.0))
+            return cls(random_weighted_range(f"{textsplit[0]}-{textsplit[2]}", *random_range))
-        elif text.startswith("random-range-middle"):
+        return cls(random_weighted_range("random", *random_range))
-            return cls(cls.triangular(random_range[0], random_range[1]))
-        elif text.startswith("random-range-high"):
-            return cls(cls.triangular(random_range[0], random_range[1], 1.0))
-        else:
-            return cls(random.randint(random_range[0], random_range[1]))
 
     @classmethod
     def from_any(cls, data: typing.Any) -> Range:
@@ -792,18 +805,6 @@ class Range(NumericOption):
     def __str__(self) -> str:
         return str(self.value)
 
-    @staticmethod
-    def triangular(lower: int, end: int, tri: float = 0.5) -> int:
-        """
-        Integer triangular distribution for `lower` inclusive to `end` inclusive.
-
-        Expects `lower <= end` and `0.0 <= tri <= 1.0`. The result of other inputs is undefined.
-        """
-        # Use the continuous range [lower, end + 1) to produce an integer result in [lower, end].
-        # random.triangular is actually [a, b] and not [a, b), so there is a very small chance of getting exactly b even
-        # when a != b, so ensure the result is never more than `end`.
-        return min(end, math.floor(random.triangular(0.0, 1.0, tri) * (end - lower + 1) + lower))
-
 
 class NamedRange(Range):
     special_range_names: typing.Dict[str, int] = {}
@@ -1000,13 +1001,19 @@ class OptionList(Option[typing.List[typing.Any]], VerifyKeys):
 class OptionSet(Option[typing.Set[str]], VerifyKeys):
     default = frozenset()
     supports_weighting = False
+    random_str: str | None
 
-    def __init__(self, value: typing.Iterable[str]):
+    def __init__(self, value: typing.Iterable[str], random_str: str | None = None):
         self.value = set(deepcopy(value))
+        self.random_str = random_str
         super(OptionSet, self).__init__()
 
     @classmethod
     def from_text(cls, text: str):
+        check_text = text.lower().split(",")
+        if ((cls.valid_keys or cls.verify_item_name or cls.verify_location_name)
+                and len(check_text) == 1 and check_text[0].startswith("random")):
+            return cls((), check_text[0])
         return cls([option.strip() for option in text.split(",")])
 
     @classmethod
@@ -1015,6 +1022,35 @@ class OptionSet(Option[typing.Set[str]], VerifyKeys):
             return cls(data)
         return cls.from_text(str(data))
 
+    def verify(self, world: typing.Type[World], player_name: str, plando_options: PlandoOptions) -> None:
+        if self.random_str and not self.value:
+            choice_list = sorted(self.valid_keys)
+            if self.verify_item_name:
+                choice_list.extend(sorted(world.item_names))
+            if self.verify_location_name:
+                choice_list.extend(sorted(world.location_names))
+            if self.random_str.startswith("random-range-"):
+                textsplit = self.random_str.split("-")
+                try:
+                    random_range = [int(textsplit[-2]), int(textsplit[-1])]
+                except ValueError:
+                    raise ValueError(f"Invalid random range {self.random_str} for option {self.__class__.__name__} "
+                                     f"for player {player_name}")
+                random_range.sort()
+                if random_range[0] < 0 or random_range[1] > len(choice_list):
+                    raise Exception(
+                        f"{random_range[0]}-{random_range[1]} is outside allowed range "
+                        f"0-{len(choice_list)} for option {self.__class__.__name__} for player {player_name}")
+                if textsplit[2] in ("low", "middle", "high"):
+                    choice_count = random_weighted_range(f"{textsplit[0]}-{textsplit[2]}",
+                                                         random_range[0], random_range[1])
+                else:
+                    choice_count = random_weighted_range("random", random_range[0], random_range[1])
+            else:
+                choice_count = random_weighted_range(self.random_str, 0, len(choice_list))
+            self.value = set(random.sample(choice_list, k=choice_count))
+        super(Option, self).verify(world, player_name, plando_options)
+
     @classmethod
     def get_option_name(cls, value):
         return ", ".join(sorted(value))

test/general/test_options.py

@@ -1,8 +1,9 @@
 import unittest
 
 from BaseClasses import PlandoOptions
-from Options import Choice, ItemLinks, PlandoConnections, PlandoItems, PlandoTexts
+from Options import Choice, ItemLinks, OptionSet, PlandoConnections, PlandoItems, PlandoTexts
 from Utils import restricted_dumps
+
 from worlds.AutoWorld import AutoWorldRegister
 
 
@@ -82,6 +83,19 @@ class TestOptions(unittest.TestCase):
                         if issubclass(option, Choice) and option.default in option.name_lookup:
                             restricted_dumps(option.from_text(option.name_lookup[option.default]))
 
+    def test_option_set_keys_random(self):
+        """Tests that option sets do not contain 'random' and its variants as valid keys"""
+        for game_name, world_type in AutoWorldRegister.world_types.items():
+            if game_name not in ("Archipelago", "Sudoku", "Super Metroid"):
+                for option_key, option in world_type.options_dataclass.type_hints.items():
+                    if issubclass(option, OptionSet):
+                        with self.subTest(game=game_name, option=option_key):
+                            self.assertFalse(any(random_key in option.valid_keys for random_key in ("random",
+                                                                                                    "random-high",
+                                                                                                    "random-low")))
+                            for key in option.valid_keys:
+                                self.assertFalse("random-range" in key)
+    
     def test_pickle_dumps_plando(self):
         """Test that plando options using containers of a custom type can be pickled"""
         # The base PlandoConnections class can't be instantiated directly, create a subclass and then cast it

worlds/generic/docs/advanced_settings_en.md

@@ -156,6 +156,9 @@ Options taking a choice of a number can also use a variety of `random` options t
 * `random-range-low-#-#`, `random-range-middle-#-#`, and `random-range-high-#-#` will choose a number at random from the
   specified numbers, but with the specified weights
 
+Options defining a unique set of options can also make use of the prior `random` options to select a subset of possible 
+options. The resulting number is used as the number of random items the set should contain.
+
 ### Example
 
 ```yaml