From 56bdd42815b4166ce53e5f2e20e96d9d4c02f553 Mon Sep 17 00:00:00 2001
From: Celtic Minstrel <CelticMinstrel@users.noreply.github.com>
Date: Thu, 18 Feb 2021 23:15:31 -0500
Subject: [PATCH] Fix segfaults and a few other issues in wesnoth.find_path

Co-authored-by: mattsc
---
 data/ai/lua/ai_helper.lua                     | 161 +++++++++++-------
 data/ai/lua/ca_castle_switch.lua              |   2 +-
 data/ai/lua/generic_recruit_engine.lua        |  10 +-
 data/ai/micro_ais/cas/ca_fast_combat.lua      |   6 +-
 .../micro_ais/cas/ca_fast_combat_leader.lua   |   6 +-
 .../scenarios/17_Scepter_of_Fire.cfg          |   2 +-
 data/lua/wml-tags.lua                         |   7 +-
 data/lua/wml/find_path.lua                    |  15 +-
 src/scripting/game_lua_kernel.cpp             |  62 +++----
 9 files changed, 158 insertions(+), 113 deletions(-)

diff --git a/data/ai/lua/ai_helper.lua b/data/ai/lua/ai_helper.lua
index 789ffc6ee219..b741735aa58f 100644
--- a/data/ai/lua/ai_helper.lua
+++ b/data/ai/lua/ai_helper.lua
@@ -9,20 +9,17 @@ local M = wesnoth.map
 -- development releases, but it is of course easily possible to copy a function
 -- from a previous release directly into an add-on if it is needed there.
 --
--- Invisible units ('viewing_side' parameter):
+-- Invisible units ('viewing_side' and 'ignore_visibility' parameters):
 -- With their default settings, the ai_helper functions use the vision a player of
 -- the respective side would see, that is, they assume no knowledge of invisible
--- units. This can be influenced with the 'viewing_side' parameter, which works
--- in the same way as it does in wesnoth.find_reach() and wesnoth.find_path():
---   - If set to a valid side number, vision for that side is used
---   - If set to an invalid side number (e.g. 0), all units on the map are seen
---   - If omitted and a function takes a a parameter linked to a specific side,
---     such as a side number or a unit, as input, vision of that side is used. In
---     this case, viewing_side is passed as part of the optional @cfg configuration
---     table and can be passed from function to function.
---   - If omitted and the function takes no such input, viewing_side is made a
---     required parameter in order to avoid mismatches between the default values
---     of different functions.
+-- units. This can be influenced with the 'viewing_side' and 'ignore_visibility' parameters,
+-- which work in the same way as they do in wesnoth.find_reach() and wesnoth.find_path():
+--   - If 'viewing_side' is set, vision for that side is used. It must be set to a valid side number.
+--   - If 'ignore_visibility' is set to true, all units on the map are seen and shroud is ignored.
+--       This overrides 'viewing_side'.
+--   - If neither parameter is given and a function takes a parameter linked to a specific side,
+--     such as a side number or a unit, as input, vision of that side is used.
+--   - For some functions that take no other side-related input, 'viewing_side' is made a required parameter.
 --
 -- Path finding:
 -- All ai_helper functions disregard shroud for path finding (while still ignoring
@@ -1066,6 +1063,16 @@ end
 
 --------- Unit related helper functions ----------
 
+function ai_helper.check_viewing_side(viewing_side, function_str)
+    -- Check that viewing_side is valid and set to an existing side
+    if (not viewing_side) then
+        error('ai_helper: missing required parameter viewing_side', 2)
+    end
+    if (type(viewing_side) ~= 'number') or (not wesnoth.sides[viewing_side]) then
+        error('ai_helper: parameter viewing_side must be a valid side number', 2)
+    end
+end
+
 function ai_helper.is_passive_leader(aspect_value, id)
     if (type(aspect_value) == 'boolean') then return aspect_value end
 
@@ -1110,25 +1117,19 @@ function ai_helper.get_visible_units(viewing_side, filter)
     -- Get units that are visible to side @viewing_side
     --
     -- Required parameters:
-    -- @viewing_side: see comments at beginning of this file
+    -- @viewing_side: must be set to a valid side number. If visibility is to be
+    --   ignored, use wesnoth.get_units() instead.
     --
     -- Optional parameters:
     -- @filter: Standard unit filter WML table for the units
     --   Example 1: { type = 'Orcish Grunt' }
     --   Example 2: { { "filter_location", { x = 10, y = 12, radius = 5 } } }
 
-    if (not viewing_side) then
-        error('ai_helper.get_visible_units() is missing required parameter viewing_side.', 2)
-    end
-    if (type(viewing_side) ~= 'number') then
-        error('ai_helper.get_visible_units(): parameter viewing_side must be a number., 2')
-    end
+    ai_helper.check_viewing_side(viewing_side)
 
     local filter_plus_vision = {}
     if filter then filter_plus_vision = ai_helper.table_copy(filter) end
-    if wesnoth.sides[viewing_side] then
-        table.insert(filter_plus_vision, { "filter_vision", { side = viewing_side, visible = 'yes' } })
-    end
+    table.insert(filter_plus_vision, { "filter_vision", { side = viewing_side, visible = 'yes' } })
 
     local units = {}
     local all_units = wesnoth.units.find_on_map()
@@ -1145,21 +1146,14 @@ function ai_helper.is_visible_unit(viewing_side, unit)
     -- Check whether @unit exists and is visible to side @viewing_side.
     --
     -- Required parameters:
-    -- @viewing_side: see comments at beginning of this file.
+    -- @viewing_side: must be set to a valid side number
     -- @unit: unit proxy table
 
-    if (not viewing_side) then
-        error('ai_helper.is_visible_unit() is missing required parameter viewing_side.', 2)
-    end
-    if (type(viewing_side) ~= 'number') then
-        error('ai_helper.is_visible_unit(): parameter viewing_side must be a number.', 2)
-    end
+    ai_helper.check_viewing_side(viewing_side)
 
     if (not unit) then return false end
 
-    if wesnoth.sides[viewing_side]
-        and unit:matches({ { "filter_vision", { side = viewing_side, visible = 'no' } } })
-    then
+    if unit:matches({ { "filter_vision", { side = viewing_side, visible = 'no' } } }) then
         return false
     end
 
@@ -1170,7 +1164,7 @@ function ai_helper.get_attackable_enemies(filter, side, cfg)
     -- Attackable enemies are defined as being being
     --   - enemies of the side defined in @side,
     --   - not petrified
-    --   - and visible to the side defined in @cfg.viewing_side.
+    --   - and visible to the side as defined in @cfg.viewing_side and @cfg.ignore_visibility.
     --   - have at least one adjacent hex that is not inside an area to avoid
     -- For speed reasons, this is done separately, rather than calling ai_helper.get_visible_units().
     --
@@ -1181,15 +1175,18 @@ function ai_helper.get_attackable_enemies(filter, side, cfg)
     -- @side: side number, if side other than current side is to be considered
     -- @cfg: table with optional configuration parameters:
     --   viewing_side: see comments at beginning of this file. Defaults to @side.
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
     --   avoid_map: if given, an enemy is included only if it does not have at least one
     --     adjacent hex outside of avoid_map
 
     side = side or wesnoth.current.side
     local viewing_side = cfg and cfg.viewing_side or side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     local filter_plus_vision = {}
     if filter then filter_plus_vision = ai_helper.table_copy(filter) end
-    if wesnoth.sides[viewing_side] then
+    if (not ignore_visibility) then
         table.insert(filter_plus_vision, { "filter_vision", { side = viewing_side, visible = 'yes' } })
     end
 
@@ -1220,21 +1217,24 @@ function ai_helper.get_attackable_enemies(filter, side, cfg)
 end
 
 function ai_helper.is_attackable_enemy(unit, side, cfg)
-    -- Check if @unit exists, is an enemy of @side, is visible to the side defined
-    -- in @cfg.viewing_side and is not petrified.
+    -- Check if @unit exists, is an enemy of @side, is visible to the side as defined
+    -- by @cfg.viewing_side and @cfg.ignore_visibility and is not petrified.
     --
     -- Optional parameters:
     -- @side: side number, defaults to current side.
     -- @cfg: table with optional configuration parameters:
     --   viewing_side: see comments at beginning of this file. Defaults to @side.
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
 
     side = side or wesnoth.current.side
     local viewing_side = cfg and cfg.viewing_side or side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     if (not unit)
         or (not wesnoth.sides.is_enemy(side, unit.side))
         or unit.status.petrified
-        or (not ai_helper.is_visible_unit(viewing_side, unit))
+        or ((not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, unit)))
     then
         return false
     end
