Core: add list/dict merging feature to triggers (#2793)

* proof of concept

* add dict support, block top/game level merge

* prevent key error when option being merged is new

* update triggers guide

* Add documentation about add/remove/replace

* move to trailing name instead of proper tag

* update docs

* confirm types

* Update Utils.py

* Update Generate.py

* pep8

* move to + syntax

* forgot to support sets

* specify received type of type error

* Update Generate.py

Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>

* Apply suggestion from review

* add test for update weights

* move test to new test case

* Apply suggestions from code review

Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>

---------

Co-authored-by: Fabian Dill <Berserker66@users.noreply.github.com>
Co-authored-by: black-sliver <59490463+black-sliver@users.noreply.github.com>

Generate.py

@@ -323,13 +323,29 @@ def roll_percentage(percentage: Union[int, float]) -> bool:
     return random.random() < (float(percentage) / 100)
 
 
-def update_weights(weights: dict, new_weights: dict, type: str, name: str) -> dict:
+def update_weights(weights: dict, new_weights: dict, update_type: str, name: str) -> dict:
     logging.debug(f'Applying {new_weights}')
-    new_options = set(new_weights) - set(weights)
+    cleaned_weights = {}
-    weights.update(new_weights)
+    for option in new_weights:
+        option_name = option.lstrip("+")
+        if option.startswith("+") and option_name in weights:
+            cleaned_value = weights[option_name]
+            new_value = new_weights[option]
+            if isinstance(new_value, (set, dict)):
+                cleaned_value.update(new_value)
+            elif isinstance(new_value, list):
+                cleaned_value.extend(new_value)
+            else:
+                raise Exception(f"Cannot apply merge to non-dict, set, or list type {option_name},"
+                                f" received {type(new_value).__name__}.")
+            cleaned_weights[option_name] = cleaned_value
+        else:
+            cleaned_weights[option_name] = new_weights[option]
+    new_options = set(cleaned_weights) - set(weights)
+    weights.update(cleaned_weights)
     if new_options:
         for new_option in new_options:
-            logging.warning(f'{type} Suboption "{new_option}" of "{name}" did not '
+            logging.warning(f'{update_type} Suboption "{new_option}" of "{name}" did not '
                             f'overwrite a root option. '
                             f'This is probably in error.')
     return weights
@@ -452,6 +468,10 @@ def roll_settings(weights: dict, plando_options: PlandoOptions = PlandoOptions.b
     world_type = AutoWorldRegister.world_types[ret.game]
     game_weights = weights[ret.game]
 
+    if any(weight.startswith("+") for weight in game_weights) or \
+       any(weight.startswith("+") for weight in weights):
+        raise Exception(f"Merge tag cannot be used outside of trigger contexts.")
+
     if "triggers" in game_weights:
         weights = roll_triggers(weights, game_weights["triggers"])
         game_weights = weights[ret.game]

Utils.py

@@ -225,6 +225,9 @@ class UniqueKeyLoader(SafeLoader):
             if key in mapping:
                 logging.error(f"YAML duplicates sanity check failed{key_node.start_mark}")
                 raise KeyError(f"Duplicate key {key} found in YAML. Already found keys: {mapping}.")
+            if (str(key).startswith("+") and (str(key)[1:] in mapping)) or (f"+{key}" in mapping):
+                logging.error(f"YAML merge duplicates sanity check failed{key_node.start_mark}")
+                raise KeyError(f"Equivalent key {key} found in YAML. Already found keys: {mapping}.")
             mapping.add(key)
         return super().construct_mapping(node, deep)
 

test/general/test_player_options.py

@@ -0,0 +1,39 @@
+import unittest
+import Generate
+
+
+class TestPlayerOptions(unittest.TestCase):
+
+    def test_update_weights(self):
+        original_weights = {
+            "scalar_1": 50,
+            "scalar_2": 25,
+            "list_1": ["string"],
+            "dict_1": {"option_a": 50, "option_b": 50},
+            "dict_2": {"option_f": 50},
+            "set_1": {"option_c"}
+        }
+
+        # test that we don't allow +merge syntax on scalar variables
+        with self.assertRaises(BaseException):
+            Generate.update_weights(original_weights, {"+scalar_1": 0}, "Tested", "")
+
+        new_weights = Generate.update_weights(original_weights, {"scalar_2": 0,
+                                                                 "+list_1": ["string_2"],
+                                                                 "+dict_1": {"option_b": 0, "option_c": 50},
+                                                                 "+set_1": {"option_c", "option_d"},
+                                                                 "dict_2": {"option_g": 50},
+                                                                 "+list_2": ["string_3"]},
+                                              "Tested", "")
+
+        self.assertEqual(new_weights["scalar_1"], 50)
+        self.assertEqual(new_weights["scalar_2"], 0)
+        self.assertEqual(new_weights["list_2"], ["string_3"])
+        self.assertEqual(new_weights["list_1"], ["string", "string_2"])
+        self.assertEqual(new_weights["dict_1"]["option_a"], 50)
+        self.assertEqual(new_weights["dict_1"]["option_b"], 0)
+        self.assertEqual(new_weights["dict_1"]["option_c"], 50)
+        self.assertNotIn("option_f", new_weights["dict_2"])
+        self.assertEqual(new_weights["dict_2"]["option_g"], 50)
+        self.assertEqual(len(new_weights["set_1"]), 2)
+        self.assertIn("option_d", new_weights["set_1"])

worlds/generic/docs/triggers_en.md

@@ -122,3 +122,30 @@ In this example (thanks to @Black-Sliver), if the `pupdunk` option is rolled, th
 again using the new options `normal`, `pupdunk_hard`, and `pupdunk_mystery`, and the exp modifier will be rerolled using
 new weights for 150 and 200. This allows for two more triggers that will only be used for the new `pupdunk_hard`
 and `pupdunk_mystery` options so that they will only be triggered on "pupdunk AND hard/mystery".
+
+Options that define a list, set, or dict can additionally have the character `+` added to the start of their name, which applies the contents of
+the activated trigger to the already present equivalents in the game options.
+
+For example:
+```yaml
+Super Metroid:
+  start_location: 
+    landing_site: 50
+    aqueduct: 50
+  start_hints:
+    - Morph Ball
+triggers:
+  - option_category: Super Metroid
+    option_name: start_location
+    option_result: aqueduct
+    options:
+      Super Metroid:
+        +start_hints:
+          - Gravity Suit
+```
+
+In this example, if the `start_location` option rolls `landing_site`, only a starting hint for Morph Ball will be created.
+If `aqueduct` is rolled, a starting hint for Gravity Suit will also be created alongside the hint for Morph Ball.
+
+Note that for lists, items can only be added, not removed or replaced. For dicts, defining a value for a present key will 
+replace that value within the dict.