I am only accessing the server side as you know. Therefore, there is less of a reason to use CollectionService and rather to use a single ModuleScript instead? In theory if I am using a ModuleScript and requiring it in every script inside every vehicle model, I’d only be using one script of which I could edit from instead of having separate scripts?
However, I assume that there must be some advantage to using either of these because there must be some performance improvements, if your using the same code all in one place?
Yes, you would only be editing one script, but there’s just the tiny issue of having extra, identical Script instances that require the ModuleScript.
Also, with CollectionService, you could do :AddTag() or :RemoveTag() to dynamically add or remove a vehicle, which is a feature that the ModuleScript method can’t provide easily.
As mentioned in ModuleScripts: “Having multiple copies of a function is disastrous when you need to change that behavior.”
The improvement in manageability is already an advantage, even though there’s little impact in performance.
True, I still have an issue even when I do use a ModuleScript. I tested 32 cars and moved them all at once and they took a while to respawn. So in terms of performance; that hasn’t really been improved at all for in which case, I could still use CollectionService on my current implementation of requiring the ModuleScript into every vehicle model that needs it?
I still feel like using CollectionService would eventually become unnecessarily complicated though, because I don’t necessarily have to have the vehicles already spawned in, especially because I am trying to use so many vehicle models at once when I could simply leave it up to the user to have to spawn them in. I’m only using .Heartbeat for when the vehicles are already spawned in for users to drive in and when they are pushed out of place they would respawn, so maybe I should have it up to the user to spawn the vehicles in, therefore no need for the use of .Heartbeat anymore or trying ways to increase the game’s performance in light of this?
When cars are moved, there’s a cooldown before cars are respawned.
Do you mean that time, or do you mean the car takes a while to load?
You can try debug-printing when vehicles start to load to see if all vehicles start reloading at the same time.
From the respawn code posted before, I see that you created a new clone of the car model.
Wouldn’t it be better to just reposition the old car and remove the car’s AssemblyLinearVelocity?
Also, in the code, I see that you used :SetPrimaryPartCFrame. SetPrimaryPartCFrame is a deprecated function that is superseded by PivotTo, so I suggest using the latter instead.
When new vehicles are added, it would fire the GetInstanceAddedSignal event to set up the vehicle functions, so you wouldn’t need to worry about vehicles being spawned in later.
You could also remove the functions by just calling :RemoveTag on the vehicle model.
If you feel like using .Heartbeat has a significant drawback on the game’s performance, it’s okay to consider letting players spawn in the vehicles themselves.
Like the cooldown takes a while to start if they are all moved at once, resulting in all of them taking longer to respawn back into their original spawning positions. That’s without using CollectionService at this point, of course.
I think it could be a result of the elapsedTime >= 1 condition that forces vehicleCooldown() to run only once per second.
You can remove that throttle condition if you want to immediately detect movement in car positions, and instead use a debounce-like system.
Definitely not a good idea to be making new while true loops 30 times a second, so make sure your code can’t do that.
If you only need one while true loop to be running at a time, you can do something like this:
local connection
local function newConnection()
if connection then connection:Disconnect() connection = nil end
connection = heartbeat:Connect(vehicleCooldown)
end
-- Then for your while loop, do something like this:
if not inCooldown and not cancelCooldown then
connection:Disconnect() -- stops the heartbeat from calling vehicleCooldown() more
while CooldownValue > 0 and respawnInProgress do
-- Inside code
task.wait(1) -- Wait for one second
-- Inside code
end
newConnection() -- allows heartbeat to call vehicleCooldown() more
Also, you have a comment that says
--sets respawning progress to true
But you don’t set that debounce to true. If you set that debounce to true there, that’s an alternative way to stop that loop from being created a million times.
Using heartbeat for every vehicle is fine, the main thing is how expensive the function heartbeat is running is.
While loops are mainly only bad practice when you’re using a zero wait time, but they’re pretty good when you don’t need to do something really fast. Your case is a great example of somewhere a while task.wait(1) loop would be good. (Instead of doing stuff 30 times a second, it’s simplier to do it one time a second if applicable.)
If I were you, I would get a table of all the cars (through collection service, parenting them all to the same folder, etc) then in one single while task.wait(1) loop I would update all of them. I would then store the other states as attributes (cooldown canceled, etc) (or more ideally in a data structure to avoid replication, though it’s really negligible b|c they only update once a second).
.Heartbeat, task.wait(), .Stepped, …, wont cause performance issues, and they should all be about the same. From my time developing, and using the micro profiler, I have noticed that doing lua math is dirt cheap (unless maybe you do a lot of CFrame math), but indexing the properties of an object or using some method isn’t as fast as I initially though.
With this in mind, you could cache the PrimaryPart of every vehicle inside a table, and access them from the table. You might need some logic to detect if the part is destroyed and stuff though.
I would also recommend using Seat:GetPropertyChangedSignal(“Occupant”), and change a local variable in your script as the occupant changes. This would remove the need to check Seat.Occupant constantly
Even so, I doubt you’d get performance issues from that script, unless you have A LOT of vehicles. This is also why I would recommend learning to use the micro profiler, as that would remove the guess work, and instead give clear indications on problematic scripts, helping you optimize what truly matters
How would I identify which of the cars that was looped through was moved though and then start the cooldown for the relevant cars. What might that look like? Just so I can get an idea.
I wouldn’t. If you’re doing it every second you don’t really need to worry about ignoring cars that haven’t moved unless you have like 3,000 cars or something.
What do you mean? Even if I had 30 cars I would still have to loop through all of them to check when one of them has moved, unless I check them all individually?
while task.wait(1) do
for _, car in ipairs(cars) do
task.defer(function()
-- get the car's distance and update the cooldown state
end)
end
end
You don’t need to try to limit a few if statements and a distance check only to cars that moved. The performance is completely negligible if you’re only doing it every 1000 milliseconds. If you’re doing it that sparsely, you could do the distance and update logic on thousands of cars and still not need to worry about it in the slightest (just make sure to remove unused or destroyed cars from the cars list above, or get a new copy of that list each iteration with something like CollectionService:GetTagged("Car")).