How can i make this code clean

Help and Feedback Code Review
,
#1

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
#2

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.

#3

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

#4

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)
#5

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

here’s before your code

#6

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

#7

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

#9

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