fix code problems found by luacheck (#2380)

actual bugs found:
* backwards_compatibility.lua (undeclared global "helper")
* core.lua (use of undeclared global "helper")
* wml_tags.transform_unit had wrong code to deal with recall_cost
* wrong variable name in cave_map_generator

data/lua/backwards-compatibility.lua

@@ -2,8 +2,6 @@
 
 -- This file may provide an implementation of Lua functions removed from the engine.
 
-local helper = wesnoth.require "helper"
-
 function wesnoth.set_music(cfg)
 	wesnoth.wml_actions.music(cfg)
 end

data/lua/cave_map_generator.lua

@@ -1,7 +1,6 @@
 local helper = wesnoth.require "helper"
 local MG = wesnoth.require "mapgen_helper"
 local LS = wesnoth.require "location_set"
-local T = helper.set_wml_tag_metatable {}
 local random = wesnoth.random
 
 local callbacks = {}
@@ -123,7 +122,7 @@ function callbacks.generate_map(params)
 			local laziness = math.max(v.data.laziness or 1, 1)
 			local width = math.max(v.data.width or 1, 1)
 			local jagged = v.data.jagged or 0
-			local calc = function(x, y, current_cost)
+			local calc = function(x, y)
 				local res = 1.0
 				if map:get_tile(x, y) == params.terrain_wall then
 					res = laziness
@@ -133,7 +132,8 @@ function callbacks.generate_map(params)
 				end
 				return res
 			end
-			local path, cost = wesnoth.find_path(v.start_x, v.start_y, v.dest_x, v.dest_y, calc, params.map_width, params.map_height)
+			local path = wesnoth.find_path(
+				v.start_x, v.start_y, v.dest_x, v.dest_y, calc, params.map_width, params.map_height)
 			for i, loc in ipairs(path) do
 				local locs_set = LS.create()
 				build_chamber(loc[1], loc[2], locs_set, width, jagged)
@@ -168,7 +168,7 @@ function callbacks.generate_scenario(params)
 	local scenario = helper.get_child(params, "scenario")
 	scenario.map_data = callbacks.generate_map(params)
 	for chamber in helper.child_range(params, "chamber") do
-		local items = helper.get_child(chamber, "items")
+		local chamber_items = helper.get_child(chamber, "items")
 		if chamber.chance == 100 and chamber_items then
 			-- TODO: Should we support [event]same_location_as_previous=yes?
 			for i,tag in ipairs(chamber_items) do

data/lua/core.lua

@@ -93,7 +93,7 @@ end
 function wml.child_array(cfg, tag)
 	ensure_config(cfg)
 	local result = {}
-	for val in helper.child_range(cfg, tag) do
+	for val in wml.child_range(cfg, tag) do
 		table.insert(result, val)
 	end
 	return result
@@ -159,7 +159,7 @@ function wesnoth.deprecation_message(elem_name, level, version, detail)
 		message = wesnoth.format(_"$elem has been deprecated indefinitely.", message_params)
 	elseif level == 2 then
 		logger = function(msg) wesnoth.log("warn", msg) end
-		if wesnoth.compare_versions(game_config.version, "<", version) then
+		if wesnoth.compare_versions(wesnoth.game_config.version, "<", version) then
 			message_params.version = version
 			message = wesnoth.format(_"$elem has been deprecated and may be removed in version $version.", message_params)
 		else
@@ -185,7 +185,9 @@ end
 function wesnoth.deprecate_api(elem_name, replacement, level, version, elem, detail_msg)
 	local message = detail_msg or ''
 	if replacement then
-		message = message .. " " .. wesnoth.format(_"(Note: You should use $replacement instead in new code)", {replacement = replacement})
+		message = message .. " " .. wesnoth.format(
+			_"(Note: You should use $replacement instead in new code)",
+			{replacement = replacement})
 	end
 	if type(level) ~= "number" or level < 1 or level > 4 then
 		error("Invalid deprecation level! Must be 1-4.")

data/lua/helper.lua

@@ -164,11 +164,6 @@ function helper.get_user_choice(attr, options)
 	return result
 end
 
-local adjacent_offset = {
-	[false] = { {0,-1}, {1,-1}, {1,0}, {0,1}, {-1,0}, {-1,-1} },
-	[true] = { {0,-1}, {1,0}, {1,1}, {0,1}, {-1,1}, {-1,0} }
-}
-
 --! Returns an iterator over adjacent locations that can be used in a for-in loop.
 -- Not deprecated because, unlike wesnoth.map.get_adjacent_tiles,
 -- this verifies that the locations are on the map.
@@ -196,7 +191,9 @@ end
 
 function helper.rand (possible_values, random_func)
 	random_func = random_func or wesnoth.random
-	assert(type(possible_values) == "table" or type(possible_values) == "string", string.format("helper.rand expects a string or table as parameter, got %s instead", type(possible_values)))
+	assert(type(possible_values) == "table" or type(possible_values) == "string",
+		string.format("helper.rand expects a string or table as parameter, got %s instead",
+		type(possible_values)))
 
 	local items = {}
 	local num_choices = 0
@@ -297,11 +294,13 @@ function helper.round( number )
 	return number
 end
 
-function helper.shuffle( t, random_func)
+function helper.shuffle( t, random_func )
 	random_func = random_func or wesnoth.random
 	-- since tables are passed by reference, this is an in-place shuffle
 	-- it uses the Fisher-Yates algorithm, also known as Knuth shuffle
-	assert( type( t ) == "table", string.format( "helper.shuffle expects a table as parameter, got %s instead", type( t ) ) )
+	assert(
+		type( t ) == "table",
+		string.format( "helper.shuffle expects a table as parameter, got %s instead", type( t ) ) )
 	local length = #t
 	for index = length, 2, -1 do
 		local random = random_func( 1, index )

data/lua/ilua.lua

@@ -16,7 +16,6 @@ local sub = string.sub
 local push = table.insert
 local pop = table.remove
 local pack = table.pack
-local floor = math.floor
 
 local declared = {}
 
@@ -69,7 +68,7 @@ function ilua.join(tbl,delim,limit,depth)
             if k > limit then
                 res = res.." ... "
                 break
-            end            
+            end
         end
     end
     pop(jstack)

data/lua/mapgen_helper.lua

@@ -24,10 +24,6 @@ local function loc_to_index(map,x,y)
 	return x + 1 + y * map.w
 end
 
-local function index_to_loc(map,index)
-	return (index - 1) % map.w, math.floor((index - 1) / map.w)
-end
-
 function map_mt.__index.set_tile(map, x, y, val)
 	map[loc_to_index(map, x, y)] = val
 end

data/lua/on_event.lua

@@ -23,9 +23,9 @@ local function on_event(eventname, arg1, arg2)
 		end
 	end
 	local priority = 0
-	local handler = nil
+	local handler
 	if type(arg1) == "function" then
-		handler = arg1	
+		handler = arg1
 	else
 		priority = arg1
 		handler = arg2

data/lua/wml-flow.lua

@@ -44,7 +44,7 @@ wml_actions["while"] = function( cfg )
 	if helper.child_count(cfg, "do") == 0 then
 		helper.wml_error "[while] does not contain any [do] tags"
 	end
-	
+
 	-- execute [do] up to 65536 times
 	for i = 1, 65536 do
 		if wesnoth.eval_conditional( cfg ) then
@@ -80,7 +80,7 @@ wesnoth.wml_actions["for"] = function(cfg)
 	if helper.child_count(cfg, "do") == 0 then
 		helper.wml_error "[for] does not contain any [do] tags"
 	end
-	
+
 	local loop_lim = {}
 	local first
 	if cfg.array ~= nil then
@@ -154,7 +154,7 @@ wml_actions["repeat"] = function(cfg)
 	if helper.child_count(cfg, "do") == 0 then
 		helper.wml_error "[repeat] does not contain any [do] tags"
 	end
-	
+
 	local times = cfg.times or 1
 	for i = 1, times do
 		for do_child in helper.child_range( cfg, "do" ) do
@@ -176,7 +176,7 @@ function wml_actions.foreach(cfg)
 	if helper.child_count(cfg, "do") == 0 then
 		helper.wml_error "[foreach] does not contain any [do] tags"
 	end
-	
+
 	local array_name = cfg.array or helper.wml_error "[foreach] missing required array= attribute"
 	local array = helper.get_variable_array(array_name)
 	if #array == 0 then return end -- empty and scalars unwanted
@@ -185,7 +185,7 @@ function wml_actions.foreach(cfg)
 	local i_name = cfg.index_var or "i"
 	local i = utils.start_var_scope(i_name) -- if i is already set
 	local array_length = wesnoth.get_variable(array_name .. ".length")
-	
+
 	for index, value in ipairs(array) do
 		-- Some protection against external modification
 		-- It's not perfect, though - it'd be nice if *any* change could be detected
@@ -214,21 +214,21 @@ function wml_actions.foreach(cfg)
 		end
 	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.
-		
+
 		If readonly=yes, then this line guarantees that the array
 		is unchanged after the [foreach] loop ends.
-		
+
 		If readonly=no, then this line updates the array with any
 		changes the user has applied through the $this_item
 		variable (or whatever variable was given in item_var).
-		
+
 		Note that altering the array via indexing (with the index_var)
 		is not supported; any such changes will be reverted by this line.
 	]]
