I’ve recently made an intermission system, nothing that big. I just want to know if my code is okay or it needs to be optimized, I feel as though I could probably write it better or shorter.
local isGameStarted = false
local getMapsFolder = game:GetService("Lighting").Maps:GetChildren()
local mapPicked = getMapsFolder[math.random(1,#getMapsFolder)]
local getLobby = game:GetService("Lighting").Lobby.M_Lobby
local intermissionStarted = game:GetService("ReplicatedStorage").Events.IntermissionStarted
local roundStarted = game:GetService("ReplicatedStorage").Events.RoundStarted
local intermissionInterval = 15 -- how long intermission is
local roundInterval = 80 -- how long round is
local function destroyMaps()
for i,v in ipairs(workspace:GetChildren()) do
if v:IsA("Model") and string.match(v.Name, "M") then
v:Destroy()
end
end
end
local function killPlayers()
for i,v in pairs(game.Players:GetPlayers()) do
if v and v.Character then
v.Character.Humanoid.Health = 0
end
end
end
local function cloneMap(getMap)
if getMap ~= nil then
local clonedMap = getMap:Clone()
clonedMap.Parent = workspace
end
end
local function startGame()
isGameStarted = true
destroyMaps()
killPlayers()
cloneMap(getLobby)
intermissionStarted:FireAllClients(intermissionInterval)
task.wait(intermissionInterval)
destroyMaps()
cloneMap(mapPicked)
roundStarted:FireAllClients(roundInterval)
task.wait(roundInterval)
isGameStarted = false
end
task.spawn(function()
while task.wait() do
if not isGameStarted then
startGame()
end
end
end)
Must the maps be stored within Lighting? It may be wiser to hold it in ServerStorage so it’s not replicated to clients.
if startGame() is intended to be an infinite loop by the coroutine calling it whenever it finishes, could startGame() not call itself within the function at the end? Or change it all to an infinite loop itself, not a declared function.
In function killPlayers(), you’re not going to get a case where v == nil when iterating the table of players returned by GetPlayers(). I personally would use v:BreakJoints() instead of setting Health to 0, I’ve never benchmarked the difference in this, but I’ve adopted it by recommendation of other devforum posts.
In cloneMap(), getMap ~= nil can be rewritten as getMap, since ~= nil is like saying “If getMap is not nonexistent,” just coding style recommendation.
Since your code is always calling destroyMaps() and cloneMap() in pairs, perhaps you could just host a variable that holds the reference to the active map instance and have one function that deletes it if it already exists before spawning the next map and reassigning the variable to the new one. This would also be more performant than iterating everything in workspace to delete “M”.
I know the solution was marked, but something else you can try to do to increase performance is to not clone your maps. Cloning a model takes way longer than just parenting it to workspace.
For testing purposes, I did:
local b = game.ServerStorage.Model
task.wait(5) -- ignore
local a = tick()
b:Clone().Parent = workspace -- cloning
print(tick()-a)
and
local b = game.ServerStorage.Model
task.wait(5) -- ignore
local a = tick()
b.Parent = workspace -- no cloning
print(tick()-a)
I found that cloning makes the process ~2x as long. It really depends on how many instances are inside your map.
Cloning took 1.1547510623931885 seconds.
Without cloning it took 0.4606599807739258 seconds.
The difference there is big. You can essentially cut the lag in half by not using :Clone everytime you need a map. Also the less instances (not just parts! this means unnecessary thumbnail cameras, etc) you have, the faster it’ll be.
The only reason you should clone maps is if the players are allowed to destroy large chunks of it. If players can blow up the entire map, then use :Clone however if the player can only destroy windows or barrels (random examples, could be anything small) then don’t use clone but instead clone the barrels/windows as those will take far less time and generate less lag.
^
I agree with this, you could just parent the map to ServerStorage/ReplicatedStorage,
I’d go with ReplicatedStorage, because I can make clients load the maps using ContentProvider so their character doesn’t clip through the map if they’re using a low-end pc
Humanoid:TakeDamage(9e9) – 9e9 is an extremely big number, will kill every default humanoid.
Iterations: 100
Time: 0.001468…
Humanoid:TakeDamage is only as fast as setting the Health manually when the Humanoid’s health is -inf(?). TakeDamage should only be used for preference reasons.
To kill a humanoid: Humanoid.Health = -9e9 – sometimes, very rarely, not even sure if it still happens, setting the health to zero can be bypassed if the player heals within the same frame.
To cause damage to a humanoid: Humanoid.Health -= 50 – 50 is a placeholder. set it to whatever you want to cause that amount of damage.
Note:
I really only replied to this for fun because I love comparing certain functions to see which is faster. Use whatever you want.