Performance Improvements to pathfinding module

Help and Feedback Code Review
#1

I’m creating a pathfinder module and I need some performance optimizations. While it may work, my code could still be improved. Also, this module uses PartCache, for all your quick part-creation needs for toggling the visible waypoints. Please tell me anything you find. Here’s the module:

--!nonstrict


------------------------
-- Services: -----------

local TweenService : TweenService = game:GetService("TweenService")
local PathfindingService : PathfindingService = game:GetService("PathfindingService")

script:WaitForChild("PartCache")
local PartCache = require(script.PartCache)


------------------------
-- Errors: -------------

local argument_Error = "Argument %i missing or nil"
local instance_Error = "%s is not a valid member of %s %q"
local index_nil_Error = "attempt to index nil with '%s'"
local unsupported_value_Error = "type '%s' is unsupported for %s %q"
local character_Error = "%q rigged incorrectly | currentRootPart %q | currentPrimaryPart %q"
local read_only_Error = "can't set value"


------------------------
-- Types: --------------

export type  ReferencePoint = Vector3 | Model | BasePart | Humanoid
export type Waypoints = (...{Position : Vector3, Action : Enum.PathWaypointAction}) -> (...PathWaypoint)

export type AgentParameters = {
	AgentRadius : number,
	AgentHeight : number,
	AgentCanJump : boolean
}

export type PathfinderObject = {
	Name : string,
	Target : ReferencePoint,
	Current : ReferencePoint,
	AgentParameters : AgentParameters,
	SetIndicatorGroup : any,
	Waypoints : Waypoints,
	Completion : number,
	Visible : boolean,

	Path : Path,
	StartDistance : number,
	CurrentWaypointIndex : number,
	WaypointsCache : string,
	Active : boolean,

	Start : any,
	Pause : any,
	Resume : any,
	Stop : any,

	Update : any,
	WalkToWaypoint : any,
	PausedAction : any,
	Action : any,

	Destroy : any
}


------------------------
-- Private: ------------

-- Functions
local function GetAgentParameters(agentParameters : AgentParameters, character : Model, humanoid : Humanoid) : AgentParameters
	if character then
		if (humanoid.RigType == Enum.HumanoidRigType.R6) then
			local head,hrp = character:FindFirstChild("Head"),character:FindFirstChild("HumanoidRootPart")
			local la,ra,leg = character:FindFirstChild("Left Arm"),character:FindFirstChild("Right Arm"),(character:FindFirstChild("Left Leg") or character:FindFirstChild("Right Leg"))

			if (head and hrp) then
				if leg then
					agentParameters.AgentHeight = (head.Size.Y + hrp.Size.Y + leg.Size.Y)
				else
					agentParameters.AgentHeight = (head.Size.Y + hrp.Size.Y)
				end

				if (la and ra) then
					agentParameters.AgentRadius = 0.5 * (hrp.Size.X + la.Size.X + ra.Size.X)
				elseif la then
					agentParameters.AgentRadius = 0.5 * (hrp.Size.X + la.Size.X)
				elseif ra then
					agentParameters.AgentRadius = 0.5 * (hrp.Size.X + ra.Size.X)
				end
			end
		end
	elseif (humanoid.RigType == Enum.HumanoidRigType.R15) and humanoid then
		local head,hrp,hip = character:FindFirstChild("Head"),character:FindFirstChild("HumanoidRootPart"),(humanoid and humanoid.HipHeight)
		local arm,leg = (character:FindFirstChild("LeftUpperArm") or character:FindFirstChild("RightUpperArm")),(character:FindFirstChild("LeftUpperLeg") or character:FindFirstChild("RightUpperLeg"))

		if (head and hrp and hip) then
			agentParameters.AgentHeight = (head.Size.Y + hrp.Size.Y + hip)
			if arm then
				agentParameters.AgentRadius = 0.5 * ((2 * arm.Size.X) + hrp.Size.X)
			elseif leg then
				agentParameters.AgentRadius = 0.5 * ((2 * leg.Size.X) + hrp.Size.X)
			end
		elseif hip then
			local height = humanoid:FindFirstChild("BodyHeightScale")
			local width = humanoid:FindFirstChild("BodyWidthScale")

			if height then
				agentParameters.AgentHeight = (height.Value * 5)
			end
			if width then
				agentParameters.AgentRadius = (width.Value * 5)
			end
		end
	end

	if humanoid then
		if humanoid.UseJumpPower then
			agentParameters.AgentCanJump = (humanoid.JumpPower > 0)
		else
			agentParameters.AgentCanJump = (humanoid.JumpHeight > 0)
		end
	end

	return agentParameters
