From eacf26a2a8e24313e5c42befd42b71409486b020 Mon Sep 17 00:00:00 2001 From: Steve Cotton Date: Sun, 9 Jan 2022 13:44:43 +0100 Subject: [PATCH] Fix [store_unit]'s stored coordinates for recalls Units on the recall list might have x,y coordinates that are on the map, which therefore need to be replaced with "recall,recall" within [store_unit]. The existing code created a temporary variable, changed the coordinates, and then returned the unchanged original instead of the temporary. Add a new test that `[put_to_recall_list]` followed by `[modify_unit]` doesn't move the unit back to the map. (cherry picked from commit 096d8aba1474e9c6583d240e7eedbcd66957f327) --- data/lua/wml-tags.lua | 2 +- .../scenarios/put_to_recall_and_modify.cfg | 90 +++++++++++++++++++ data/test/scenarios/test_modify_unit.cfg | 31 ++++++- wml_test_schedule | 1 + 4 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 data/test/scenarios/put_to_recall_and_modify.cfg diff --git a/data/lua/wml-tags.lua b/data/lua/wml-tags.lua index 14c2efb3dfb..2f0a8d314a7 100644 --- a/data/lua/wml-tags.lua +++ b/data/lua/wml-tags.lua @@ -402,7 +402,7 @@ function wml_actions.store_unit(cfg) ucfg.x = 'recall' ucfg.y = 'recall' end - utils.vwriter.write(writer, u.__cfg) + utils.vwriter.write(writer, ucfg) if kill_units then u:erase() end end end diff --git a/data/test/scenarios/put_to_recall_and_modify.cfg b/data/test/scenarios/put_to_recall_and_modify.cfg new file mode 100644 index 00000000000..007f121e878 --- /dev/null +++ b/data/test/scenarios/put_to_recall_and_modify.cfg @@ -0,0 +1,90 @@ +#textdomain wesnoth-test + +##### +# API(s) being tested: [put_to_recall_list], [modify_unit], [role][auto_recall] +## +# Actions: +# Create Charlie, belonging to side 1, on the map. +# Repose Charlie on the recall list. +# Assign Charlie to side 2. +# Recall Charlie next to his side's leader. +## +# Expected end state: +# Charlie is on the map next to side 2's leader. +##### + +# This is almost the same as the modify_unit_which_recall_list test, except that +# creating Charlie on the map can leave an on-map x,y pair in Charlie's data, +# which needs to be handled within the WML API. +{GENERIC_UNIT_TEST "put_to_recall_and_modify" ( + [event] + name = start + + # Create a unit on the map, belonging to side 1. + [unit] + x,y=2,2 + type=Orcish Grunt + side=1 + id=Charlie + name=_"Charlie" + role=TestSubject + canrecruit=no + [/unit] + + [put_to_recall_list] + id=Charlie + [/put_to_recall_list] + + # Check Charlie has moved to the recall list. Using count=0 is equivalent to putting a + # [not] around the [have_unit] - using two different but equivalent syntaxes makes it + # easier to work out which of the two asserts failed. + {ASSERT ( + [have_unit] + count=0 + id=Charlie + search_recall_list=no + [/have_unit] + )} + + [modify_unit] + side=2 + [filter] + id=Charlie + [/filter] + [/modify_unit] + + # Check that Charlie is still on the recall list, that modifying him + # hasn't moved him back to the map. + {ASSERT ( + [not] + [have_unit] + id=Charlie + search_recall_list=no + [/have_unit] + [/not] + )} + + [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] +)} diff --git a/data/test/scenarios/test_modify_unit.cfg b/data/test/scenarios/test_modify_unit.cfg index 00d9e0e443e..ced92a6bac6 100644 --- a/data/test/scenarios/test_modify_unit.cfg +++ b/data/test/scenarios/test_modify_unit.cfg @@ -1,8 +1,21 @@ -# wmllint: no translatables +#textdomain wesnoth-test + +##### +# API(s) being tested: [modify_unit] on the recall list, [role][auto_recall] +## +# Actions: +# Create Charlie, belonging to side 1, on the recall list. +# Assign Charlie to side 2. +# Recall Charlie next to his side's leader. +## +# Expected end state: +# Charlie is on the map next to side 2's leader. +##### # 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. +# unstored the unit. 1.15.0 introduced a "fast-path" which optimised setting +# some attributes, but which changed the behavior of the API in some edge +# cases; 1.16.0 disabled that fast-path. # # 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 @@ -23,6 +36,7 @@ type=Orcish Grunt side=1 id=Charlie + name=_"Charlie" role=TestSubject canrecruit=no [/unit] @@ -34,6 +48,17 @@ [/filter] [/modify_unit] + # Check that Charlie is still on the recall list, that modifying him + # hasn't moved him back to the map. + {ASSERT ( + [not] + [have_unit] + id=Charlie + search_recall_list=no + [/have_unit] + [/not] + )} + [role] role=TestSubject reassign=no diff --git a/wml_test_schedule b/wml_test_schedule index f1080ceac13..03cc7241133 100644 --- a/wml_test_schedule +++ b/wml_test_schedule @@ -109,6 +109,7 @@ 0 replace_schedule_prestart 0 modify_unit_facing 0 modify_unit_which_recall_list +0 put_to_recall_and_modify 0 event_handlers_in_events_1 0 event_handlers_in_events_3 0 event_handlers_in_events_2