Merge pull request #517 from CelticMinstrel/fix-for

Fix for-loops

changelog

@@ -37,6 +37,24 @@ Version 1.13.1+dev:
    * New parameter write_to_mods in wesnoth.add_modification
    * Added wesnoth.random function
    * helper.shuffle is now synced
+   * Remove wesnoth.get_unit(underlying_id)
+   * Add wesnoth.get_unit(string_id)
+   * Change wesnoth.message so that it can display translatable strings
+   * Change wesnoth.put_unit so that the unit is passed as the first parameter
+   * Add wesnoth.erase_unit, which replaces wesnoth.put_unit when called without a unit
+   * Add wesnoth.unit_vision_cost
+   * Add wesnoth.unit_jamming_cost
+   * Add methods to proxy unit metatable:
+       matches, to_map, to_recall, clone, extract, advance, add_modification,
+       resistance, defense, movement, vision, jamming, ability, transform
+     All are equivalent to a similar wesnoth.* function, but are called as
+     unit:fcn_name(arguments) instead of as wesnoth.fcn_name(unit, arguments)
+   * Add wesnoth.races[race_id].traits
+   * Add wesnoth.unit_types[unit_type_id].traits
+   * pairs() and ipairs() now work on vconfig userdata objects
+   * Add helper.get_nth_child
+   * Add helper.child_count
+   * Add helper.child_array
  * Music and sound effects:
    * New dwarf hit and die sounds.
  * Terrains:
@@ -107,6 +125,8 @@ Version 1.13.1+dev:
    * Add new looping tags: [for], [foreach], [repeat]
    * Add new flow control tags: [break], [continue], [return]
    * Added a new [difficulty] tag for defining a campaign's difficulty level
+   * Add new syntax for [option], similar to the new difficulty syntax
+   * Add [explain] ActionWML that tells why a conditional failed (for debugging)
  * Editor:
    * Added Category field and color sliders to the Edit Label panel.
  * Miscellaneous and bug fixes:

data/lua/wml-tags.lua

@@ -321,17 +321,16 @@ wml_actions["while"] = function( cfg )
 				local action = utils.handle_event_commands(do_child, "loop")
 				if action == "break" then
 					utils.set_exiting("none")
-					goto exit
+					return
 				elseif action == "continue" then
 					utils.set_exiting("none")
 					break
 				elseif action ~= "none" then
-					goto exit
+					return
 				end
 			end
 		else return end
 	end
-	::exit::
 end
 
 wml_actions["break"] = function(cfg)
@@ -348,7 +347,7 @@ end
 
 wesnoth.wml_actions["for"] = function(cfg)
 	local first, last, step
-	if cfg.array then
+	if cfg.array ~= nil then
 		first = 0
 		last = wesnoth.get_variable(cfg.array .. ".length") - 1
 		step = 1
@@ -359,22 +358,28 @@ wesnoth.wml_actions["for"] = function(cfg)
 	else
 		first = cfg.start or 0
 		last = cfg["end"] or first
-		step = cfg.step or ((last - first) / math.abs(last - first))
+		step = cfg.step
+		if not step then
+			if last < first then step = -1 else step = 1 end
+		end
 	end
-	if ((last - first) / math.abs(last - first)) ~= (step / math.abs(step)) then
+	if step == 0 then -- Sanity check
+		helper.wml_error("[for] has a step of 0!")
+	end
+	if (first < last and step < 0) or (first > last and step > 0) then
 		-- Sanity check: If they specify something like start,end,step=1,4,-1