@@ -242,7 +242,7 @@ function wml_actions.switch(cfg)
 	-- Execute all the [case]s where the value matches.
 	for v in helper.child_range(cfg, "case") do
 		for w in utils.split(v.value) do
-			if w == tostring(var_value) then 
+			if w == tostring(var_value) then
 				local action = utils.handle_event_commands(v, "switch")
 				found = true
 				if action ~= "none" then goto exit end

data/lua/wml-tags.lua

@@ -125,7 +125,7 @@ end
 
 function wml_actions.store_unit_type_ids(cfg)
 	local types = {}
-	for k,v in pairs(wesnoth.unit_types) do
+	for k in pairs(wesnoth.unit_types) do
 		table.insert(types, k)
 	end
 	table.sort(types)
@@ -276,7 +276,7 @@ function wml_actions.unit_worth(cfg)
 end
 
 function wml_actions.lua(cfg)
-	local cfg = helper.shallow_literal(cfg)
+	cfg = helper.shallow_literal(cfg)
 	local bytecode, message = load(cfg.code or "")
 	if not bytecode then error("~lua:" .. message, 0) end
 	bytecode(helper.get_child(cfg, "args"))
@@ -580,7 +580,7 @@ function wml_actions.transform_unit(cfg)
 
 			unit.hitpoints = hitpoints
 			unit.experience = experience