@@ -1251,6 +1251,7 @@ function ai_helper.get_closest_enemy(loc, side, cfg)
     -- @side: number of side for which to find enemy; defaults to current side
     -- @cfg: table with optional configuration parameters:
     --   viewing_side: see comments at beginning of this file. Defaults to @side.
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
 
     side = side or wesnoth.current.side
 
@@ -1443,6 +1444,7 @@ function ai_helper.next_hop(unit, x, y, cfg)
     -- @cfg: standard extra options for wesnoth.find_path()
     --   including:
     --     viewing_side: see comments at beginning of this file. Defaults to side of @unit
+    --     ignore_visibility: see comments at beginning of this file. Defaults to nil.
     --   plus:
     --     ignore_own_units: if set to true, then own units that can move out of the way are ignored
     --     path: if given, find the next hop along this path, rather than doing new path finding
@@ -1455,6 +1457,10 @@ function ai_helper.next_hop(unit, x, y, cfg)
     --       in the way) as possible. Setting 'fan_out=false' restores the old behavior. The main
     --       disadvantage of the new method is that it needs to do more path finding and therefore takes longer.
 
+    local viewing_side = cfg and cfg.viewing_side or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
+
     local path, cost
     if cfg and cfg.path then
         path = cfg.path
@@ -1478,6 +1484,9 @@ function ai_helper.next_hop(unit, x, y, cfg)
                 local unit_in_way
                 if (not cfg) or (not cfg.ignore_units) then
                     unit_in_way = wesnoth.units.get(path[i][1], path[i][2])
+                    if unit_in_way and (not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, unit_in_way)) then
+                        unit_in_way = nil
+                    end
 
                     -- If ignore_own_units is set, ignore own side units that can move out of the way
                     if cfg and cfg.ignore_own_units then
@@ -1528,10 +1537,12 @@ function ai_helper.next_hop(unit, x, y, cfg)
             inverse_reach_map:insert(r[1], r[2], inverse_cost)
         end
 
-        local units = ai_helper.get_visible_units(
-            cfg and cfg.viewing_side or unit.side,
-            { { "not", { id = unit.id } }
-        })
+        local units
+        if ignore_visibility then
+            units = wesnoth.units.find_on_map({ { "not", { id = unit.id } } })
+        else
+            units = ai_helper.get_visible_units(viewing_side, { { "not", { id = unit.id } } })
+        end
         local unit_map = LS.create()
         for _,u in ipairs(units) do unit_map:insert(u.x, u.y, u.id) end
 
@@ -1564,15 +1575,20 @@ function ai_helper.can_reach(unit, x, y, cfg)
     --   ignore_units: if true, ignore both own and enemy units
     --   exclude_occupied: if true, exclude hex if there's a unit there, irrespective of value of 'ignore_units'
     --   viewing_side: see comments at beginning of this file. Defaults to side of @unit
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
 
     cfg = cfg or {}
     local viewing_side = cfg.viewing_side or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
-    -- Is there a unit at the goal hex?
+    -- Is there a visible unit at the goal hex?
     local unit_in_way = wesnoth.units.get(x, y)