end

local function UseAgentParameters(current : ReferencePoint) : boolean
	if typeof(current) == "Instance" then
		if current:IsA("Model") then
			return true
		elseif current:IsA("Part") then
			return true
		end
		return current:IsA("Humanoid")
	else
		return false
	end
end

local function getPosition(value : ReferencePoint) : Vector3
	if typeof(value) == "Instance" then
		if value:IsA("Model") then
			return value.PrimaryPart.Position
		elseif value:IsA("Part") then
			return value.Position
		elseif value:IsA("Humanoid") then
			return value.RootPart.Position
		end
	end
	return (value :: Vector3)
end

local CloneTable
CloneTable = function(t) : {}
	local copy = {}
	for key,value in pairs(t) do
		if typeof(value) == "table" then
			copy[key] = CloneTable(value)
		else
			copy[key] = value
		end
	end

	return copy
end

local function len(Dictionary) : number
	local length : number = 0

	for _,__ in pairs(Dictionary) do
		length += 1
	end

	return length
end

local function assertwarn(requirement: boolean, warnMessage: string)
	if not requirement then
		warn(warnMessage)
	end
end

local function emptyFunction()

end

-- Default Properties
local default_AgentParameters : AgentParameters = {AgentRadius = 2, AgentHeight = 5, AgentCanJump = true}
local empty_AgentParameters : AgentParameters = {AgentRadius = 0, AgentHeight = 0, AgentCanJump = true}
local default_Proxy : {} = {}
local default_MetaTable : {__index : any, __newindex : any, __tostring : any} = {
	__index = function(self : PathfinderObject, key : string) : any
		local rawValue = self[default_Proxy][key]

		assert(typeof(rawValue) ~= "nil", instance_Error:format(key,self[default_Proxy]["Name"],script:GetFullName()))

		return rawValue
	end,

	__newindex = function(self : PathfinderObject, key : string, value : any)
		local rawValues = self[default_Proxy]

		assert(typeof(rawValues[key]) ~= "nil", instance_Error:format(key,rawValues["Name"],script:GetFullName()))

		self:Update(key, value)
	end,

	__tostring = function(self : PathfinderObject) : string
		return self[default_Proxy]["Name"]
	end
}

-- Set Values
local set_types : {string} = {"Name","Current","Target","Visible","AgentParameters","Waypoints"}

-- Waypoints Cache and Folders
local PartWaypoint = script:WaitForChild("Default Waypoint")
local WaypointsCacheFolder : Folder = (workspace:FindFirstChild("Waypoints Cache") or Instance.new("Folder"))
local WaypointsCache : {[string]: PartCache.PartCache} = {Default = PartCache.new(PartWaypoint, 400)}


------------------------
-- Setup: --------------

-- PathfinderModule
local PathfinderModule = {}
PathfinderModule.__index = default_MetaTable.__index
PathfinderModule.__newindex = default_MetaTable.__newindex

PathfinderModule.Name = "Pathfinder"
PathfinderModule.Current = Vector3.new(0, 0, 0)
PathfinderModule.Target = Vector3.new(0, 0, 0)
PathfinderModule.AgentParameters = default_AgentParameters
PathfinderModule.Waypoints = {}
PathfinderModule.Completion = 0
PathfinderModule.Visible = false

PathfinderModule.CurrentWaypointIndex = 0
PathfinderModule.WaypointsCache = "Default"
PathfinderModule.PausedAction = emptyFunction
PathfinderModule.Action = emptyFunction
PathfinderModule.Active = false
PathfinderModule.Path = false

-- Waypoints Cache
WaypointsCacheFolder.Name = "Waypoints Cache"
WaypointsCacheFolder.Parent = workspace

WaypointsCacheFolder = WaypointsCacheFolder:FindFirstChild("Default Waypoints") or Instance.new("Folder", WaypointsCacheFolder)
WaypointsCacheFolder.Name = "Default Waypoints"

WaypointsCache["Default"]:SetCacheParent(WaypointsCacheFolder)

WaypointsCacheFolder = nil

PartWaypoint.CFrame = CFrame.new(0, 10e8, 0)
PartWaypoint = nil

-- Other Stuff
if script:FindFirstChild("API") then script:FindFirstChild("API"):Destroy() end -- Destroy the API file while in-game, as it's unneeded


------------------------
-- Constructor: --------

local Class = {}
function Class.new(current : ReferencePoint, target : ReferencePoint, agentParameters : AgentParameters) : PathfinderObject
	-- Pathfinder Object
	local object : PathfinderObject = {
		Name = nil,
		Target = nil,
		Current = nil,
		AgentParameters = nil,
		SetIndicatorGroup = nil,
		Waypoints = nil,
		Completion = nil,
		Visible = nil,

		Path = nil,
		StartDistance = nil,
		CurrentWaypointIndex = nil,
		WaypointsCache = nil,
		Active = nil,

		Start = nil,
		Pause = nil,
		Resume = nil,
		Stop = nil,

		Update = nil,
		WalkToWaypoint = nil,
		PausedAction = nil,
		Action = nil,

		Destroy = nil
	}

	-- Initalize Pathfinder Object
	object[default_Proxy] = CloneTable(PathfinderModule)
	object[default_Proxy].__index = nil
	object[default_Proxy].__newindex = nil

	object[default_Proxy].AgentParameters = agentParameters or default_AgentParameters
	object[default_Proxy].Path = PathfindingService:CreatePath(object[default_Proxy]["AgentParameters"])

	-- Sets Metatable
	setmetatable(object, default_MetaTable)

	-- Calls Update
	object:Update("Current", current)
	object:Update("Target", target)
	object:Update("AgentParameters", agentParameters)

	return object
end

function Class:CreateIndicatorGroup(Name : string, Indicator : BasePart)
	assert(Name, argument_Error:format(1))
	assert(Indicator, argument_Error:format(2))
	
	local Parent : Instance? = WaypointsCache[PathfinderModule.WaypointsCache].CurrentCacheParent.Parent
	
	WaypointsCacheFolder = Parent:FindFirstChild(Name .. " Waypoints") or Instance.new("Folder")
	WaypointsCacheFolder.Name = (Name .. " Waypoints")
	WaypointsCacheFolder.Parent = Parent
	
	WaypointsCache[Name] = PartCache.new(Indicator, 400, Parent)
	Indicator.CFrame = CFrame.new(0, 10e8, 0)
end

function Class:RemoveIndicatorGroup(Name : string)
	assert(Name, argument_Error:format(1))
	
	WaypointsCache[Name]:Dispose()
	WaypointsCache[Name] = nil
end


------------------------
-- Functions: ----------

function PathfinderModule:SetIndicatorGroup(GroupName : string)
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("WalkToWaypoint"))
	
	self[default_Proxy]["WaypointsCache"] = GroupName
end

function PathfinderModule:WalkToWaypoint()
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("WalkToWaypoint"))

	self.Current:MoveTo(self.Waypoints[self.CurrentWaypointIndex])
	self.Current.MoveToFinished:Wait()

	self.CurrentWaypointIndex += 1
end

