Core: add list/dict merging feature to triggers (ArchipelagoMW#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 <[email protected]> * 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 <[email protected]> --------- Co-authored-by: Fabian Dill <[email protected]> Co-authored-by: black-sliver <[email protected]>
EmilyV99 · Apr 15, 2024 · de3fbdd · de3fbdd
1 parent 28ef730
commit de3fbdd
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 5 deletions.
diff --git a/Generate.py b/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)
-    weights.update(new_weights)
+    cleaned_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]

diff --git a/Utils.py b/Utils.py
@@ -225,6 +225,9 @@ def construct_mapping(self, node, deep=False):
             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)
 

diff --git a/test/general/test_player_options.py b/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"])
diff --git a/worlds/generic/docs/triggers_en.md b/worlds/generic/docs/triggers_en.md
@@ -121,4 +121,31 @@ For example:
 In this example (thanks to @Black-Sliver), if the `pupdunk` option is rolled, then the difficulty values will be rolled
 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".
+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.