-    if (cfg.exclude_occupied)
-      and unit_in_way and ai_helper.is_visible_unit(viewing_side, unit_in_way)
-    then
+    if unit_in_way and (not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, unit_in_way)) then
+        unit_in_way = nil
+    end
+
+    if (cfg.exclude_occupied) and unit_in_way then
         return false
     end
 
@@ -1580,10 +1596,7 @@ function ai_helper.can_reach(unit, x, y, cfg)
     -- or a unit of own side that cannot move away (this might be slow, don't know)
     if (not cfg.ignore_units) then
         -- If there's a unit at the goal that's not on own side (even ally), return false
-        if unit_in_way
-            and (unit_in_way.side ~= unit.side)
-            and ai_helper.is_visible_unit(viewing_side, unit_in_way)
-        then
+        if unit_in_way and (unit_in_way.side ~= unit.side) then
             return false
         end
 
@@ -1619,12 +1632,15 @@ function ai_helper.get_reachmap(unit, cfg)
     -- @cfg: table with optional configuration parameters:
     --   moves: if set to 'max', unit MP is set to max_moves before calculation
     --   viewing_side: see comments at beginning of this file. Defaults to side of @unit
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
     --   exclude_occupied: if true, exclude hexes that have units on them; defaults to
     --     false, in which case hexes with own units with moves > 0 are included
     --   avoid_map: location set of hexes to be excluded
     --   plus all other parameters to wesnoth.find_reach
 
     local viewing_side = cfg and cfg.viewing_side or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     local old_moves = unit.moves
     if cfg and (cfg.moves == 'max') then unit.moves = unit.max_moves end
@@ -1637,7 +1653,14 @@ function ai_helper.get_reachmap(unit, cfg)
             is_available = false
         else
             local unit_in_way = wesnoth.units.get(loc[1], loc[2])
-            if unit_in_way and (unit_in_way.id ~= unit.id) and ai_helper.is_visible_unit(viewing_side, unit_in_way) then
+            if unit_in_way and (unit_in_way.id == unit.id) then
+                unit_in_way = nil
+            end
+            if unit_in_way and (not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, unit_in_way)) then
+                unit_in_way = nil
+            end
+
+            if unit_in_way then
                 if cfg and cfg.exclude_occupied then
                     is_available = false
                 elseif (unit_in_way.side ~= unit.side) or (unit_in_way.moves == 0) then
@@ -1668,7 +1691,7 @@ end
 
 function ai_helper.find_path_with_shroud(unit, x, y, cfg)
     -- Same as wesnoth.find_path, just that it works under shroud as well while still
-    -- ignoring invisible units. It does this by using viewing_side=0 and taking
+    -- ignoring invisible units. It does this by using ignore_visibility=true and taking
     -- invisible units off the map for the path finding process.
     --
     -- Notes on some of the optional parameters that can be passed in @cfg:
@@ -1676,19 +1699,23 @@ function ai_helper.find_path_with_shroud(unit, x, y, cfg)
     --    for determining which units are hidden and need to be extracted, as that
     --    is what the path_finder code uses. If set to an invalid side, we can use
     --    default path finding as shroud is ignored then anyway.
+    --  - ignore_visibility: see comments at beginning of this file. Defaults to nil.
+    --      This applies to the units only in this function, as it always ignores shroud.
     --  - ignore_units: if true, hidden units do not need to be extracted because
     --    all units are ignored anyway
 
     local viewing_side = (cfg and cfg.viewing_side) or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     local path, cost
-    if wesnoth.sides[viewing_side] and wesnoth.sides[viewing_side].shroud then
+    if wesnoth.sides[viewing_side].shroud then
         local extracted_units = {}
         if (not cfg) or (not cfg.ignore_units) then
             local all_units = wesnoth.units.find_on_map()
             for _,u in ipairs(all_units) do
-                if (u.side ~= viewing_side)
-                    and (not ai_helper.is_visible_unit(viewing_side, u))
+                if (u.id ~= unit.id) and (u.side ~= viewing_side)
+                    and (not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, u))
                 then
                     u:extract()
                     table.insert(extracted_units, u)
