Fix [store_unit_defense] and add [store_unit_defense_on], add unit test

The existing tag has a confusing name - it returns the chance to be hit rather
than the defense, for example 30 would be returned for a unit on 70% terrain.
The new tag returns a higher-is-better value.

data/lua/wml-tags.lua

@@ -953,16 +953,33 @@ function wesnoth.wml_actions.cancel_action(cfg)
 end
 
 function wesnoth.wml_actions.store_unit_defense(cfg)
+	wesnoth.deprecated_message("[store_unit_defense]", 3, "1.17.0", "This function returns the chance to be hit, high values represent bad defenses. Using [store_unit_defense_on] is recommended instead.")
+
 	local unit = wesnoth.units.find_on_map(cfg)[1] or wml.error "[store_unit_defense]'s filter didn't match any unit"
 	local terrain = cfg.terrain
 	local defense
 
 	if terrain then
-		defense = units:defense(terrain)
+		defense = unit:chance_to_be_hit(terrain)
+	elseif cfg.loc_x and cfg.loc_y then
+		defense = unit:chance_to_be_hit(wesnoth.get_terrain(cfg.loc_x, cfg.loc_y))
+	else
+		defense = unit:chance_to_be_hit(wesnoth.get_terrain(unit.x, unit.y))
+	end
+	wml.variables[cfg.variable or "terrain_defense"] = defense
+end
+
+function wesnoth.wml_actions.store_unit_defense_on(cfg)
+	local unit = wesnoth.units.find_on_map(cfg)[1] or wml.error "[store_unit_defense_on]'s filter didn't match any unit"
+	local terrain = cfg.terrain
+	local defense
+
+	if terrain then
+		defense = unit:defense_on(terrain)
 	elseif cfg.loc_x and cfg.loc_y then
-		defense = units:defense(wesnoth.get_terrain(cfg.loc_x, cfg.loc_y))
+		defense = unit:defense_on(wesnoth.get_terrain(cfg.loc_x, cfg.loc_y))
 	else
-		defense = units:defense(wesnoth.get_terrain(unit.x, unit.y))
+		defense = unit:defense_on(wesnoth.get_terrain(unit.x, unit.y))
 	end
 	wml.variables[cfg.variable or "terrain_defense"] = defense
 end

data/schema/core/actionwml.cfg

@@ -1097,6 +1097,15 @@
 		{SIMPLE_KEY terrain terrain_code}
 		{SIMPLE_KEY variable string}
 	[/tag]
+	[tag]
+		name="store_unit_defense_on"
+		max=infinite
+		super="$filter_unit"
+		{SIMPLE_KEY loc_x s_unsigned}
+		{SIMPLE_KEY loc_y s_unsigned}
+		{SIMPLE_KEY terrain terrain_code}
+		{SIMPLE_KEY variable string}
+	[/tag]
 	[tag]
 		name="store_unit_type"
 		max=infinite

data/test/scenarios/store_unit_defense.cfg

@@ -0,0 +1,107 @@
+# wmllint: no translatables
+
+{GENERIC_UNIT_TEST "test_store_unit_defense_on" (
+    [event]
+        name = start
+
+        # Alice and Bob are both standing on castles, add another unit on grassland
+        {NOTRAIT_UNIT 1 "Footpad" 1 1}
+
+        [store_unit_defense_on]
+            id=alice
+            # default output will be to terrain_defense
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 60}}
+
+        # Check that both terrain= and variable= are supported
+        [store_unit_defense_on]
+            type="Footpad"
+            terrain=Ke
+            variable=footpad_defense
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL footpad_defense equals 70}}
+        # Check it didn't overwrite the default variable
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 60}}
+
+        [store_unit_defense_on]
+            id=alice
+            terrain=Ke
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 60}}
+
+        [store_unit_defense_on]
+            id=alice
+            terrain=Gg
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 40}}
+
+        # Void is one of the terrains that's always defined, even if it's not used on the map
+        [store_unit_defense_on]
+            id=alice
+            terrain=Xv
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 0}}
+
+        # On this map, 1,1 is grassland
+        [store_unit_defense_on]
+            id=alice
+            loc_x,loc_y=1,1
+        [/store_unit_defense_on]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 40}}
+
+        {SUCCEED}
+    [/event]
+)}
+
+# Expected to succeed but trigger a deprecation warning (a BROKE_STRICT result)
+{GENERIC_UNIT_TEST "test_store_unit_defense_deprecated" (
+    [event]
+        name = start
+
+        # Alice and Bob are both standing on castles, add another unit on grassland
+        {NOTRAIT_UNIT 1 "Footpad" 1 1}
+
+        [store_unit_defense]
+            id=alice
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 40}}
+
+        # Check that both terrain= and variable= are supported
+        [store_unit_defense]
+            type="Footpad"
+            terrain=Ke
+            variable=footpad_defense
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL footpad_defense equals 30}}
+        # Check it didn't overwrite the default variable
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 40}}
+
+        [store_unit_defense]
+            id=alice
+            terrain=Ke
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 40}}
+
+        [store_unit_defense]
+            id=alice
+            terrain=Gg
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 60}}
+
+        # Void is one of the terrains that's always defined, even if it's not used on the map
+        [store_unit_defense]
+            id=alice
+            terrain=Xv
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 100}}
+
+        # On this map, 1,1 is grassland
+        [store_unit_defense]
+            id=alice
+            loc_x,loc_y=1,1
+        [/store_unit_defense]
+        {ASSERT {VARIABLE_CONDITIONAL terrain_defense equals 60}}
+
+        {SUCCEED}
+    [/event]
+)}

wml_test_schedule

@@ -103,6 +103,8 @@
 0 events-test_victory
 0 events-test_defeat
 0 events-test_die
+0 test_store_unit_defense_on
+5 test_store_unit_defense_deprecated
 #
 # LUA
 #