Anything wrong with this code?

Help and Feedback Scripting Support
#1

I’m trying to get this code to be a perfect loop of a lobby-game-lobby, but for some reason it doesn’t loop correctly.

I’ve tried rearranging some code but it never worked.

Everything that is defined is correct, no errors in the developer console.

Here’s the code:

while true do
	game.Teams.Lobby.AutoAssignable = true
	game.Teams.Bravo.AutoAssignable = false
	game.Teams.Foxtrot.AutoAssignable = false
	local lobby = game.ReplicatedStorage.LobbyMap:Clone()
	lobby.Parent = workspace
	workspace.LobbyMusic.Playing = true
	for _, v in pairs(game.Players:GetChildren()) do
		if v:IsA("Player") then
			v.Team = game.Teams.Lobby
			wait()
			local i = v.Backpack:FindFirstChildOfClass("Tool")
			v.i:Destroy()
			if v.Character:FindFirstChildOfClass("Tool") then
				v.Character:FindFirstChildOfClass("Tool")
			end
			v.Character.Humanoid.Health = 0
		end
	end
	if game.Workspace:FindFirstChild("Map") then
		game.Workspace["Map"]:Destroy()
	end
	local m = Instance.new("Hint", workspace)
	for i = 30,1,-1 do
		m.Text = ("Game will start in: "..i)
		wait(1)
	end
	if m.Text == "Game will end in: 1" then
		wait(1)
		m:Destroy()
	end
	Folder = game.ReplicatedStorage.Maps:GetChildren()
	local rMap = Folder[math.random(1,#Folder)]:Clone()
	m:Destroy()
	local h = Instance.new("Message")
	h.Parent = game.Workspace 
	h.Text = "The next map will be... "..rMap.Name.."!"
	wait(2)
	h:Remove()
	workspace.LobbyMusic.Playing = false
	rMap.Name = "Map"
	rMap.Parent = game.Workspace
	game.Teams.Lobby.AutoAssignable = false
	game.Teams.Bravo.AutoAssignable = true
	game.Teams.Foxtrot.AutoAssignable = true
	local red = game.Teams.Bravo
	local yellow = game.Teams.Foxtrot
	local teams = {red, yellow}
	for _, v in pairs(game.Players:GetPlayers()) do
		if v:IsA("Player") then
			wait()
			v.Character.Humanoid.Health = 0
			wait()
			if #red:GetPlayers() > #yellow:GetPlayers() then
				v.Team = yellow
			elseif #red:GetPlayers() < #yellow:GetPlayers() then
				v.Team = red
			else
				v.Team = teams[math.random(1, #teams)]
			end
		end
	end
	if game.Workspace:FindFirstChild("LobbyMap") then
		game.Workspace["LobbyMap"]:Destroy()
	end
	local m2 = Instance.new("Hint", workspace)
	for i = 300,1,-1 do
		m2.Text = ("Game will end in: "..i)
		wait(1)
	end
	if m2.Text == "Game will end in: 1" then
		wait(1)
		m2:Destroy()
	end
end
#2

This is a intimidating segment of code here. I would love to help, but I need a bit more information.

What do you mean by “it doesn’t loop correctly”. Do you mean that at some point in the loop the thread just stops? Does the loop run a few times before it stops, or does it stop on its first go-around? Also, if either of those are the case, can you point to specifically where the loop stops?

1 Like
#3

Yes, at the end of the loop on it’s first time (and only time) it stops after the ‘game will end in’ text. Also, before I posted this I didn’t have the destroy text for the end text, but it still had the same outcome.

#4

Hmm… I can’t see anything that would completely stop the loop; however, I do notice that around the middle of the loop, you have the following code snippet:

for i = 30,1,-1 do
	m.Text = ("Game will start in: "..i)
	wait(1)
end
if m.Text == "Game will end in: 1" then
	wait(1)
	m:Destroy()
end

Seems like the if statement should check if the text is “Game will start in: 1” instead of end, since the preceding text uses “start”.

However, this should not be the reason for the failed loop. I am quite stumped with this. Could you send a video of this loop in action?

#5

I shortened the time for the video,

I tried to keep it as short as possible but as you can see the message just gets destroyed and nothing else happens.

#6

The loop isn’t the issue, as the loop seems to be functioning totally fine.
However, while im not entirely too sure, I am concerned about:

	if game.Workspace:FindFirstChild("Map") then
		game.Workspace["Map"]:Destroy()
	end

which, if Map is misnamed (either poor capitalization or different name all together) could result in the map not being deleted.
However, I do not have enough information to make that many decisions. Also feels really really disorganized. And also I personally do not like having infinite loops of any kind.

#7
  • Replace wait() with task.wait()
  • Try while task.wait() do instead of while true do, as it would loop every heartbeat, instead of all the time.
  • For the loop below, you should eithier use another function for it or call it in a coroutine, as it would significantly slow down the loop.
#8

You should check the output window in studio, the script is yielding most likely due to an error, you can also access the output window by pressing F9 in game and navigating to the “Server” tab.

As for cleanliness well, yeah, it needs a lot of cleaning up. There are some redundant checks like this one:

	if m.Text == "Game will end in: 1" then
		wait(1)
		m:Destroy()
	end

If you aren’t going to do anything if the text isn’t "Game will end in: 1" then there really isn’t a point to check it, flow moves linearly since everything you’re doing is in series, so the check runs after the for loop is done.

As @coolalex1835 pointed out, you should use task.wait() instead of wait(), and you should avoid calling them without arguments unless it really is necessary.

I’d also avoid any variable declarations within a loop unless it’s necessary because they will keep getting redeclared, so you can move

	local red = game.Teams.Bravo
	local yellow = game.Teams.Foxtrot
	local teams = {red, yellow}

and any other variable declarations out of the loop.

You should also try to write code that is more self-documenting, so avoid using variable names like local m = Instance.new("Hint", workspace) or local m2 = Instance.new("Hint", workspace).

I don’t know where your script is but it should be in ServerScriptService if it isn’t already, I’d also move the maps into ServerStorage instead of ReplicatedStorage if you plan on having a large number of maps, even though they will appear faster on the client side if they are in ReplicatedStorage they will still be in memory on the client, which could impact performance despite them not being rendered at all.

You should also note that :Remove() is deprecated, and you should use :Destroy() instead, and you use dot indexing to retreive services (game.Teams and such), this is slow and doesn’t consider the possibility that the service does not have an instance in the game, so it is better to use game:GetServi ce("ServiceName").

If you are sure the service exists then you should use game:FindService("ServiceName") which only partially accounts for the serivce not existing (returns nil as opposed to :GetService() creating an instance of the service) in exchange for being faster.

I’d also recommend using workspace instead of game.Workspace just for the sake of cleanliness.

To conclude, yeah, lots of things are wrong with this code, but none of them aren’t fixable and you shouldn’t be discouraged!

Edit: typo

1 Like
#9

Do you think I need to use game:GetService(“Players”) as well? Also is @coolalex1835 right about the while task.wait()? I just don’t know if that is what I need or not.

2 Likes
#10

I also recommend to put task.wait inside the loop. I don’t see any

#11

Alright so basically it works a bit better now, but now whenever it loops, it does not show the “Game will start in: blank” text. No errors /warnings in the output

#12

It also spawns the lobby, so it definitely goes until the hint

#13 
local m2 = Instance.new("Hint", workspace)
for i = 300,0,-1 do
	m2.Text = ("Game will end in: "..i)
	wait(1)
end
m2:Destroy()

Have the for loop count down to zero and remove the if conditional statement which proceeds it, then just destroy the “Hint” instance and start the loop over again.

#14 
local players = game:GetService("Players")
local replicated = game:GetService("ReplicatedStorage")
local lobbyMap = replicated.LobbyMap
local mapsFolder = replicated.Maps
local maps = mapsFolder:GetChildren()

local lobbyMusic = workspace.LobbyMusic

local teams = game:GetService("Teams")
local lobby = teams.Lobby
local bravo = teams.Bravo
local foxtrot = teams.Foxtrot
local gameTeams = {bravo, foxtrot}

while true do
	task.wait(5)
	lobby.AutoAssignable = true
	bravo.AutoAssignable = false
	foxtrot.AutoAssignable = false
	
	local lobbyClone = lobbyMap:Clone()
	lobbyMap.Parent = workspace
	lobbyMusic:Play()
	
	for _, player in ipairs(players:GetPlayers()) do
		player.Team = lobby
		local character = player.Character
		local humanoid = character.Humanoid
		humanoid:UnequipTools()
		for _, tool in ipairs(player.Backpack:GetChildren()) do
			if tool:IsA("Tool") then
				tool:Destroy()
			end
		end
	end
	
	local hint1 = Instance.new("Hint")
	hint1.Parent = workspace
	for i = 30, 0, -1 do
		hint1.Text = ("Game will start in: "..i)
		task.wait(1)
	end
	hint1:Destroy()
	
	local randomMap = maps[math.random(#maps)]:Clone()
	
	local message1 = Instance.new("Message")
	message1.Parent = workspace
	message1.Text = "The next map will be... "..randomMap.Name.."!"
	
	task.wait(2)
	message1:Destroy()
	lobbyMusic:Stop()
	randomMap.Name = "Map"
	randomMap.Parent = workspace

	lobby.AutoAssignable = false
	bravo.AutoAssignable = true
	foxtrot.AutoAssignable = true
	
	for _, player in ipairs(players:GetPlayers()) do
		player:LoadCharacter()
		if #bravo:GetPlayers() > #foxtrot:GetPlayers() then
			player.Team = foxtrot
		elseif #bravo:GetPlayers() < #foxtrot:GetPlayers() then
			player.Team = bravo
		else
			player.Team = gameTeams[math.random(#gameTeams)]
		end
	end
	
	local hint2 = Instance.new("Hint")
	hint2.Parent = workspace
	for i = 300, 0, -1 do
		hint2.Text = ("Game will end in: "..i)
		task.wait(1)
	end
	hint2:Destroy()
	
	local currentMap = workspace:FindFirstChild("Map")
	if currentMap then
		currentMap:Destroy()
	end
	
	local lobby = workspace:FindFirstChild("LobbyMap")
	if lobby then
		lobby:Destroy()
	end
end
#15

is there a reason to having the task.wait() in the beginning of the code instead of the end? also is there any other place I could destroy the LobbyMap? Because on some maps you could see the lobby above you the entire time, sometimes you are under.

#16

The wait can be moved to directly before the loop starts (it’s just a way of letting the game load first, the lobby can be destroyed whenever you choose fit (providing it exists).

#17 
if not game:IsLoaded() then
	game.Loaded:Wait()
end

local players = game:GetService("Players")
local replicated = game:GetService("ReplicatedStorage")
local lobbyMap = replicated.LobbyMap
local mapsFolder = replicated.Maps
local maps = mapsFolder:GetChildren()

local lobbyMusic = workspace.LobbyMusic

local teams = game:GetService("Teams")
local lobby = teams.Lobby
local bravo = teams.Bravo
local foxtrot = teams.Foxtrot
local gameTeams = {bravo, foxtrot}

while true do
	lobby.AutoAssignable = true
	bravo.AutoAssignable = false
	foxtrot.AutoAssignable = false

	local lobbyClone = lobbyMap:Clone()
	lobbyMap.Parent = workspace
	lobbyMusic:Play()

	for _, player in ipairs(players:GetPlayers()) do
		player.Team = lobby
		local character = player.Character
		local humanoid = character.Humanoid
		humanoid:UnequipTools()
		for _, tool in ipairs(player.Backpack:GetChildren()) do
			if tool:IsA("Tool") then
				tool:Destroy()
			end
		end
	end

	local hint1 = Instance.new("Hint")
	hint1.Parent = workspace
	for i = 30, 0, -1 do
		hint1.Text = ("Game will start in: "..i)
		task.wait(1)
	end
	hint1:Destroy()

	local randomMap = maps[math.random(#maps)]:Clone()

	local message1 = Instance.new("Message")
	message1.Parent = workspace
	message1.Text = "The next map will be... "..randomMap.Name.."!"

	task.wait(2)
	message1:Destroy()
	lobbyMusic:Stop()
	randomMap.Name = "Map"
	randomMap.Parent = workspace

	lobby.AutoAssignable = false
	bravo.AutoAssignable = true
	foxtrot.AutoAssignable = true

	for _, player in ipairs(players:GetPlayers()) do
		player:LoadCharacter()
		if #bravo:GetPlayers() > #foxtrot:GetPlayers() then
			player.Team = foxtrot
		elseif #bravo:GetPlayers() < #foxtrot:GetPlayers() then
			player.Team = bravo
		else
			player.Team = gameTeams[math.random(#gameTeams)]
		end
	end
	
	local lobby = workspace:FindFirstChild("LobbyMap")
	if lobby then
		lobby:Destroy()
	end
	
	local hint2 = Instance.new("Hint")
	hint2.Parent = workspace
	for i = 300, 0, -1 do
		hint2.Text = ("Game will end in: "..i)
		task.wait(1)
	end
	hint2:Destroy()

	local currentMap = workspace:FindFirstChild("Map")
	if currentMap then
		currentMap:Destroy()
	end
end