Use to-be-closed variables to scope WML variables in tag definitions (#5536)

data/lua/wml-flow.lua

@@ -111,7 +111,7 @@ wesnoth.wml_actions["for"] = function(cfg)
 		return
 	end
 	local i_var = cfg.variable or "i"
-	local save_i = utils.start_var_scope(i_var)
+	local save_i <close> = utils.scoped_var(i_var)
 	wml.variables[i_var] = first
 	local function loop_condition()
 		local sentinel = loop_lim.last
@@ -146,7 +146,6 @@ wesnoth.wml_actions["for"] = function(cfg)
 		wml.variables[i_var] = wml.variables[i_var] + loop_lim.step
 	end
 	::exit::
-	utils.end_var_scope(i_var, save_i)
 end
 
 wml_actions["repeat"] = function(cfg)
@@ -180,9 +179,9 @@ function wml_actions.foreach(cfg)
 	local array = wml.array_variables[array_name]
 	if #array == 0 then return end -- empty and scalars unwanted
 	local item_name = cfg.variable or "this_item"
-	local this_item = utils.start_var_scope(item_name) -- if this_item is already set
+	local this_item <close> = utils.scoped_var(item_name) -- if this_item is already set
 	local i_name = cfg.index_var or "i"
-	local i = utils.start_var_scope(i_name) -- if i is already set
+	local i <close> = utils.scoped_var(i_name) -- if i is already set
 	local array_length = wml.variables[array_name .. ".length"]
 
 	for index, value in ipairs(array) do
@@ -214,10 +213,6 @@ function wml_actions.foreach(cfg)
 	end
 	::exit::
 
-	-- house cleaning
-	utils.end_var_scope(item_name, this_item)
-	utils.end_var_scope(i_name, i)
-
 	--[[
 		This forces the readonly key to be taken literally.
 

data/lua/wml-utils.lua

@@ -166,14 +166,14 @@ function utils.parenthetical_split(str)
 end
 
 --note: when using these, make sure that nothing can throw over the call to end_var_scope
-function utils.start_var_scope(name)
+local function start_var_scope(name)
 	local var = wml.array_variables[name] --containers and arrays
 	if #var == 0 then var = wml.variables[name] end --scalars (and nil/empty)
 	wml.variables[name] = nil
 	return var
 end
 
-function utils.end_var_scope(name, var)
+local function end_var_scope(name, var)
 	wml.variables[name] = nil
 	if type(var) == "table" then
 		wml.array_variables[name] = var
@@ -182,8 +182,43 @@ function utils.end_var_scope(name, var)
 	end
 end
 
+function utils.scoped_var(name)
+	local orig = start_var_scope(name)
+	return setmetatable({
+		set = function(self, new)
+			if type(new) == "table" then
+				wml.array_variables[name] = new
+			else
+				wml.variables[name] = new
+			end
+		end,
+		get = function(self)
+			local val = wml.array_variables[name]
+			if #val == 0 then val = wml.variables[name] end
+			return val
+		end
+	}, {
+		__metatable = "scoped WML variable",
+		__close = function(self)
+			end_var_scope(name, orig)
+		end,
+		__index = function(self, key)
+			if key == '__original' then
+				return orig
+			end
+		end,
+		__newindex = function(self, key, val)
+			if key == '__original' then
+				error("scoped variable '__original' value is read-only", 1)
+			end
+		end
+	})
+end
+
 utils.trim = wesnoth.deprecate_api('wml_utils.trim', 'stringx.trim', 1, nil, stringx.trim)
 utils.parenthetical_split = wesnoth.deprecate_api('wml_utils.parenthetical_split', 'stringx.quoted_split or stringx.split', 1, nil, utils.parenthetical_split)
 utils.split = wesnoth.deprecate_api('wml_utils.split', 'stringx.split', 1, nil, utils.split)
+utils.start_var_scope = wesnoth.deprecate_api('wml_utils.start_var_scope', 'wml_utils.scoped_var', 1, nil, start_var_scope, 'Assign the scoped_var to a to-be-closed local variable.')
+utils.end_var_scope = wesnoth.deprecate_api('wml_utils.end_var_scope', 'wml_utils.scoped_var', 1, nil, end_var_scope, 'Assign the scoped_var to a to-be-closed local variable.')
 
 return utils

data/lua/wml/find_path.lua

@@ -6,7 +6,7 @@ function wesnoth.wml_actions.find_path(cfg)
 	local unit = wesnoth.units.find_on_map(filter_unit)[1] or wml.error("[find_path]'s filter didn't match any unit")
 	local filter_location = wml.get_child(cfg, "destination") or wml.error( "[find_path] missing required [destination] tag" )
 	-- support for $this_unit
-	local this_unit = utils.start_var_scope("this_unit")
+	local this_unit <close> = utils.scoped_var("this_unit")
 
 	wml.variables["this_unit"] = nil -- clearing this_unit
 	wml.variables["this_unit"] = unit.__cfg -- cfg field needed
@@ -116,17 +116,13 @@ function wesnoth.wml_actions.find_path(cfg)
 
 		if cost >= 42424241 then -- it's the high value returned for unwalkable or busy terrains
 			wml.variables[tostring(variable)] = { hexes = 0 } -- set only length, nil all other values
-			-- support for $this_unit
-			wml.variables["this_unit"] = nil -- clearing this_unit
-			utils.end_var_scope("this_unit", this_unit)
-		return end
+			return
+		end
 
 		if not allow_multiple_turns and turns > 1 then -- location cannot be reached in one turn
 			wml.variables[tostring(variable)] = { hexes = 0 }
-			-- support for $this_unit
-			wml.variables["this_unit"] = nil -- clearing this_unit
-			utils.end_var_scope("this_unit", this_unit)
-		return end -- skip the cycles below
+			return
+		end -- skip the cycles below
 
 		wml.variables[tostring( variable )] =
 			{
@@ -165,8 +161,4 @@ function wesnoth.wml_actions.find_path(cfg)
 				}
 		end
 	end
-
-	-- support for $this_unit
-	wml.variables["this_unit"] = nil -- clearing this_unit
-	utils.end_var_scope("this_unit", this_unit)
 end

data/lua/wml/harm_unit.lua

@@ -19,7 +19,7 @@ function wml_actions.harm_unit(cfg)
 		else return false end
 	end
 
-	local this_unit = utils.start_var_scope("this_unit")
+	local this_unit <close> = utils.scoped_var("this_unit")
 
 	for index, unit_to_harm in ipairs(wesnoth.units.find_on_map(filter)) do
 		if unit_to_harm.valid then
@@ -202,7 +202,4 @@ function wml_actions.harm_unit(cfg)
 
 		wml_actions.redraw {}
 	end
-
-	wml.variables["this_unit"] = nil -- clearing this_unit
-	utils.end_var_scope("this_unit", this_unit)
 end

data/lua/wml/modify_unit.lua

@@ -184,12 +184,11 @@ local function simple_modify_unit(cfg)
 		end
 	end
 
-	local this_unit = utils.start_var_scope("this_unit")
+	local this_unit <close> = utils.scoped_var("this_unit")
 	for i, u in ipairs(wesnoth.units.find(filter)) do
 		wml.variables["this_unit"] = u.__cfg
 		handle_unit(u)
 	end
-	utils.end_var_scope("this_unit", this_unit)
 end
 
 function wml_actions.modify_unit(cfg)
@@ -289,11 +288,10 @@ function wml_actions.modify_unit(cfg)
 	wml_actions.store_unit { {"filter", filter}, variable = unit_variable }
 	local max_index = wml.variables[unit_variable .. ".length"] - 1
 
-	local this_unit = utils.start_var_scope("this_unit")
+	local this_unit <close> = utils.scoped_var("this_unit")
 	for current_unit = 0, max_index do
 		handle_unit(current_unit)
 	end
-	utils.end_var_scope("this_unit", this_unit)
 
 	wml.variables[unit_variable] = nil
 end

data/lua/wml/random_placement.lua

@@ -9,7 +9,7 @@ wesnoth.wml_actions.random_placement = function(cfg)
 	local num_items = cfg.num_items or wml.error("[random_placement] missing required 'num_items' attribute")
 	local variable = cfg.variable or wml.error("[random_placement] missing required 'variable' attribute")
 	local allow_less = cfg.allow_less == true
-	local variable_previous = utils.start_var_scope(variable)
+	local variable_previous <close> = utils.scoped_var(variable)
 	local math_abs = math.abs
 	local locs = wesnoth.get_locations(filter)
 	if type(num_items) == "string" then
@@ -87,11 +87,8 @@ wesnoth.wml_actions.random_placement = function(cfg)
 				utils.set_exiting("none")
 				break
 			elseif action ~= "none" then
-				utils.end_var_scope(variable, variable_previous)
 				return
 			end
 		end
 	end
-	utils.end_var_scope(variable, variable_previous)
-
 end

data/test/scenarios/test_scoped_vars.cfg

@@ -0,0 +1,27 @@
+{GENERIC_UNIT_TEST "test_scoped_vars" (
+    [event]
+        name = prestart
+		{VARIABLE test_var 1}
+		#[inspect][/inspect]
+		{ASSERT {VARIABLE_CONDITIONAL test_var equals 1}}
+        [lua]
+            code = <<
+				local wml_utils = wesnoth.require "wml-utils"
+				local var <close> = wml_utils.scoped_var("test_var")
+				-- This runs the contents of [args] as WML actions
+				wml_utils.handle_event_commands(...)
+			>>
+			[args]
+				#[inspect][/inspect]
+				{ASSERT {VARIABLE_CONDITIONAL test_var equals ""}}
+				{VARIABLE test_var 5}
+				#[inspect][/inspect]
+				{ASSERT {VARIABLE_CONDITIONAL test_var equals 5}}
+			[/args]
+        [/lua]
+
+		#[inspect][/inspect]
+        {ASSERT {VARIABLE_CONDITIONAL test_var equals 1}}
+		{SUCCEED}
+    [/event]
+)}

wml_test_schedule

@@ -115,6 +115,7 @@
 0 test_wml_actions
 0 test_wml_conditionals
 0 lua_wml_tagnames
+0 test_scoped_vars
 0 as_text
 #
 # Pathfinding