How can I improve my follow player script?

[AustnBlox (Austin)]

I have this script which has an npc which follows the player but I feel it needs improving. Please let me know what I can improve in this code:

--// Services //--
local PathfindingService = game:GetService("PathfindingService")
local RunService = game:GetService("RunService")
local ServerStorageService = game:GetService("ServerStorage")
local WorkspaceService = game:GetService("Workspace")
local PlayersService = game:GetService("Players")

--// Player Variables //--
local Character = script.Parent
local HumanoidRootPart = Character.HumanoidRootPart

--// Rig Variables //--
local Rig = ServerStorageService.Rig
local RigHumanoid = Rig.Humanoid
local RigHumanoidRootPart = Rig.HumanoidRootPart

--// Debounces //--
local TeleportingDebounce = false

--// Rig Setup //--
Rig.Parent = WorkspaceService
Rig.PrimaryPart:SetNetworkOwner(nil)

--// Functions //--
local function FollowPlayer()
	local Path = PathfindingService:CreatePath()
	
	local ComputedPath = Path:ComputeAsync(Rig.HumanoidRootPart.Position, HumanoidRootPart.Position - Vector3.new(5, 0, 0))
	
	return Path
end

--// Connections //--
RunService.Stepped:Connect(function()
	FollowPlayer()
	
	local Path = FollowPlayer()
	
	for _, Waypoint in pairs(Path:GetWaypoints()) do
		RigHumanoid:MoveTo(Waypoint.Position)
	end
	
	local Distance = (HumanoidRootPart.Position.Magnitude - RigHumanoidRootPart.Position.Magnitude)
	
	if Distance >= 25 or Distance <= -25 then
		if not TeleportingDebounce then
			TeleportingDebounce = true
			
			RigHumanoidRootPart.CFrame = HumanoidRootPart.CFrame * CFrame.new(-5, 0, 0)
			
			task.wait(2)
			
			TeleportingDebounce = false
		end
	end
end)

[kiloe2]

Looks like solid stuff! Here are some improvements you can make.

The first is to utilize the single responsibility principle in your RunService.Stepped callback function. For functions, the single responsibility principle means that each function should perform just one task. Let’s look at how we can break this up:

In the callback function, it looks like you have the following tasks:

RunService.Stepped:Connect(function()
    -- 1. Create the path for the npc to folow
    -- 2. Iterate over the waypoints and move the NPC to them
    -- 3. Determine whether the NPC can teleport
    -- 4. If so, teleport
end)

This works, but if the funcitonality of this script has to expand, then chaging anything in the future is likely to be a whole mess of re-reading and re-understanding the code. Instead, we can break those tasks off into their own functions, and then run them as necessary in the callback function.
Consider the following:

local function MakePath()
end

local function FollowPath(Path)
end

local function DetermineWhetherCanTeleport()
end

local function Teleport()
end

RunService.Stepped:Connect(function()
    local Path = MakePath()
    FollowPath(Path)
    if DetermineWhetherCanTeleport() then
        Teleport()
    end
end)

This way, whenever you have to change how one task works, you dont have to weed through everything else to do one particular thing.

–

Now, onto performance. Usually, I’m a huge proponent of the saying, “premature optimization is the root of all evil,” but since pathfinding can be computationally expensive, there are some improvements here that I don’t want you to miss.

Firstly, take a look at these few lines:

--// Connections //--
RunService.Stepped:Connect(function()
	FollowPlayer()
	
	local Path = FollowPlayer()

Notice that you’re running FollowPlayer twice every Stepped fire, and the first time you do, the value returned just gets thrown away. Getting rid of that first FollowPlayer call will cut the pathfinding workload in half.

Secondly, you’re pathfinding every RunService.Stepped fire, which means you’re pathfinding every single frame. On a small scale, you might be able to get away with this. If you plan on having more than a handful of NPCs, though, this could cause some lag. Consider whether you need to pathfind every frame. Chances are, you could cut that down to once per half-second, or even once per second, and still achieve the same goal.
If you’ve followed the single responsibility principle suggestion from before, it might look something like this:

local PATHFINDING_COOLDOWN_SECONDS = 1

local LastPathfindTick = 0

local function Pathfind()
   local Path = MakePath()
   FollowPath(Path)
   LastPathfindTick = tick()
end

RunService.Stepped:Connect(function()
   if tick() - LastPathfindTick > PATHFINDING_COOLDOWN_SECONDS then
       Pathfind()
   end

   if DetermineWhetherCanTeleport() then
       Teleport()
   end
end)

–

If you have any questions, please do let me know. Good luck. You’ve got some good stuff here!

[AustnBlox (Austin)]

Thank you so much! This was extremely helpful.