Round system code review

Help and Feedback Code Review
#1

Code in question:

local mapPos = game.Workspace.MapPosition
local mapRotation = game:GetService("ReplicatedStorage"):WaitForChild("Maps")

local intermissionTimer = 10
local gameTimer = 50
local gameActive = false
local timeMulti = 1

--Intermission phase
while true do
	print("Intermission")
	
	repeat
		intermissionTimer = intermissionTimer - 1
		print(intermissionTimer)
		task.wait(1)
	until intermissionTimer <= 0
	
	print("Round needs to start")
	
	local randomMap = mapRotation:GetChildren()[math.random(1, #mapRotation:GetChildren())]
	clonedMap = randomMap:Clone()
	
	clonedMap:SetPrimaryPartCFrame(mapPos.CFrame)
	clonedMap.Parent = game.Workspace.CurrentMap
	
	local mapName = clonedMap:GetAttribute("MapName")
	
	print("Map Selected: "..mapName)
	
	task.wait(2)
	
	playersAlive = {}
	playersWon = {}
	
	for i, player in pairs(game.Players:GetPlayers()) do
		table.insert(playersAlive, player.Name)
		player.Character.HumanoidRootPart.CFrame = clonedMap.SpawnPart.CFrame
	end
	
	gameActive = true
	
	repeat
		print("Game in progress")
		gameTimer -= 1
		print(gameTimer)
		task.wait(1 * timeMulti)
	until gameTimer == 0
	
	clonedMap:Destroy()
	
	for i, player in pairs(playersAlive) do
		game.Players[player].Character.HumanoidRootPart.CFrame = game.Workspace.SpawnLocation.CFrame
	end
	
	gameActive = false
	gameTimer = 10
	intermissionTimer = 10
end

Any room for improvement?

#2

Be careful when handling Players’ Characters or any descendants of them. There are instances where players won’t be fully loaded in (such as them joining recently or leaving recently), if that happens you’ll get the "Attempt to index nil with Character" bug causing the whole round system to implode and break.

Use sanity checks regarding players (if player.Character then)

On that note in general, I would also recommend wrapping the whole round system in a pcall.

while true do
    pcall(function()
        -- round stuff
    end)
end

The last thing you want is for some sneaky bug to pop up and then a server gets bricked, (player’s end up doing nothing and then leaving) doing this just ensures that even if an error were to occur, it will loop around and start anew.

2 Likes
#3

Use the Players service’s PlayerAdded and PlayerRemoved events to handle player joins and leaves instead of looping through all players in the game

Add error handling for cases where the mapRotation object or its children are not available.

Remember to check if a player.Character is there

#4

Personally I’d use recursion instead of an infinite loop, that’s just my personal preference, looks a lot cleaner in my opinion :slight_smile: