How can i make this code clean

I just want to check if my code is clean and doesn’t make any crashes

I’m making a timer player limiter teleporter

local ts = game:GetService('TeleportService')
local PlaceId = 9479160887
local maxPlayers = 16
local Players = game:GetService('Players')
local ReplicatedStorage = game:GetService('ReplicatedStorage')
local Folder = script.Parent:WaitForChild('Players')
local intermissionLength = 30
local Players = game:GetService('Players')
local remote1 = ReplicatedStorage:WaitForChild('Remotes'):WaitForChild('JoinLobby')
local remote2 = ReplicatedStorage:WaitForChild('Remotes'):WaitForChild('LeaveLobby')
local node = script.Parent.Parent:WaitForChild('TeleportNode')
local node2 = script.Parent.Parent:WaitForChild('TeleportNode2')
local playerCount = script.Parent:WaitForChild('billboard'):WaitForChild('SurfaceGui'):WaitForChild('TextLabel')
local timerText = script.Parent:WaitForChild('billboard'):WaitForChild('timer'):WaitForChild('TextLabel')

local TimerActive = false

function checkIfExists(Character) -- checks if player exists
	for _,value in pairs(Folder:GetChildren()) do
		if value.Value == Character then
			return true
		end
	end
	return false

end

function teleportParty()
	local Plrs = {}
	for _,value in pairs(Folder:GetChildren()) do 
		if value then
			table.insert(Plrs,Players:GetPlayerFromCharacter(value.Value))
		end
	end
	local Code = ts:ReserveServer(PlaceId)
	ts:TeleportToPrivateServer(PlaceId,Code,Plrs)
	Folder:ClearAllChildren()	
end

function timer()
	if not TimerActive then
		TimerActive = true
		intermissionLength = 30
		while wait() do
			for i = intermissionLength,0,-1 do
				timerText.Visible = true
				wait(1)
				intermissionLength =  i
				timerText.Text = i
				if #Folder:GetChildren() == 0   then
					intermissionLength = 30
					TimerActive = false
					return
				end
			end
			if #Folder:GetChildren() >= maxPlayers or intermissionLength == 0 then
				TimerActive = false
				teleportParty()				
			end
		end
	end
end


function UpdateGui()
	playerCount.Text = tostring(#Folder:GetChildren())..'/'..tostring(maxPlayers)
end


script.Parent.Touched:Connect(function(part)
	if part.Parent:FindFirstChild('Humanoid') then
		local player = part.Parent
		if not checkIfExists(player) then
			local Tag = Instance.new('ObjectValue')
			Tag.Value = part.Parent
			Tag.Parent = Folder
			part.Parent:SetPrimaryPartCFrame(node.CFrame)
			remote1:FireClient(Players:GetPlayerFromCharacter(part.Parent))
		end
		if #Folder:GetChildren() > 0 then
			timer()
		end
	end
end)

remote2.OnServerEvent:Connect(function(player)
	for _,value in pairs(Folder:GetChildren()) do
		if value.Value == player.Character then
			value:Destroy()
			player.Character:SetPrimaryPartCFrame(node2.CFrame)
		end
	end

end)



Folder.ChildAdded:Connect(UpdateGui)
Folder.ChildRemoved:Connect(UpdateGui)

Feedback would be appreciated (:

2 Likes

This category is only for scripting support not for feedback I think.
Question “How can I make this code clean” is fine for this category.

I think this question would be fitting for the Code review topic. However let me take a look and see what I can do.

Alrighty this should be better, and hopefully still work.

I moved around some of your variables to make them make more sense, removed a duplicate variable, and changed a lot of your if statements to Guard Clauses.

--			SERVICES			--

local Players = game:GetService('Players')
local ts = game:GetService('TeleportService')
local ReplicatedStorage = game:GetService('ReplicatedStorage')

--			 REMOTES			--

local remote1 = ReplicatedStorage:WaitForChild('Remotes'):WaitForChild('JoinLobby')
local remote2 = ReplicatedStorage:WaitForChild('Remotes'):WaitForChild('LeaveLobby')

--			GAME INFO			--
local PlaceId = 9479160887
local maxPlayers = 16

--		PLAYERS AND NODES		--
local Folder = script.Parent:WaitForChild('Players')
local node = script.Parent.Parent:WaitForChild('TeleportNode')
local node2 = script.Parent.Parent:WaitForChild('TeleportNode2')

--			UI STUFF			--

local playerCount = script.Parent:WaitForChild('billboard'):WaitForChild('SurfaceGui'):WaitForChild('TextLabel')
local timerText = script.Parent:WaitForChild('billboard'):WaitForChild('timer'):WaitForChild('TextLabel')
local intermissionLength = 30

local TimerActive = false

--			FUNCTIONS			--

function checkIfExists(Character) -- checks if player exists
	for _,value in pairs(Folder:GetChildren()) do
		if value.Value == Character then return true end
	end
	return false
end

function teleportParty()
	local Plrs = {}
	for _,value in pairs(Folder:GetChildren()) do 
		if not value then continue end
		table.insert(Plrs,Players:GetPlayerFromCharacter(value.Value))
	end
	local Code = ts:ReserveServer(PlaceId)
	ts:TeleportToPrivateServer(PlaceId,Code,Plrs)
	Folder:ClearAllChildren()	
end

function timer()
	if TimerActive then return end
	
	TimerActive = true
	intermissionLength = 30
	
	while task.wait() do
		for i = intermissionLength,0,-1 do
			timerText.Visible = true
			wait(1)
			intermissionLength =  i
			timerText.Text = i
			if #Folder:GetChildren() == 0   then
				intermissionLength = 30
				TimerActive = false
				return
			end
		end
		if #Folder:GetChildren() >= maxPlayers or intermissionLength == 0 then
			TimerActive = false
			teleportParty()				
		end
	end
end


function UpdateGui()
	playerCount.Text = tostring(#Folder:GetChildren())..'/'..tostring(maxPlayers)
end


script.Parent.Touched:Connect(function(part)
	if not part.Parent:FindFirstChild('Humanoid') then return end
	
	local player = part.Parent
	if not checkIfExists(player) then
		local Tag = Instance.new('ObjectValue')
		Tag.Value = player
		Tag.Parent = Folder
		part.Parent:SetPrimaryPartCFrame(node.CFrame)
		remote1:FireClient(Players:GetPlayerFromCharacter(part.Parent))
	end
	if #Folder:GetChildren() > 0 then
		timer()
	end
end)

remote2.OnServerEvent:Connect(function(player)
	for _,value in pairs(Folder:GetChildren()) do
		if not value.Value == player.Character then continue end
		value:Destroy()
		player.Character:SetPrimaryPartCFrame(node2.CFrame)
	end

end)

Folder.ChildAdded:Connect(UpdateGui)
Folder.ChildRemoved:Connect(UpdateGui)

hey I just tried your code but the timer didn’t work
here’s After your code

here’s before your code

Ok I just made an edit to the script, try it now!

it worked but when you leave the numbers on the timer they stay the as same when you were there it doesn’t come back to 30

sorry for the late response

Also sorry for the late response, can you check the explorer throughout the testing to make sure that the playersFolder thingy is actually updating