function PathfinderModule:Start(waypoints : Waypoints)
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Start"))


	self.Completion = (self.CurrentWaypointIndex / #self.Waypoints)

	if self[default_Proxy]["Current"] and self[default_Proxy]["Target"] then
		print("Started")
	end
end

function PathfinderModule:Pause(pauseTime : number)
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Pause"))

	self.Active = false
	self.PausedAction = self.Action
	self.Action = emptyFunction

	if (pauseTime > 0) or self.Active then
		delay(pauseTime, function()
			self:Resume()
		end)
	end
end

function PathfinderModule:Resume()
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Resume"))

	self.Active = true
	self.Action = self.PausedAction
end

function PathfinderModule:Stop()
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Stop"))

	if (#self.Waypoints > 0) or self.Active then
		self.Active = false
		self.CurrentWaypointIndex = 1
	end
end

function PathfinderModule:Destroy()
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Destroy"))
	
	for key,_ in pairs(self) do
		self[key] = nil
	end
end

function PathfinderModule:Update(key : string, value : any)
	assert(getmetatable(self) == default_MetaTable, index_nil_Error:format("Update"))

	local Current = self.Current -- The current PVInstance or the current Vector3 position
	local Target = self.Target -- The current PVInstance or the current Vector3 position

	local Path : Path = self.Path -- The path from Current to Target

	-- Sets value to the correct value
	if (not table.find(set_types, key)) then
		error(read_only_Error)
	elseif typeof(value) == "Instance" then
		if value:IsA("Model") then
			local humanoid : Humanoid? = value:FindFirstChildOfClass("Humanoid")
			if humanoid then
				assertwarn(humanoid.RootPart ~= value.PrimaryPart, character_Error:format(value:GetFullName(),humanoid.RootPart:GetFullName(),value.PrimaryPart:GetFullName()))

				self[default_Proxy][key] = humanoid
			else
				self[default_Proxy][key] = (value :: Model)
			end
		elseif value:IsA("Part") then
			local checkParent : Instance
			repeat
				checkParent = value.Parent
				print(checkParent)
			until (checkParent.Parent == workspace)

			local humanoid : Humanoid? = checkParent:FindFirstChildOfClass("Humanoid")
			if humanoid then
				assert(humanoid.RootPart ~= value:GetRootPart(), character_Error:format(value:GetFullName(),humanoid.RootPart:GetFullName(),value.PrimaryPart:GetFullName()))

				self[default_Proxy][key] = humanoid
			else
				self[default_Proxy][key] = (value:GetRootPart() :: BasePart)
			end
		end
	elseif typeof(value) == "Vector3" or typeof(value) == "boolean" or typeof(value) == "string" then
		self[default_Proxy][key] = (value :: Vector3 | boolean | string)
	elseif (key == "AgentParameters") then
		self[default_Proxy][key] = (value :: AgentParameters)
	else
		error(unsupported_value_Error:format(typeof(value),self.Name,self.Name .. "." .. key))
	end

	-- Re-Defines Current and Target
	Current = getPosition(self.Current)
	Target = getPosition(self.Target)

	-- Configures the self.Path and self.Waypoints
	if (Current ~= Target) then
		-- Sets self.AgentParameters to the correct value
		if typeof(self.Current) == "Vector3" then
			self[default_Proxy]["AgentParameters"] = empty_AgentParameters
		end

		-- Sets self.Waypoints to the correct values
		spawn(function()
			Path:ComputeAsync(Current, Target)
			self[default_Proxy]["Waypoints"] = Path:GetWaypoints()

			local Cache : PartCache.PartCache = WaypointsCache[self[default_Proxy]["WaypointsCache"]]

			local Waypoints = self[default_Proxy]["Waypoints"]
			local VisualWaypoints = Cache.InUse

			for i=((#VisualWaypoints > #Waypoints) and #VisualWaypoints) or #Waypoints,1,-1 do
				if Waypoints[i] then
					local part = VisualWaypoints[i] or Cache:GetPart()
					
					part.CFrame = CFrame.lookAt(Waypoints[i]["Position"],(Waypoints[i+1] or Waypoints[1])["Position"])
				else
					Cache:ReturnPart(VisualWaypoints[i])
				end
			end
			
			repeat
				local left : number = (#Waypoints - #Cache.InUse)
				print(left)
				for i=(#Cache.InUse+1),#Waypoints do
					if Waypoints[i] then
						local part = VisualWaypoints[i] or Cache:GetPart()
						part.CFrame = CFrame.new(Waypoints[i]["Position"])
					else
						Cache:ReturnPart(VisualWaypoints[i])
					end
				end
			until (#Cache.InUse >= #Waypoints)
		end)
	end
end


------------------------
-- Return: -------------

return Class
1 Like
#2

Could you point out which specific parts of this code you’re looking for performance improvements on? There’s a whole lot of code rather than just specific parts that you feel may need optimisation. Things like your CopyTable function (why isn’t the function declaration syntax consistent with your other functions?) don’t need any comments, for example.

#3

This I believe doesn’t help on performance, but you can just have one _ for the loop arguments.

#4

Sorry! I forgot about that. One of the parts I wanted attention to be put on are the loops I used in the code. However, the main focus I want is on the metatables and how I implemented them. This was my first time creating a Object-Oriented module that actually had a purpose and wasn’t just practice with a few lines of code, so while I understand some of the concepts of OOP, I don’t know if I implemented them correctly or efficiently.

#5

What do you mean by this? Do you mean instead of have it as _,__ to have it instead as something more like a,b? Is it beacause I use the same character twice?

#6

No! I mean don’t have the second argument, you don’t need to declare it.

You can have loops like this:

for index in pairs(table_idk) do
     table_idk[index]= nil;
end

In this case I didn’t need a value, so I can’t just not declare it. Meaning you can literally just use ONE _ instead of two.

#7

Oh. Thanks, I didn’t know that. I’ve edited my script a bit since, so I’ll edit the original post.