-			recall_cost = unit.recall_cost
+			unit.recall_cost = recall_cost
 
 			for key, value in pairs(status) do unit.status[key] = value end
 			if unit.status.unpoisonable then unit.status.poisoned = nil end
@@ -650,7 +650,7 @@ end
 
 function wml_actions.store_starting_location(cfg)
 	local writer = utils.vwriter.init(cfg, "location")
-	for possibly_wrong_index, side in ipairs(wesnoth.get_sides(cfg)) do
+	for _, side in ipairs(wesnoth.get_sides(cfg)) do
 		local loc = wesnoth.get_starting_location(side.side)
 		if loc then
 			local terrain = wesnoth.get_terrain(loc[1], loc[2])
@@ -847,7 +847,7 @@ wml_actions.unstore_unit = function(cfg)
 			x,y = wesnoth.find_vacant_tile(x, y, check_passability and unit)
 		end
 		unit:to_map(x, y, cfg.fire_event)
-		local text = nil
+		local text
 		if unit_cfg.gender == "female" then
 			text = cfg.female_text or cfg.text
 		else
@@ -956,7 +956,7 @@ function wesnoth.wml_actions.store_unit_defense(cfg)
 	local unit = wesnoth.get_units(cfg)[1] or helper.wml_error "[store_unit_defense]'s filter didn't match any unit"
 	local terrain = cfg.terrain
 	local defense
-	
+
 	if terrain then
 		defense = wesnoth.unit_defense(unit, terrain)
 	elseif cfg.loc_x and cfg.loc_y then

data/lua/wml-utils.lua

@@ -65,8 +65,8 @@ function utils.get_sides(cfg, key_name, filter_name)
 end
 
 function utils.optional_side_filter(cfg, key_name, filter_name)
-	local key_name = key_name or "side"
-	local filter_name = filter_name or "filter_side"
+	key_name = key_name or "side"
+	filter_name = filter_name or "filter_side"
 	if cfg[key_name] == nil and helper.get_child(cfg, filter_name) == nil then
 		return true
 	end
@@ -116,7 +116,7 @@ function utils.handle_event_commands(cfg, scope_type)
 		local insert_from
 		if cmd == "insert_tag" then
 			cmd = arg.name
-			local from = arg.variable or 
+			local from = arg.variable or
 				helper.wml_error("[insert_tag] found with no variable= field")
 
 			arg = wesnoth.get_variable(from)