Disable the fast path of [modify_unit] (#6222)

With these two lists emptied, is_simple() will always return false, and the
code will always use the slow path. The slow path is slow because it stores and
unstores the unit, triggering the desired side effects of unstoring a unit.

Fixes issue #5133, and tests that with the new unit test.

Fixes bug #4978, that changing the facing wasn't updating the display.

data/lua/wml/modify_unit.lua

@@ -15,52 +15,13 @@ local function make_set(t)
 end
 
 local known_attributes = make_set {
-	"mode",
-	"x",
-	"y",
-	"ai_special",
-	"goto_x",
-	"goto_y",
-	"extra_recruit",
-	"side",
-	"name",
-	"role",
-	"facing",
-	"attacks_left",
-	"hitpoints",
-	"max_hitpoints",
-	"moves",
-	"max_moves",
-	"experience",
-	"max_experience",
-	"resting",
-	"canrecruit",
-	"type",
-	"variation",
-	"ellipse",
-	"halo",
-	"recall_cost",
-	"description",
-	"hidden",
-	"unrenamable",
-	"profile",
-	"zoc",
-	"usage",
-	"upkeep",
+	-- This is the list of attributes that can be handled by the fast path,
+	-- for the 1.16 release it's empty to avoid issues #4978 and #5133.
 }
 
 local known_tags = make_set {
-	"object",
-	"advancement",
-	"trait",
-	"effect",
-	"filter",
-	"status",
-	"set_variable",
-	-- todo: "set_variables",
-	"clear_variable",
-	"filter_recall",
-	"variables",
+	-- This is the list of tags that can be handled by the fast path,
+	-- for the 1.16 release it's empty to avoid issues #4978 and #5133.
 }
 
 local function is_simple(cfg)

data/test/scenarios/test_modify_unit.cfg

@@ -0,0 +1,60 @@
+# wmllint: no translatables
+
+# In 1.14 and before, the implementation of [modify_unit] always stored and
+# unstored the unit. 1.16 introduced a "fast-path" which optimised setting some
+# attributes, but which changed the behavior of the API in some edge cases.
+#
+# The store and unstore also ensured that the animation code noticed the
+# change, and the fast path meant that changes for facing or low hitpoints
+# failed to change the animation (issue 4978). However, that's a UI issue which
+# doesn't affect the API, can't be automatically tested via the WML test
+# framework, and which could be fixed during a stable branch.
+
+# In this test, changing the side of a unit on a side's recall list needs to
+# have the side-effect of moving it to the correct side's recall list.
+{GENERIC_UNIT_TEST "modify_unit_which_recall_list" (
+    [event]
+        name = start
+
+        # Create a unit on the recall list of side 1
+        [unit]
+            x=recall
+            y=recall
+            type=Orcish Grunt
+            side=1
+            id=Charlie
+            role=TestSubject
+            canrecruit=no
+        [/unit]
+
+        [modify_unit]
+            side=2
+            [filter]
+                id=Charlie
+            [/filter]
+        [/modify_unit]
+
+        [role]
+            role=TestSubject
+            reassign=no
+            [auto_recall][/auto_recall]
+        [/role]
+
+        # Check that Charlie has been recalled next to side 2's leader
+        #
+        # Currently [role][auto-recall] uses the recall list (not the side attribute)
+        # to determine where on the map to recall the unit; if this changes then this
+        # test could pass with a false positive.
+        {ASSERT (
+            [have_unit]
+                id=Charlie
+                [filter_location]
+                    location_id=2
+                    radius=1
+                [/filter_location]
+            [/have_unit]
+        )}
+
+        {SUCCEED}
+    [/event]
+)}

wml_test_schedule

@@ -108,6 +108,7 @@
 0 modify_turns_four
 0 replace_schedule_prestart
 0 modify_unit_facing
+0 modify_unit_which_recall_list
 0 event_handlers_in_events_1
 0 event_handlers_in_events_3
 0 event_handlers_in_events_2