-		-- then we interpret it as start,end,step=4,1,-1
-		-- (The step takes precedence since it's optional.)
-		last, first = first, last
+		-- then we do nothing
+		return
 	end
 	local i_var = cfg.variable or "i"
 	local save_i = utils.start_var_scope(i_var)
+	local sentinel = last + step
 	wesnoth.set_variable(i_var, first)
 	local function loop_condition()
-		if first < last then
-			return wesnoth.get_variable(i_var) <= last
+		if first < sentinel then
+			return wesnoth.get_variable(i_var) < sentinel
 		else
-			return wesnoth.get_variable(i_var) >= last
+			return wesnoth.get_variable(i_var) > sentinel
 		end
 	end
 	while loop_condition() do
@@ -499,6 +504,58 @@ function wml_actions.switch(cfg)
 	end
 end
 
+-- This is mainly for use in unit test macros, but maybe it can be useful elsewhere too
+function wml_actions.explain(cfg)
+	local logger = cfg.logger or "warning"
+	
+	-- This function returns true if it managed to explain the failure
+	local function explain(current_cfg, expect)
+		for i,t in ipairs(current_cfg) do
+			local tag, this_cfg = t[1], t[2]
+			-- Some special cases
+			if tag == "or" or tag == "and" then
+				if explain(current_cfg, expect) then
+					return true
+				end
+			elseif tag == "not" then
+				if explain(current_cfg, not expect) then
+					return true
+				end
+			elseif tag == "true" or tag == "false" then
+				-- We don't explain these ones.
+				return true
+			elseif wesnoth.eval_conditional{t} == expect then
+				local explanation = "The following conditional test %s:"
+				if expect then
+					explanation = explanation:format("passed")
+				else
+					explanation = explanation:format("failed")
+				end
+				explanation = string.format("%s\n\t[%s]", explanation, tag)
+				for k,v in pairs(this_cfg) do
+					if type(k) ~= "number" then
+						local format = "%s\n\t\t%s=%s"
+						local literal = helper.literal(this_cfg)[k]
+						if literal ~= v then
+							format = format + "=%s"
+						end
+						explanation = string.format(format, explanation, k, literal, v)
+					end
+				end
+				explanation = string.format("%s\n\t[/%s]", explanation, tag)
+				if tag == "variable" then
+					explanation = string.format("%s\n\tNote: The variable %s currently has the value %q.", explanation, this_cfg.name, wesnoth.get_variable(this_cfg.name))
+				end
+				wesnoth.wml_actions.wml_message{message = explanation, logger = logger}
+				return true
+			end
+		end
+	end
+	
+	-- Use not twice here to convert nil to false
+	explain(cfg, not not cfg.result)
+end
+
 function wml_actions.scroll_to(cfg)
 	local loc = wesnoth.get_locations( cfg )[1]
 	if not loc then return end

data/test/macros/wml_unit_test_macros.cfg

@@ -9,6 +9,10 @@
             [/endlevel]
         [/then]
         [else]
+        	[explain]
+        		result=no
+        		{X}
+        	[/explain]
             [endlevel]
                 result=defeat
                 linger_mode = yes
@@ -21,6 +25,10 @@
     [if]
         {X}
         [else]
+        	[explain]
+        		result=no
+        		{X}
+        	[/explain]
             [endlevel]
                 result=defeat
                 linger_mode = yes

data/test/scenarios/for-loops.cfg

@@ -0,0 +1,206 @@
+
+# Note: All these loops must be set up to run either 0 or 1 times.
+#define FOR_LOOP_TEST_STEP NAME START END STEP CONTENT
+	{GENERIC_UNIT_TEST {NAME} (
+		[event]
+			name=start
+			{VARIABLE n 0}
+			[for]
+				start,end,step={START},{END},{STEP}
+				[do]
+					{VARIABLE_OP n add 1}
+					{ASSERT (
+						{VARIABLE_CONDITIONAL i equals {START}}
+						[or]
+							{VARIABLE_CONDITIONAL i equals {END}}
+						[/or]
+					)}
+				[/do]
+			[/for]
+			[fire_event]
+				name=no_error
+			[/fire_event]
+		[/event]
+		{CONTENT}
+	)}
+#enddef
+
+{FOR_LOOP_TEST_STEP forloop_all_zero 0 0 0 (
+	[event]
+		name=no_error
+		{FAIL}
+	[/event]
+	[event]
+		name=start
+		{SUCCEED}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_step_zero 0 1 0 (
+	[event]
+		name=no_error
+		{FAIL}
+	[/event]
+	[event]
+		name=start
+		{SUCCEED}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_once_positive 0 0 1 (
+	[event]
+		name=no_error
+		{RETURN ({VARIABLE_CONDITIONAL n equals 1})}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_once_negative 0 0 -1 (
+	[event]
+		name=no_error
+		{RETURN ({VARIABLE_CONDITIONAL n equals 1})}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_twice_matched 0 1 1 (
+	[event]
+		name=no_error
+		{RETURN ({VARIABLE_CONDITIONAL n equals 2})}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_twice_unmatched 0 1 -1 (
+	[event]
+		name=no_error
+		{RETURN ({VARIABLE_CONDITIONAL n equals 0})}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_step_large_positive 0 10 10 (
+	[event]
+		name=no_error
+		{SUCCEED}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+{FOR_LOOP_TEST_STEP forloop_step_large_negative 10 0 -10 (
+	[event]
+		name=no_error
+		{SUCCEED}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+#undef FOR_LOOP_TEST_STEP
+
+{GENERIC_UNIT_TEST forloop_empty_array (
+	[event]
+		name=start
+		{VARIABLE n 0}
+		[for]
+			array=nothing
+			[do]
+				{VARIABLE_OP n add 1}
+			[/do]
+		[/for]
+		[fire_event]
+			name=no_error
+		[/fire_event]
+	[/event]
+	[event]
+		name=no_error
+		{SUCCEED}
+	[/event]
+	[event]
+		name=start
+		{FAIL}
+	[/event]
+)}
+
+#define FOR_LOOP_ARRAY_TEST NAME FINAL_VALUE REVERSE
+	{GENERIC_UNIT_TEST {NAME} (
+		[event]
+			name=start
+			[set_variables]
+				name=array
+				[value]
+					value=10
+				[/value]
+				[value]
+					value=7
+				[/value]
+				[value]
+					value=2
+				[/value]
+			[/set_variables]
+			{VARIABLE n 0}
+			[for]
+				array=array
+				reverse={REVERSE}
+				[do]
+					{VARIABLE n $array[$i].value}
+				[/do]
+			[/for]
+			[fire_event]
+				name=no_error
+			[/fire_event]
+		[/event]
+		[event]
+			name=no_error
+			{RETURN ({VARIABLE_CONDITIONAL n equals {FINAL_VALUE}})}
+		[/event]
+		[event]
+			name=start
+			{FAIL}
+		[/event]
+	)}
+#enddef
+
+{FOR_LOOP_ARRAY_TEST forloop_array 2 no}
+
+{FOR_LOOP_ARRAY_TEST forloop_array_reverse 10 yes}
+
+#undef FOR_LOOP_ARRAY_TEST
+
+#define FOR_LOOP_TEST_STEP NAME START END EXTRA CONTENT
+	{GENERIC_UNIT_TEST NAME (
+		[event]
+			name=start
+			{VARIABLE n 0}
+			[for]
+				start,end={START},{END}
+				{EXTRA}
+				[do]
+					{VARIABLE_OP n add 1}
+				[/do]
+			[/for]
+			[fire_event]
+				name=no_error
+			[/fire_event]
+		[/event]
+		{CONTENT}
+	)}
+#enddef
+

run_wml_tests

@@ -262,7 +262,7 @@ do
       LoadFile="$OPTARG"
       ;;
     a )
-      extra_opts+="$OPTARG"
+      extra_opts+=" $OPTARG"
       ;;
     t )
       echo "Replacing default timer of 10 with" "$OPTARG" "seconds."

wml_test_schedule

@@ -156,3 +156,15 @@
 0 check_interrupts_continue_global
 0 check_interrupts_elseif
 0 check_interrupts_case
+# For-loop tests
+0 forloop_all_zero
+0 forloop_step_zero
+0 forloop_once_positive
+0 forloop_once_negative
+0 forloop_twice_matched
+0 forloop_twice_unmatched
+0 forloop_empty_array
+0 forloop_array
+0 forloop_array_reverse
+0 forloop_step_large_positive
+0 forloop_step_large_negative