Fix segfaults and a few other issues in wesnoth.find_path

Co-authored-by: mattsc

data/ai/lua/ca_castle_switch.lua

@@ -20,7 +20,7 @@ local function get_reachable_enemy_leaders(unit, avoid_map)
     for _,e in ipairs(potential_enemy_leaders) do
         -- Cannot use AH.find_path_with_avoid() here as there might be enemies all around the enemy leader
         if (not avoid_map:get(e.x, e.y)) then
-            local path, cost = wesnoth.find_path(unit, e.x, e.y, { ignore_units = true, viewing_side = 0 })
+            local path, cost = wesnoth.find_path(unit, e.x, e.y, { ignore_units = true, ignore_visibility = true })
             if cost < AH.no_path then
                 table.insert(enemy_leaders, e)
             end

data/ai/lua/generic_recruit_engine.lua

@@ -587,7 +587,7 @@ return {
                 -- If the recruited unit cannot reach the target hex, return it to the pool of targets
                 if recruit_data.recruit.target_hex and recruit_data.recruit.target_hex[1] then
                     local unit = wesnoth.units.get(recruit_data.recruit.best_hex[1], recruit_data.recruit.best_hex[2])
-                    local path, cost = wesnoth.find_path(unit, recruit_data.recruit.target_hex[1], recruit_data.recruit.target_hex[2], {viewing_side=0, max_cost=unit.max_moves+1})
+                    local path, cost = wesnoth.find_path(unit, recruit_data.recruit.target_hex[1], recruit_data.recruit.target_hex[2], {ignore_visibility=true, max_cost=unit.max_moves+1})
                     if cost > unit.max_moves then
                         -- The last village added to the list should be the one we tried to aim for, check anyway
                         local last = #recruit_data.castle.assigned_villages_x
@@ -676,7 +676,7 @@ return {
             local target_hex = recruit_data.recruit.target_hex
 
             local reference_hex = target_hex[1] and target_hex or best_hex
-            local enemy_location, distance_to_enemy = AH.get_closest_enemy(reference_hex, wesnoth.current.side, { viewing_side = 0 })
+            local enemy_location, distance_to_enemy = AH.get_closest_enemy(reference_hex, wesnoth.current.side, { ignore_visibility = true })
 
             -- If no enemy is on the map, then we first use closest enemy start hex,
             -- and if that does not exist either, a location mirrored w.r.t the center of the map
@@ -731,7 +731,7 @@ return {
                     random_gender = false
                 }
                 if target_hex[1] then
-                    local path, cost = wesnoth.find_path(recruit_unit, target_hex[1], target_hex[2], {viewing_side=0, max_cost=wesnoth.unit_types[recruit_id].max_moves+1})
+                    local path, cost = wesnoth.find_path(recruit_unit, target_hex[1], target_hex[2], {ignore_visibility=true, max_cost=wesnoth.unit_types[recruit_id].max_moves+1})
                     if cost > wesnoth.unit_types[recruit_id].max_moves then
                         -- Unit cost is effectively higher if cannot reach the village
                         efficiency_index = 2
@@ -854,7 +854,7 @@ return {
                 if target_hex[1] then
                     recruitable_units[recruit_id].x = best_hex[1]
                     recruitable_units[recruit_id].y = best_hex[2]
-                    local path, cost = wesnoth.find_path(recruitable_units[recruit_id], target_hex[1], target_hex[2], {viewing_side=0, max_cost=wesnoth.unit_types[recruit_id].max_moves+1})
+                    local path, cost = wesnoth.find_path(recruitable_units[recruit_id], target_hex[1], target_hex[2], {ignore_visibility=true, max_cost=wesnoth.unit_types[recruit_id].max_moves+1})
                     if cost > wesnoth.unit_types[recruit_id].max_moves then
                         -- penalty if the unit can't reach the target village
                         bonus = bonus - 0.2
@@ -993,7 +993,7 @@ return {
                             local key = unit.type .. '_' .. v[1] .. '-' .. v[2] .. '_' .. c[1]  .. '-' .. c[2]
                             local path, unit_distance
                             if (not recruit_data.unit_distances[key]) then
-                                path, unit_distance = wesnoth.find_path(unit, c[1], c[2], {viewing_side=0, max_cost=fastest_unit_speed+1})
+                                path, unit_distance = wesnoth.find_path(unit, c[1], c[2], {ignore_visibility=true, max_cost=fastest_unit_speed+1})
                                 recruit_data.unit_distances[key] = unit_distance
                             else
                                 unit_distance = recruit_data.unit_distances[key]

data/ai/micro_ais/cas/ca_fast_combat.lua

@@ -58,7 +58,7 @@ function ca_fast_combat:evaluation(cfg, data)
     end
 
     -- Exclude hidden enemies, except if attack_hidden_enemies=yes is set in [micro_ai] tag
-    local viewing_side = wesnoth.current.side
+    local viewing_side, ignore_visibility = wesnoth.current.side, false
     if (not cfg.attack_hidden_enemies) then
         local hidden_enemies = AH.get_live_units {
             { "filter_side", { { "enemy_of", { side = wesnoth.current.side } } } },
@@ -69,7 +69,7 @@ function ca_fast_combat:evaluation(cfg, data)
             enemy_map:remove(e.x, e.y)
         end
     else
-        viewing_side = 0
+        ignore_visibility = true
     end
 
     local aggression = ai.aspects.aggression
@@ -84,7 +84,7 @@ function ca_fast_combat:evaluation(cfg, data)
         if unit and unit.valid and (unit.attacks_left > 0) and (#unit.attacks > 0) then
             local unit_info = FAU.get_unit_info(unit, gamedata)
             local unit_copy = FAU.get_unit_copy(unit.id, gamedata)
-            local attacks = AH.get_attacks({ unit }, { include_occupied = cfg.include_occupied_attack_hexes, viewing_side = viewing_side })
+            local attacks = AH.get_attacks({ unit }, { include_occupied = cfg.include_occupied_attack_hexes, viewing_side = viewing_side, ignore_visibility = ignore_visibility })
 
             if (#attacks > 0) then
                 local max_rating, best_target, best_dst = - math.huge

data/ai/micro_ais/cas/ca_fast_combat_leader.lua

@@ -51,7 +51,7 @@ function ca_fast_combat_leader:evaluation(cfg, data)
     end
 
     -- Exclude hidden enemies, except if attack_hidden_enemies=yes is set in [micro_ai] tag
-    local viewing_side = wesnoth.current.side
+    local viewing_side, ignore_visibility = wesnoth.current.side, false
     if (not cfg.attack_hidden_enemies) then
         local hidden_enemies = AH.get_live_units {
             { "filter_side", { { "enemy_of", { side = wesnoth.current.side } } } },
@@ -62,7 +62,7 @@ function ca_fast_combat_leader:evaluation(cfg, data)
             enemy_map:remove(e.x, e.y)
         end
     else
-        viewing_side = 0
+        ignore_visibility = true
     end
 
     local aggression = ai.aspects.aggression
@@ -119,7 +119,7 @@ function ca_fast_combat_leader:evaluation(cfg, data)
         end
     end
 
-    local attacks = AH.get_attacks({ leader }, { include_occupied = cfg.include_occupied_attack_hexes, viewing_side = viewing_side })
+    local attacks = AH.get_attacks({ leader }, { include_occupied = cfg.include_occupied_attack_hexes, viewing_side = viewing_side, ignore_visibility = ignore_visibility })
 
     if (#attacks > 0) then
         local max_rating, best_target, best_dst = - math.huge

data/campaigns/Heir_To_The_Throne/scenarios/17_Scepter_of_Fire.cfg

@@ -717,7 +717,7 @@
                 local ai_helper = wesnoth.require "ai/lua/ai_helper.lua"
                 local delf = wesnoth.units.find_on_map { id = 'Delfador' }[1]
                 local sceptre_loc= wesnoth.special_locations.sceptre
-                local path = wesnoth.find_path(delf, sceptre_loc[1], sceptre_loc[2], {ignore_units = true, viewing_side = 0}) -- # wmllint: noconvert
+                local path = wesnoth.find_path(delf, sceptre_loc[1], sceptre_loc[2], {ignore_units = true, ignore_visibility = true}) -- # wmllint: noconvert
                 _ = wesnoth.textdomain 'wesnoth-httt'
 
                 local dirs = {  _"I sense the path to the Sceptre is to the east of me.",

data/lua/wml-tags.lua

@@ -437,7 +437,12 @@ function wml_actions.store_reachable_locations(cfg)
 	local range = cfg.range or "movement"
 	local moves = cfg.moves or "current"
 	local variable = cfg.variable or wml.error "[store_reachable_locations] missing required variable= key"
-	local reach_param = { viewing_side = cfg.viewing_side or 0 }
+	local reach_param = { viewing_side = cfg.viewing_side }
+	if cfg.viewing_side == 0 then
+		wml.error "[store_reachable_locations] invalid viewing_side"
+	elseif cfg.viewing_side == nil then
+		reach_param.ignore_visibility = true
+	end
 	if range == "vision" then
 		moves = "max"
 		reach_param.ignore_units = true

data/lua/wml/find_path.lua

@@ -23,7 +23,7 @@ function wesnoth.wml_actions.find_path(cfg)
 	end
 
 	local allow_multiple_turns = cfg.allow_multiple_turns
-	local viewing_side
+	local ignore_visibility = not cfg.check_visibility
 
 	local nearest_by_cost = true
 	local nearest_by_distance = false
@@ -38,8 +38,6 @@ function wesnoth.wml_actions.find_path(cfg)
 		nearest_by_steps = true
 	end
 
-	if not cfg.check_visibility then viewing_side = 0 end -- if check_visiblity then shroud is taken in account
-
 	-- only the first location with the lowest distance and lowest movement cost will match.
 	local locations = wesnoth.get_locations(filter_location)
 
@@ -57,7 +55,12 @@ function wesnoth.wml_actions.find_path(cfg)
 		else
 			local distance = wesnoth.map.distance_between ( unit.x, unit.y, location[1], location[2] )
 			-- if we pass an unreachable location then an empty path and high value cost will be returned
-			local path, cost = wesnoth.find_path( unit, location[1], location[2], { max_cost = max_cost, ignore_units = ignore_units, ignore_teleport = ignore_teleport, viewing_side = viewing_side } )
+			local path, cost = wesnoth.find_path( unit, location[1], location[2], {
+				max_cost = max_cost,
+				ignore_units = ignore_units,
+				ignore_teleport = ignore_teleport,
+				ignore_visibility = ignore_visibility
+			} )
 
 			if #path == 0 or cost >= 42424241 then
 				-- it's not a reachable hex. 42424242 is the high value returned for unwalkable or busy terrains
@@ -104,7 +107,7 @@ function wesnoth.wml_actions.find_path(cfg)
 				max_cost = max_cost,
 				ignore_units = ignore_units,
 				ignore_teleport = ignore_teleport,
-				viewing_side = viewing_side
+				ignore_visibility = ignore_visibility
 			})
 		local turns
 
@@ -142,7 +145,7 @@ function wesnoth.wml_actions.find_path(cfg)
 					max_cost = max_cost,
 					ignore_units = ignore_units,
 					ignore_teleport = ignore_teleport,
-					viewing_side = viewing_side
+					ignore_visibility = ignore_visibility
 				} )
 			local sub_turns
 

src/scripting/game_lua_kernel.cpp

@@ -1695,11 +1695,13 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 	int arg = 1;
 	map_location src, dst;
 	const unit* u = nullptr;
+	int viewing_side = 0;
 
 	if (lua_isuserdata(L, arg))
 	{
 		u = &luaW_checkunit(L, arg);
 		src = u->get_location();
+		viewing_side = u->side();
 		++arg;
 	}
 	else
@@ -1708,6 +1710,7 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 		unit_map::const_unit_iterator ui = units().find(src);
 		if (ui.valid()) {
 			u = ui.get_shared_ptr().get();
+			viewing_side = u->side();
 		}
 		++arg;
 	}
@@ -1721,26 +1724,30 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 		return luaL_argerror(L, arg - 2, "invalid location");
 
 	const gamemap &map = board().map();
-	int viewing_side = 0;
 	bool ignore_units = false, see_all = false, ignore_teleport = false;
 	double stop_at = 10000;
 	std::unique_ptr<pathfind::cost_calculator> calc;
 
 	if (lua_istable(L, arg))
 	{
 		ignore_units = luaW_table_get_def<bool>(L, arg, "ignore_units", false);
-
+		see_all = luaW_table_get_def<bool>(L, arg, "ignore_visibility", false);
 		ignore_teleport = luaW_table_get_def<bool>(L, arg, "ignore_teleport", false);
 
-		stop_at = luaW_table_get_def<double>(L, arg, "stop_at", stop_at);
+		stop_at = luaW_table_get_def<double>(L, arg, "max_cost", stop_at);
 
 
 		lua_pushstring(L, "viewing_side");
 		lua_rawget(L, arg);
 		if (!lua_isnil(L, -1)) {
 			int i = luaL_checkinteger(L, -1);
 			if (i >= 1 && i <= static_cast<int>(teams().size())) viewing_side = i;
-			else see_all = true;
+			else {
+				// If there's a unit, we have a valid side, so fall back to legacy behaviour.
+				// If we don't have a unit, legacy behaviour would be a crash, so let's not.
+				if(u) see_all = true;
+				deprecated_message("wesnoth.find_path with viewing_side=0 (or an invalid side)", DEP_LEVEL::FOR_REMOVAL, {1, 17, 0}, "To consider fogged and hidden units, use ignore_visibility=true instead.");
+			}
 		}
 		lua_pop(L, 1);
 
@@ -1757,20 +1764,22 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 		calc.reset(new lua_pathfind_cost_calculator(L, arg));
 	}
 
-	const team& viewing_team = viewing_side
-		? board().get_team(viewing_side)
-		: board().get_team(u->side());
-
 	pathfind::teleport_map teleport_locations;
 
 	if(!ignore_teleport) {
-		teleport_locations = pathfind::get_teleport_locations(*u, viewing_team, see_all, ignore_units);
+		if(viewing_side == 0) {
+			return luaL_error(L, "wesnoth.find_path: ignore_teleport=false requires a valid viewing_side");
+		} else {
+			teleport_locations = pathfind::get_teleport_locations(*u, board().get_team(viewing_side), see_all, ignore_units);
+		}
 	}
 
 	if (!calc) {
-		if (!u) return luaL_argerror(L, 1, "unit not found");
+		if(!u) {
+			return luaL_argerror(L, 1, "unit not found OR custom cost function not provided");
+		}
 
-		calc.reset(new pathfind::shortest_path_calculator(*u, viewing_team,
+		calc.reset(new pathfind::shortest_path_calculator(*u, board().get_team(viewing_side),
 			teams(), map, ignore_units, false, see_all));
 	}
 
@@ -1819,40 +1828,33 @@ int game_lua_kernel::intf_find_reach(lua_State *L)
 		++arg;
 	}
 
-	int viewing_side = 0;
+	int viewing_side = u->side();
 	bool ignore_units = false, see_all = false, ignore_teleport = false;
 	int additional_turns = 0;
 
 	if (lua_istable(L, arg))
 	{
-		lua_pushstring(L, "ignore_units");
-		lua_rawget(L, arg);
-		ignore_units = luaW_toboolean(L, -1);
-		lua_pop(L, 1);
-
-		lua_pushstring(L, "ignore_teleport");
-		lua_rawget(L, arg);
-		ignore_teleport = luaW_toboolean(L, -1);
-		lua_pop(L, 1);
-
-		lua_pushstring(L, "additional_turns");
-		lua_rawget(L, arg);
-		additional_turns = lua_tointeger(L, -1);
-		lua_pop(L, 1);
+		ignore_units = luaW_table_get_def<bool>(L, arg, "ignore_units", false);
+		see_all = luaW_table_get_def<bool>(L, arg, "ignore_visibility", false);
+		ignore_teleport = luaW_table_get_def<bool>(L, arg, "ignore_teleport", false);
+		additional_turns = luaW_table_get_def<int>(L, arg, "max_cost", additional_turns);
 
 		lua_pushstring(L, "viewing_side");
 		lua_rawget(L, arg);
 		if (!lua_isnil(L, -1)) {
 			int i = luaL_checkinteger(L, -1);
 			if (i >= 1 && i <= static_cast<int>(teams().size())) viewing_side = i;
-			else see_all = true;
+			else {
+				// If there's a unit, we have a valid side, so fall back to legacy behaviour.
+				// If we don't have a unit, legacy behaviour would be a crash, so let's not.
+				if(u) see_all = true;
+				deprecated_message("wesnoth.find_reach with viewing_side=0 (or an invalid side)", DEP_LEVEL::FOR_REMOVAL, {1, 17, 0}, "To consider fogged and hidden units, use ignore_visibility=true instead.");
+			}
 		}
 		lua_pop(L, 1);
 	}
 
-	const team& viewing_team = viewing_side
-		? board().get_team(viewing_side)
-		: board().get_team(u->side());
+	const team& viewing_team = board().get_team(viewing_side);
 
 	pathfind::paths res(*u, ignore_units, !ignore_teleport,
 		viewing_team, additional_turns, see_all, ignore_units);