Is my round intermission code okay?

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)
3 Likes

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”.

3 Likes

Thanks for the recommendations, I applied all of them and it works like a charm and looks cleaner than ever.

3 Likes

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.

1 Like

^
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

Remember the TakeDamage function. Do something like this:

if v.Character then
v.Character:WaitForChild("Humanoid"):TakeDamage(v.Character.Humanoid.MaxHealth)
end

TakeDamage is fine to use for preference, but it isn’t faster than simply setting the Health to zero.

Humanoid.Health = 0
Iterations: 100
Time: 0.000075…

Humanoid:TakeDamage(Humanoid.MaxHealth)
Iterations: 100
Time: 0.001790…

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 = -9e9sometimes, 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 -= 5050 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. :slight_smile:

1 Like

Very interesting. Thanks for the pointers, and it be great if Roblox ever added a function that kills humanoids.

1 Like