You receive that warning if Path:Stop() is called when Path.Status is in idle state. If you want to properly handle this, just add an if statement like this:
if Path.Status == SimplePath.StatusType.Active then Path:Stop() end
1 Like
Sorry about that, the bug is now fixed.
1 Like
firstly thanks for fixing the minor glitch before
I have a problem I’m really hoping you can help with.
This problem is present in SimplePath but not subject to it.
I have a setup that looks something like this:
agent.Humanoid.Died:Connect(function()
path:Destroy()
end)
local function pathfind()
path:Run()
end
path.Error:Connect(pathfind)
path.WaypointReached:Connect(pathfind)
path.Blocked:Connect(pathfind)
path.Reached:Connect(pathfind)
pathfind()
As you can see when the agent dies the path gets destroyed - disconnecting the signals as not to cause memory leaks.
The issue is is that SimplePath has yielding code that could run after Path:Destroy(), it’s a race condition bug that looks something like this:
- Path:Run() gets called
- Path:Run() calls ComputeAsync (a yielding function)
- Path:Destroy() gets called (a non yielding function)
- ComputeAsync finshes yielding
- Any code after ComputeAsync is running when the Path object is “dead”
- The output fills up with a slew of “this is not a valid member” errors
In this instance it’s not a big deal, I can mask the errors by wrapping Path:Run() in a pcall. But this is an underlying problem I have been trying to solve.
My question basically is “How to end a program with yielding code”
I made a thread with this question but I didn’t get meaningful replies (or I didn’t understand them, but I think it’s the latter)
Here’s a little test file to further demonstrate
pathfinding_example.rbxl (53.6 KB)
If you know an answer please let me know this has been bothering me for a while now, and I don’t want to have to put flag checks in my code every time it yields.
Well, let’s take a look. I made a simple script to demonstrate how you can go about doing this.
local car = {}
car.__index = car
function car.new()
return setmetatable({
_speed = 0;
}, car)
end
function car:Drive()
self._speed += 100
task.wait(3)
--If car was destroyed during the yield, this part will error
self._speed -= 100
end
function car:Destroy()
self._speed = nil
setmetatable(self, nil)
end
local car = car.new()
task.spawn(car.Drive, car)
car:Destroy()
Here, the car.Drive method yields and executes some code after the yield as well. But if car was destroyed during the yield, self._speed would not exist so this will error. So the most obvious fix here would be to make sure car still exists after the yield. For example, you can add something like this:
function car:Drive()
self._speed += 100
task.wait(3)
--Checks if self._speed still exists
--If car is destroyed, self is just an empty table
if not self._speed then return end
self._speed -= 100
end
In most cases, you will have lots of code after a yield and it won’t be really efficient to add an if statement for every value you have inside of your object. If you take a look at the car.Destroy method, it overrides the object’s metatable to nil. We can use this to our advantage to easily check if the object was destroyed or not. For example, in the car.Drive, we can change it to:
function car:Drive()
self._speed += 100
task.wait(5)
--Checks if metatable still exists
if not getmetatable(self) then return end
self._speed -= 100
end
As you can see above, to make sure your code runs cleanly, all you have to do is perform checks to make sure a value exists after a yield. In this case, if we want to make sure the car object still exists after a yield, just check if the metatable still exists.
So essentially you’re saying check a flag (or in your case metatable) after every time the code yields. I was hoping there was some kind of “set once and forget” way of doing it because I can have a lot of things that yield and it feels janky having to put a check by every yield and inside every loop.
For your case even if it does error it’s not that big of a deal since the object is destroyed and it’s the end of the thread.
Where I am worried about is in cases like a RoundService where when I call Start it can be changing upvalues so when I call End and then Start again I can have 2 instance of Start changing the same values which can break completely break the game. Even in a case like yours I might want to change some values on Destroy, like maybe move the position or play animation and if something in the main code runs over that that will also break.
I thought of some things like any code that runs after a yield gets put in an event and then after every yield it fires the next event, then on destroy it disconnects all the events. But that seems very hacky and if I’m going to all that effort I may as do your method and do a simple check.
I feel like there has to be a definitive answer because millions of programs have a stop button so it has to somehow let yielding code know not to run.
I guess I’ll use checks after every time the code yields and not worry about it, I think I’m overthinking it because I feel like I’m an intermediate scripter and multiple threads and race conditions get me insecure about my code even though it’s probably fine.
Thank you again for your help!
I was working on something like this recently. What I wanted to achieve was to override a previous loop when the function is called, something like this:
local function makeLoop(i)
for i = 1, i do
print(i)
task.wait(1)
end
end
--Make this line override the previous running loop
task.defer(makeLoop, 20)
makeLoop(50)
With the current code above, it will end up making both loops run in parallel. A good technique might be to use BindableEvents. Whenever you call the function again, fire this bindable event. Since the connection for this bindable is in the previous loop, you can disconnect it as soon as it fires. Then in the loop, after each yield, just check if the connection exists before continuing. For example, in code, it would look something like this:
local bindable = Instance.new("BindableEvent")
local function makeLoop(i)
--Fire the bindable at the beginning of each call to cancel previous loop
bindable:Fire()
local connection
--Create a new connection whenever function is called
connection = bindable.Event:Connect(function()
connection = connection:Disconnect()
end)
for i = 1, i do
print(i)
task.wait(1)
--After each yield, check if connection still exists
if not connection then return end
end
--Disconnect connection after loop ends
if connection then connection:Disconnect() end
end
--This line will now override the previous loop
task.defer(makeLoop, 20)
makeLoop(50)
Other than this, to change certain values after a yield as you mentioned, things like position and animation, store the necessary values in a local variable within the scope of the function, even if, for example, the metatable doesn’t exist anymore, the reference to those object still exists, so the objects are not destroyed yet.
1 Like
Thanks, I didn’t even if think of that! I was using an IsRound boolean to check, but now that I think of it that same variable will be set to true when the next round starts so yielding code from the first round can get right through that check! This is the kind of race condition bug I’d spend hours trying to track down.
Checks must still be done after each yield but it’s better than nothing.
v2.2.1 (2022-02-28)
- Fixed
Path:Destroy()- Properly handles the deletion of visual waypoints
- Automatically stops further execution when destroyed
- Updated reference link
2 Likes
thanks for your continued work on this! So I’ve been using simple path to get two teams of bots navigating around my maps to different objectives successfully. Right now they can grab a teams flag and capture it, basically on an attack loop. It’s all managed in a central script, and there can be up to 20 bots at any given time. I haven’t added the bots attacking aspect yet.
What I’m hoping to achieve is when the bots get within x range of an enemy team bot, they will completely stop, attack, and not resume their path until there’s no additional targets within range. I’m trying to think of the most efficient way to accomplish this in code, but it seems like I need to run a conditional check on path:WaypointReached, and calculate the nearest visible enemy for each bot every single time they reach a waypoint…which might get fairly intensive. if they are within the attack range, then trigger a path:Stop and run the attack logic.
It seems like this will be repeating a lot of calculations each waypoint, but I’m not sure how I could lessen it. Maybe something to do with keeping a master targets table handy for each bot to iterate through, but not sure. I appreciate any thoughts or ideas!
Sorry for the late reply. I honestly don’t think it will be intensive in this context. I think you should stick to performing the calculation results every time WaypointReached is fired. It is a direct approach that produces effective results.
Should I create a SimplePath Discord for community help/support?
- Yes
- No
0 voters
Very useful module. I didn’t really know how pathfinding worked and/or why my AI kept stuttering/falling/dying/becoming dumb, but after using this it became SO much easier to make AI. 10/10 would recommend
1 Like
im having a problem where the AI will randomly turn back for a second, and randomly jump for no reason while chasing the player. (THIS ONLY HAPPENS IF THE AI CHASES A PLAYER WITHOUT REACHING THE WAYPOINT THEY WERE GOING TO BEFORE CHASING)
code:
local SimplePath = require(game.ReplicatedStorage.SimplePath)
local runService = game:GetService('RunService')
local Dummy = script.Parent
local state = "walking"
local chasingPlayer
local playerAvailableForCatching = function(player)
--[[
if player.Character and player.Character:FindFirstChild("Head") and player.Character.Humanoid.Health > 0 then
if player.Character:FindFirstChildWhichIsA("Tool") then return true end
local isSafe = false
for _, safe in pairs(workspace.SafeSpots:GetChildren()) do
if isTouching(player.Character.Head.Position, safe) then
if sawPlayersHeadInBush[player] == safe then continue end
isSafe = true
break
end
end
if isSafe then return end
return true
end
]]
return player.Character and player.Character:FindFirstChild("Head") and player.Character.Humanoid.Health > 0
end
local function canSee(player, rayLength)
local dotresult = Dummy.Head.CFrame.LookVector:Dot(CFrame.new(Dummy.Head.Position, player.Character.Head.Position).LookVector)
if dotresult <= -0.3 then return end
local params = RaycastParams.new()
params.FilterDescendantsInstances = {Dummy}
params.FilterType = Enum.RaycastFilterType.Blacklist
local maxDistance = 50
local distance = (Dummy.HumanoidRootPart.Position - player.Character.HumanoidRootPart.Position).Magnitude
local ray = workspace:Raycast(Dummy.Head.Position, CFrame.new(Dummy.Head.Position, player.Character.Head.Position).LookVector * (rayLength or 150), params)
if ray and ray.Instance then
if ray.Instance:IsDescendantOf(player.Character) then
if distance < maxDistance then
return true
else
return false
end
end
end
return false
--return true
end
local dummypath = SimplePath.new(Dummy, {
AgentRadius = 2,
AgentHeight = 5,
})
local lastTargetPoint
local chase = function(player)
state = "chase"
while playerAvailableForCatching(player) do
dummypath:Run(player.Character.Head.Position)
task.wait()
end
dummypath:Run(player.Character.Torso.Position)
dummypath._events.Reached.Event:Wait()
chasingPlayer = nil
state = "walking"
end
local function chaseIfCanSee()
if state == "chase" then return end
for _, player in pairs(game.Players:GetPlayers()) do
if playerAvailableForCatching(player) and canSee(player) then
chase(player)
end
end
end
task.spawn(function()
while task.wait(.2) do
chaseIfCanSee()
end
end)
local function runpath(path)
for _, part in pairs(path) do
lastTargetPoint = part.Position
while true do
if state == "walking" then
while (Dummy.PrimaryPart.Position * Vector3.new(1, 0, 1) - part.Position * Vector3.new(1, 0, 1)).Magnitude > 5 do
dummypath:Run(part.Position)
task.wait(.2)
end
if state == "walking" then break end
end
task.wait()
end
print("path advance")
end
end
local function getpath(rev)
local total = #workspace.Path:GetChildren()
local l = {}
for i = 1, total do
if rev then
i = total - i
end
table.insert(l, workspace.Path:FindFirstChild(i))
end
return l
end
for _, part in pairs(Dummy:GetDescendants()) do
if part:IsA("BasePart") then
part.Touched:Connect(function(part)
local char = part.Parent
local humanoid = char:FindFirstChildWhichIsA("Humanoid")
if humanoid then
humanoid.Health = 0
end
end)
end
end
--runService.Heartbeat:Connect(function()
while Dummy.Parent do
runpath(getpath())
runpath(getpath(true))
task.wait()
end
--end)
Are you using the latest version of SimplePath?
now im using it and the same problem still occurs. the NPC is still trying to get to the Waypoints and also trying to get to the Player at the same time
Im not completely sure what you mean by trying to get to the Player and waypoints at the same time. Pathfinding is done by walking from one waypoint to the next, if the NPC is moving towards the player, it moves by walking to each of the waypoints.
alright so for example, the NPC is walking to random waypoints, and then a Player goes near the NPC while the NPC is going towards a waypoint. this is where the bug occurs: the NPC is trying to go to that waypoint but since a player is near its trying to go to the player AND the waypoint which causes it to keep turning back and forth
as you can see in the video, the npc is trying to get to a waypoint then i cut it off, so now its going for me AND the waypoint, which is why it keeps turning
(i posted the script im using in the post above)
2 Likes
I don’t think this is a bug with SimplePath. Looking at the behaviour in the video, Path:Run() is continuously being called for both the player and the other part simultaneously. I’ve glossed over your script and from what I can tell, the code that does pathfinding to the player is independent of the code that does pathfinding to the parts and this is probably whats causing that.
1 Like
Hmm why not make it so that it checks if the player is within a certain range and pathfind to them and use a bool to make it so that the NPC doesnt even think of going back to the part. But once the player is out of range, it then switches the bool to false and goes back to any nearby station and goes to random one until it then finds the player again.