Wondering if my pathfinding method is inefficient or needs improvement

Help and Feedback Code Review
, ,
#1

So I had recently made this pathfinding script after trying for a while to get it to a state where I felt It felt good enough for me to use for my NPCs, but from what I’ve seen of pathfinding it looks nothing compared to how I got it to work. It makes me think that this code would probably be inefficient and prone to being broken or probably too costly for the game to run constantly. Would this code be fine or would it need some reworking?

--// Variables \\--

local pathfindingService = game:GetService("PathfindingService")

local character = script.Parent
local humanoid = character:FindFirstChild("Humanoid")
local humRootPart = character:FindFirstChild("HumanoidRootPart")

local path = pathfindingService:CreatePath({
	AgentRadius = 3,
	AgentHeight = 6,
	AgentCanJump = true,
	AgentCanClimb = true,
	Costs = {
		Water = math.huge
	}
})
local endPos = workspace.EndPoint

local blockedConnection
local reachedConnection

local waypoints = {}
local waypointIndex = 1

humRootPart:SetNetworkOwner(nil)

--// Functions \\--

local function pathfind(goal)

	local success, failed = pcall(function()
		path:ComputeAsync(humRootPart.Position, goal.Position)
	end)

	if success and path.Status == Enum.PathStatus.Success then
		print("Path Found!")

		waypoints = path:GetWaypoints()
		
		if #waypoints > 0 then
			if  #waypoints >= 3 and waypoints[3].Action == Enum.PathWaypointAction.Jump then
				if humanoid.FloorMaterial ~= Enum.Material.Air then

					humanoid:ChangeState(Enum.HumanoidStateType.Jumping)
					humanoid:MoveTo(waypoints[3].Position)
					humanoid.MoveToFinished:Wait()
				end
			elseif #waypoints >= 3 and waypoints[3].Action == Enum.PathWaypointAction.Custom then
				humanoid:MoveTo(waypoints[3].Position)
			else
				waypointIndex = 2
				humanoid:MoveTo(waypoints[waypointIndex].Position)
			end
			print(waypoints[3])
		end
	else
		warn("No path found! ".. tostring(failed))
	end
end

--// Loops \\--

task.wait(4)

while task.wait() do
	pathfind(endPos)
end
4 Likes
#2

Here is my main takeaways:

local success, failed = pcall(function()
		path:ComputeAsync(humRootPart.Position, goal.Position)
	end)

This line is fine, however, you might just want to wrap the entire pathfinding function into a pcall() as pathfinding errors are very uncommon unless you correctly check for all possible errors.

if #waypoints > 0 then
My preference would be if #waypoints <= 0 then return end

first off, you need to loop through your waypoints to even have a purpose for them. Also, your methods are very strange.

for i, waypoint in waypoints do
       if waypoint.Action == Enum.PathWaypointAction.Jump and not jumpcooldown then
          humanoid:ChangeState(Enum.HumanoidStateType.Jumping)
          jumpcooldown = true task.delay(.5, function() jumpcooldown = false end)
       end
       humanoid:MoveTo(waypoint.Position)
end

Additionally, I added a jumping cooldown. I’m not sure why you specifically check the third waypoint, but obviously this wont always work because the third waypoint could be right next to the player or several studs away.

Finally, if you are looking to further improve general pathfinding, I recommend you raycast from the Agent to the Target and check if they are in line of sight. If so, just use MoveTo() on their player, as waypoints can be iffy sometimes.

1 Like
#3

The reason most of the code I wrote is how it is, was because I had finally gotten it to work the way I wanted to as well. I had already spent ages on trying to get the pathfinding to work and I felt myself starting to become a bit burnt out whilst trying to work on it. I couldn’t find many sources online on how I should do things so it was pretty much me on my own, and I practically tried anything to get it working no matter how strange the code may seem as by how you pointed out. Thank you for the feedback and suggestions, I’ll try my best to change around the script to be a bit better than it is now.