Change mob spawning randomization from polar coordinates to spherical coordinates, move position validation code from find_spawning_position to get_next_mob_spawn_pos, minimize code remaining in find_spawning_position

mods/ENTITIES/mcl_mobs/spawning.lua

@@ -603,17 +603,47 @@ local function get_next_mob_spawn_pos(pos)
 	-- TODO We should consider spawning something a little further away sporadically.
 	-- It would be good for sky farms and variance, rather than all being on the 24 - 32 block away radius
 	local distance = math_random(MOB_SPAWN_ZONE_INNER, MOB_SPAWN_ZONE_MIDDLE)
-	local angle = math_random() * two_pi
 
 	-- TODO Floor xoff and zoff and add 0.5 so it tries to spawn in the middle of the square. Less failed attempts.
-	local xoff = math_round(distance * math_cos(angle))
-	local zoff = math_round(distance * math_sin(angle))
-	return vector.offset(pos, xoff, 0, zoff)
-end
+	-- Use spherical coordinates https://en.wikipedia.org/wiki/Spherical_coordinate_system#Cartesian_coordinates
+	local theta = math_random() * two_pi
+	local phi = math_random() * two_pi
+	local xoff = math_round(distance * math_sin(theta) * math_cos(phi))
+	local yoff = math_round(distance * math_cos(theta))
+	local zoff = math_round(distance * math_sin(theta) * math_sin(phi))
+	local goal_pos = vector.offset(pos, xoff, yoff, zoff)
 
-local function decypher_limits(posy)
-	posy = math_floor(posy)
-	return posy - MOB_SPAWN_ZONE_MIDDLE, posy + MOB_SPAWN_ZONE_MIDDLE
+	if not ( math.abs(goal_pos.x) <= SPAWN_MAPGEN_LIMIT and math.abs(pos.y) <= SPAWN_MAPGEN_LIMIT and math.abs(goal_pos.z) <= SPAWN_MAPGEN_LIMIT ) then
+		mcl_log("Pos outside mapgen limits: " .. minetest.pos_to_string(goal_pos))
+		return nil
+	end
+
+	-- Ask engine for valid spawn locations
+	local spawning_position_list = find_nodes_in_area_under_air(
+			{x = goal_pos.x, y = math_round(pos.y) - MOB_SPAWN_ZONE_MIDDLE, z = goal_pos.z},
+			{x = goal_pos.x, y = math_round(pos.y) + MOB_SPAWN_ZONE_MIDDLE, z = goal_pos.z},
+			{"group:solid", "group:water", "group:lava"}
+	)
+
+	-- Select only the locations at a valid distance
+	local valid_positions = {}
+	for _,check_pos in ipairs(spawning_position_list) do
+		local dist = vector.distance(pos, check_pos)
+		if dist >= MOB_SPAWN_ZONE_INNER and dist <= MOB_SPAWN_ZONE_OUTER then
+			valid_positions[#valid_positions + 1] = check_pos
+		end
+	end
+	spawning_position_list = valid_positions
+
+	-- No valid locations, failed to find a position
+	if #spawning_position_list == 0 then
+		mcl_log("Spawning position isn't good. Do not spawn: " .. minetest.pos_to_string(goal_pos))
+		return nil
+	end
+
+	-- Pick a random valid location
+	mcl_log("Spawning positions available: " .. minetest.pos_to_string(goal_pos))
+	return spawning_position_list[math_random(1, #spawning_position_list)]
 end
 
 --a simple helper function for mob_spawn
@@ -938,42 +968,16 @@ if mobs_spawn then
 
 	local function find_spawning_position(pos, max_times)
 		local spawning_position
-
-		local max_loops = 1
-		if max_times then max_loops = max_times end
-
-		local y_min, y_max = decypher_limits(pos.y)
+		local max_loops = max_times or 1
 
 		--mcl_log("mapgen_limit: " .. SPAWN_MAPGEN_LIMIT)
-		local i = 0
-		repeat
-			local goal_pos = get_next_mob_spawn_pos(pos)
+		while max_loops > 0 do
+			local spawning_position = get_next_mob_spawn_pos(pos)
+			if spawning_position then return spawning_position
+			max_loops = max_loops - 1
 
-			if math.abs(goal_pos.x) <= SPAWN_MAPGEN_LIMIT and math.abs(pos.y) <= SPAWN_MAPGEN_LIMIT and math.abs(goal_pos.z) <= SPAWN_MAPGEN_LIMIT then
-				local spawning_position_list = find_nodes_in_area_under_air(
-						{x = goal_pos.x, y = y_min, z = goal_pos.z},
-						{x = goal_pos.x, y = y_max, z = goal_pos.z},
-						{"group:solid", "group:water", "group:lava"}
-				)
-				if #spawning_position_list > 0 then
-					mcl_log("Spawning positions available: " .. minetest.pos_to_string(goal_pos))
-					spawning_position = spawning_position_list[math_random(1, #spawning_position_list)]
-				else
-					mcl_log("Spawning position isn't good. Do not spawn: " .. minetest.pos_to_string(goal_pos))
 		end
-
-			else
-				mcl_log("Pos outside mapgen limits: " .. minetest.pos_to_string(goal_pos))
-			end
-
-
-			i = i + 1
-			if i >= max_loops then
-				mcl_log("Cancel finding spawn positions at: " .. max_loops)
-				break
-			end
-		until spawning_position
-		return spawning_position
+		return nil
 	end
 
 	local cumulative_chance = nil