@@ -1698,7 +1725,7 @@ function ai_helper.find_path_with_shroud(unit, x, y, cfg)
 
         local cfg_copy = {}
         if cfg then cfg_copy = ai_helper.table_copy(cfg) end
-        cfg_copy.viewing_side = 0
+        cfg_copy.ignore_visibility = true
         path, cost = wesnoth.find_path(unit, x, y, cfg_copy)
 
         for _,extracted_unit in ipairs(extracted_units) do
@@ -1943,10 +1970,13 @@ function ai_helper.move_unit_out_of_way(ai, unit, cfg)
     --   dx, dy: the direction in which moving out of the way is preferred
     --   labels: if set, display labels of the rating for each hex the unit can reach
     --   viewing_side: see comments at beginning of this file. Defaults to side of @unit.
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
     --   all other optional parameters to wesnoth.find_reach()
 
     cfg = cfg or {}
     local viewing_side = cfg.viewing_side or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     local dx, dy
     if cfg.dx and cfg.dy then
@@ -1961,7 +1991,7 @@ function ai_helper.move_unit_out_of_way(ai, unit, cfg)
     for _,loc in ipairs(reach) do
         local unit_in_way = wesnoth.units.get(loc[1], loc[2])
         if (not unit_in_way)       -- also excludes current hex
-            or (not ai_helper.is_visible_unit(viewing_side, unit_in_way))
+            or ((not ignore_visibility) and (not ai_helper.is_visible_unit(viewing_side, unit_in_way)))
         then
             local rating = loc[3]  -- also disfavors hexes next to visible enemy units for which loc[3] = 0
 
@@ -2009,9 +2039,12 @@ function ai_helper.movefull_outofway_stopunit(ai, unit, x, y, cfg)
     --
     -- @cfg: table with optional configuration parameters:
     --   viewing_side: see comments at beginning of this file. Defaults to side of @unit
+    --   ignore_visibility: see comments at beginning of this file. Defaults to nil.
     --   all other optional parameters to ai_helper.move_unit_out_of_way() and wesnoth.find_path()
 
     local viewing_side = cfg and cfg.viewing_side or unit.side
+    ai_helper.check_viewing_side(viewing_side)
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     if (type(x) ~= 'number') then
         if x[1] then
@@ -2026,7 +2059,7 @@ function ai_helper.movefull_outofway_stopunit(ai, unit, x, y, cfg)
     if (cost <= unit.moves) then
         local unit_in_way = wesnoth.units.get(x, y)
         if unit_in_way and (unit_in_way ~= unit)
-            and ai_helper.is_visible_unit(viewing_side, unit_in_way)
+            and (ignore_visibility or ai_helper.is_visible_unit(viewing_side, unit_in_way))
         then
             ai_helper.move_unit_out_of_way(ai, unit_in_way, cfg)
         end
@@ -2043,15 +2076,15 @@ end
 ---------- Attack related helper functions --------------
 
 function ai_helper.get_attacks(units, cfg)
-    -- Get all attacks the units stored in @units can do. Invisible enemies are
-    -- excluded unless option @cfg.viewing_side=0 is used.
+    -- Get all attacks the units stored in @units can do. Enemies invisible to the side
+    -- of @units are excluded, unless option @cfg.ignore_visibility=true is used.
     --
     -- This includes a variety of configurable options, passed in the @cfg table
     -- @cfg: table with optional configuration parameters:
     --   moves: "current" (default for units on current side) or "max" (always used for units on other sides)
     --   include_occupied (false): if set, also include hexes occupied by own-side units that can move away
     --   simulate_combat (false): if set, also simulate the combat and return result (this is slow; only set if needed)
-    --   viewing_side: see comments at beginning of this file. Defaults to side of @units
+    --   ignore_visibility: see comments at beginning of this file. Defaults to side of @units
     --   all other optional parameters to wesnoth.find_reach()
     --
     -- Returns {} if no attacks can be done, otherwise table with fields:
