Scripting Helpers is winding down operations and is now read-only. More info→
Ad
Log in to vote
0

Path finding script is slow?

Asked by 5 years ago
Edited 5 years ago

Idea


I made a path finding killer.It worked.But a little slower. Can someone solve it for me? Please.

function PFS(TargetPart)
    local TargetPos = TargetPart.Position--Target's position in that time.
    -------------
    PFSing = true
    local Break_For_Loop = false
    -------------
    local Path = PathfindingService:CreatePath()--Path in that time

    Path:ComputeAsync(AI_Position.Position,TargetPos)--Create old path
    local waypoints = Path:GetWaypoints()
    for Number,point in pairs(waypoints) do
    if Break_For_Loop == false then
    local NowPath = PathfindingService:CreatePath()--The path now
        NowPath:ComputeAsync(AI_Position.Position,TargetPart.Position)
        local nowPathWaypoints = NowPath:GetWaypoints()
        if (#nowPathWaypoints - #waypoints >= Chance) or (#waypoints - #nowPathWaypoints >= Chance) then
            humanoid:MoveTo(point.Position)
            humanoid.MoveToFinished:Wait()
                        print("Too different!")
            Break_For_Loop = true

        else
                        humanoid:MoveTo(point.Position)

            wait(0.1)

        end
    end
    end
    PFSing = false
end

1 answer

Log in to vote
0
Answered by
BlueTaslem 18071 Moderation Voter Administrator Community Moderator Super Administrator
5 years ago

First, let's clean up your code.

You should indent your code consistently so that the block structure is clear:

function PFS(TargetPart)
    -- Target's position in that time.
    local TargetPos = TargetPart.Position


    PFSing = true
    local Break_For_Loop = false

    -- Path in that time
    local Path = PathfindingService:CreatePath()

    -- Create old path
    Path:ComputeAsync(AI_Position.Position, TargetPos)


    local waypoints = Path:GetWaypoints()
    for Number, point in pairs(waypoints) do
        if Break_For_Loop == false then
            -- The path now
            local NowPath = PathfindingService:CreatePath()
            NowPath:ComputeAsync(AI_Position.Position, TargetPart.Position)
            local nowPathWaypoints = NowPath:GetWaypoints()
            if (#nowPathWaypoints - #waypoints >= Chance) or (#waypoints - #nowPathWaypoints >= Chance) then
                humanoid:MoveTo(point.Position)
                humanoid.MoveToFinished:Wait()
                print("Too different!")
                Break_For_Loop = true
            else
                humanoid:MoveTo(point.Position)

                wait(0.1)
            end
        end
    end
    PFSing = false
end

The Break_For_Loop variable can be removed. Instead, just break out of the loop.

Number isn't used. Replace it with _ so that it's not so distracting.

Using math.abs will make your if statement in the loop much clearer. Chance is definitely a terrible name for what it is being used for. You should pick a name that makes sense.

Comments like -- The path now are redundant when you have a name like NowPath. Just delete them. On the other hand, waypoints could be named originalWaypoints to make it clearer that that's what it was.

Having global variables is generally messy. Since this function doesn't use PFSing, I'm guessing it doesn't belong in this function, but rather should be managed by the caller of this function.

Both branches of your if begin with the same statement, so you can move it out instead of repeating yourself.

This results in the following much more compact and easier to read code:

function PFS(TargetPart)
    local Path = PathfindingService:CreatePath()
    Path:ComputeAsync(AI_Position.Position, TargetPart.Position)
    local originalWaypoints = Path:GetWaypoints()
    for _, point in pairs(originalWaypoints) do
        local NowPath = PathfindingService:CreatePath()
        NowPath:ComputeAsync(AI_Position.Position, TargetPart.Position)
        local nowPathWaypoints = NowPath:GetWaypoints()
        humanoid:MoveTo(point.Position)
        if math.abs(#nowPathWaypoints - #originalWaypoints) >= Chance then
            humanoid.MoveToFinished:Wait()
            print("Too different!")
            break
        else
            wait(0.1)
        end
    end
end

There are a number of problems here.

First, 0.1 is not going to be enough time in general to move to the next spot, since consecutive waypoints can be spread quite far apart.

:ComputeAsync is quite expensive. However, you're not even bothering to use the result. Paths already have a Blocked event that tells you whether or not the path is no longer usable; comparing the number of waypaths in two paths doesn't mean anything -- the paths could be almost exactly the same but have a different number of waypoints or be totally different and have the same number of waypoints.

And unless you have a rapidly changing map (or a rapidly moving AND nearby target), your NPCs are probably fine following any un-blocked path at least for a while.

0
OMG THANKS YOU! DERP9487 41 — 5y
Ad

Answer this question