First off, I want to commend you on not shying away from long variables names. Lots of people like to abbreviate for convenience and just make it way more annoying to read. When it comes to projects that will last more then a week, readability >>> slight convenience. Personally, I also like to apply this to for loops as well, especially if they are nested or use both the key and the value of the table. In this case, you could change
for i, v in pairs(Waypoints:GetChildren()) do
into
for index, waypoint in pairs(Waypoints:GetChildren()) do
but it really doesn’t tend to matter to much.
Here’s some real improvements I can suggest:
1) Sort the OrderedWaypoints table after you finish populating it.
Right now you use the table.sort function every time the for loop goes over a child in Waypoints. And you do so whether you add something to the table or not! Move that table.sort to be right after for loop so it only runs once.
2) Minor: Utilize endWaypoint
From what I can tell you don’t even use ‘endWaypoint’ after defining it. However, if you do want to use it in your code then you can simplify v.Name == "RailWaypointEnd" to v == endWaypoint
3) Use more variables
In your touched event you end up repeating the same long strings of code rather then just storing that information once in variables. You can simplify this code as well as make the code much cleaner/easier like so:
if hit.Parent:FindFirstChildOfClass("Humanoid") then
speed = hit.Parent:FindFirstChildOfClass("Humanoid").WalkSpeed
local weld = Instance.new("Weld", hit.Parent.PrimaryPart)
weld.Name = "RailWeld"
weld.Part0 = Railpart
weld.Part1 = hit.Parent.PrimaryPart
to:
local character = hit.Parent
local humanoid = character:FindFirstChildOfClass("Humanoid")
if humanoid then
speed = humanoid.WalkSpeed
local weld = Instance.new("Weld", character.PrimaryPart)
weld.Name = "RailWeld"
weld.Part0 = Railpart
weld.Part1 = character.PrimaryPart
4) Add a debounce
The touched event can trigger more than once! Infact it pretty much always triggers at least a few times with what seems like a single touch. To prevent any unseen funky business as well as handle the chaos that is multiplayer games with more then 1 person running about, you can make it so this code can only run if it’s not already running!
local speed = 0
local isRunning = false
Railpart.Touched:Connect(function(hit)
if not isRunning then
isRunning = true
local character = hit.Parent
local humanoid = character:FindFirstChildOfClass("Humanoid")
if humanoid then
Don’t forget to set isRunning back to false when the zipline is done!
5) Properly incorporate travel time
Though your code seems to properly lerp the railcart, it’s not doing so at the designated speed. If you want travelTime to accurately lerp the Railpart to the next waypoint in the given amount of time, you need to pay more attention to task.wait(). The task.wait() is what is giving your lerp a sense of time rather then just instantly running everything so fast you can barely notice. Since you aren’t using special events like Heartbeat or RenderStepped to run code at each frame, it’s natural that you would want the wait time to be as short as possible for smoother movement in which case leaving task.wait() empty is a good choice. We still need to know exactly how long we are waiting to get an accurate travel time and while an empty wait will generally wait for about 0.03 seconds it can vary. Luckily, the wait function actually returns how long it truly waits for so we can just do:
local waitTime = task.wait()
Then we can use that to find out just how far we need to lerp our part along the path to stay on time. Here’s some code for that:
local waitTime = task.wait()
local lerpIncrement = waitTime / travelTime
local lerpValue = previousLerpValue + lerpIncrement
How would you properly implement that though? Afterall, if we are centering this around the task.wait() then the number of iterations is not set in stone (that tells us we don’t want to use a for loop anymore). Here’s how you could do it with a repeat loop:
local travelTime = _
local alpha = 0
repeat
--Do the math
local waitTime = task.wait()
local increment = waitTime / travelTime
alpha = alpha + increment
--Make sure we don't go too far!
if alpha > 1 then
alpha = 1
end
--Time to lerp!
Railpart.CFrame = startCFrame:Lerp(v.CFrame, alpha)
until alpha == 1
You could also do this with a while loop however one of the very few times you would use a repeat loop over a while loop is when the first iteration should happen no matter what.