@@ -2067,7 +2100,7 @@ function ai_helper.get_attacks(units, cfg)
     if (not units[1]) then return attacks end
 
     local side = units[1].side  -- all units need to be on same side
-    local viewing_side = cfg and cfg.viewing_side or side
+    local ignore_visibility = cfg and cfg.ignore_visibility
 
     -- 'moves' can be either "current" or "max"
     -- For unit on current side: use "current" by default, or override by cfg.moves
@@ -2098,7 +2131,7 @@ function ai_helper.get_attacks(units, cfg)
         if (unit.side == side) then
             my_unit_map:insert(unit.x, unit.y, i)
         else
-            if ai_helper.is_visible_unit(viewing_side, unit) then
+            if ignore_visibility or ai_helper.is_visible_unit(side, unit) then
                 other_unit_map:insert(unit.x, unit.y, i)
             end
         end
@@ -2157,7 +2190,9 @@ function ai_helper.get_attacks(units, cfg)
                         for _,uiw_loc in ipairs(uiw_reach) do
                             -- Unit in the way of the unit in the way
                             local uiw_uiw = wesnoth.units.get(uiw_loc[1], uiw_loc[2])
-                            if (not uiw_uiw) or (not ai_helper.is_visible_unit(viewing_side, uiw_uiw)) then
+                            if (not uiw_uiw)
+                                or ((not ignore_visibility) and (not ai_helper.is_visible_unit(side, uiw_uiw)))
+                            then
                                 add_target = true
                                 break
                             end
diff --git a/data/ai/lua/ca_castle_switch.lua b/data/ai/lua/ca_castle_switch.lua
index cd097d6c74e7..e24ee7fdfa8a 100644
--- a/data/ai/lua/ca_castle_switch.lua
+++ b/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
diff --git a/data/ai/lua/generic_recruit_engine.lua b/data/ai/lua/generic_recruit_engine.lua
index 212f3073766b..a7bc1bba90fa 100644
--- a/data/ai/lua/generic_recruit_engine.lua
+++ b/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]
diff --git a/data/ai/micro_ais/cas/ca_fast_combat.lua b/data/ai/micro_ais/cas/ca_fast_combat.lua
index 2943eacadc55..07ce6ac85ba9 100644
--- a/data/ai/micro_ais/cas/ca_fast_combat.lua
+++ b/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
diff --git a/data/ai/micro_ais/cas/ca_fast_combat_leader.lua b/data/ai/micro_ais/cas/ca_fast_combat_leader.lua
index 0131cb871877..e7959e17cfa1 100644
--- a/data/ai/micro_ais/cas/ca_fast_combat_leader.lua
+++ b/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
diff --git a/data/campaigns/Heir_To_The_Throne/scenarios/17_Scepter_of_Fire.cfg b/data/campaigns/Heir_To_The_Throne/scenarios/17_Scepter_of_Fire.cfg
index 3adba959b7de..d736d9f11755 100644
--- a/data/campaigns/Heir_To_The_Throne/scenarios/17_Scepter_of_Fire.cfg
+++ b/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.",
diff --git a/data/lua/wml-tags.lua b/data/lua/wml-tags.lua
index a87eed89d832..5c4b471b2af4 100644
--- a/data/lua/wml-tags.lua
+++ b/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
diff --git a/data/lua/wml/find_path.lua b/data/lua/wml/find_path.lua
index 30b31bcac468..fd3d586ba0fc 100644
--- a/data/lua/wml/find_path.lua
+++ b/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
 
diff --git a/src/scripting/game_lua_kernel.cpp b/src/scripting/game_lua_kernel.cpp
index 932c1e1e15c9..c649456262bf 100644
--- a/src/scripting/game_lua_kernel.cpp
+++ b/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,7 +1724,6 @@ 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;
@@ -1729,10 +1731,10 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 	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");
@@ -1740,7 +1742,12 @@ int game_lua_kernel::intf_find_path(lua_State *L)
 		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);