How can I improve this script?

Hello!

So I have this script here that inserts a map into Workspace and teleports the player to it when the game starts. (Sorry if that feels too bland, the game is VERY simple.)

Everything works great, I just want to know how to improve the main game script.

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")

local Text = game.Workspace:WaitForChild("Text")

task.wait(4)

local function GameStart()
	ReplicatedStorage.GameStart:FireAllClients()
	local map = ReplicatedStorage.Maps.Map1:Clone()
	map.Name = "Map"
	map.Parent = game.Workspace
	task.wait(1)
	Text.Value = "Game Starting In 5 Seconds!"
	task.wait(1)
	Text.Value = "Game Starting In 4 Seconds!"
	task.wait(1)
	Text.Value = "Game Starting In 3 Seconds!"
	task.wait(1)
	Text.Value = "Game Starting In 2 Seconds!"
	task.wait(1)
	Text.Value = "Game Starting In 1 Seconds!"
	task.wait(1)
	Text.Value = "The Game Has Started!  GO!"
	SoundService.GameMusic:Play()
	SoundService.LobbyMusic:Stop()
	game.Workspace.Barrier.CanCollide = false
	game.Workspace.Barrier.Transparency = 1
end
game.Workspace.Finish.Touched:Connect(function(otherPart)
	if otherPart.Parent:FindFirstChild("Humanoid") ~= nil then
		SoundService.LobbyMusic:Play()
		SoundService.GameMusic:Stop()
		otherPart.Parent:MoveTo(game.Workspace.SpawnPoint.Position)
		ReplicatedStorage.GameEnd:FireAllClients()
		game.Workspace.Barrier.CanCollide = true
		Text.Value = otherPart.Parent.Name .. "Has Won! Congrats!"
		task.wait(1)
		game.Workspace:WaitForChild("Map"):Destroy()
		Text.Value = "Intermission"
		game.Workspace.Barrier.Transparency = 0.6
		task.wait(15)
		GameStart()
	end
end)

SoundService.LobbyMusic:Play()
task.wait(5)
GameStart()

I feel like it’s really messy and the part that changes the Text.Value could be improved using a for loop, but I’m just not sure what would be best. Thank you for your replies and have a wonderful day! :slight_smile:

2 Likes

While I’m still an amateur at programming I’ll try to give some tips.

The only thing I could suggest is using concatenation and adding a loop for the game countdown. This can minimize some unnecessary code.

map.Parent = game.Workspace
for i =5, 1, -1 do
	Text.Value = "Game Starting In " .. i.. " Seconds!"
	task.wait(1)
end
Text.Value = "The Game Has Started!  GO!"
SoundService.GameMusic:Play()
1 Like

That’s perfect! That function looks so much nicer now!

But I just noticed that whenever the game ends, the output is spammed with a warning: Infinite yield possible on 'Workspace:WaitForChild("Map")'

Also, when the next round starts the map isn’t in the workspace so you’re just standing on the baseplate.

change it to

Workspace:WaitForChild("Map",10)

the reason ur getting that error is because you arent telling how long the game should wait for the map so it could potentially be waiting forever if something goes wrong with the spawning the map

1 Like

Now it gives me this error:

Workspace.GameHandler:34: attempt to index nil with 'Destroy'

I’ve gotten this before, but I don’t remember how to fix it.

im pretty sure its because you declare map locally in the game start function and in the finish touch theres no such variable as map

1 Like

I declared the map instead at the top of script, but I still the error.

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")

local Text = game.Workspace:WaitForChild("Text")

local map = ReplicatedStorage:WaitForChild("Maps"):WaitForChild("Map1"):Clone()
map.Name = "Map"
map.Parent = game.Workspace

task.wait(4)

local function GameStart()
	ReplicatedStorage.Events.GameStart:FireAllClients()
	
	for i = 5, 1, -1 do
		Text.Value = "Game Starting In " .. i .. " Seconds!"
		task.wait(1)
	end
	Text.Value = "The Game Has Started!  GO!"
	SoundService.GameMusic:Play()
	SoundService.LobbyMusic:Stop()
	game.Workspace.Barrier.CanCollide = false
	game.Workspace.Barrier.Transparency = 1
end

game.Workspace.Finish.Touched:Connect(function(otherPart)
	if otherPart.Parent:FindFirstChild("Humanoid") ~= nil then
		SoundService.LobbyMusic:Play()
		SoundService.GameMusic:Stop()
		otherPart.Parent:MoveTo(game.Workspace.SpawnPoint.Position)
		ReplicatedStorage.Events.GameEnd:FireAllClients()
		game.Workspace.Barrier.CanCollide = true
		Text.Value = otherPart.Parent.Name .. "Has Won! Congrats!"
		task.wait(1)
		game.Workspace:WaitForChild("Map", 10):Destroy()
		Text.Value = "Intermission"
		game.Workspace.Barrier.Transparency = 0.6
		task.wait(10)
		GameStart()
	end
end)

SoundService.LobbyMusic:Play()
task.wait(5)
GameStart()

why dont you just use the variable u delcared at the top instead of waiting for it

Like this?

game.Workspace.Map:Destroy()

you should just use the variable map like

if map then
map: Destroy ()
end
1 Like

Expertly formatted
He means

if map then
	map:Destroy ()
end
1 Like

I would recommend that you use Players:GetPlayerFromCharacter() instead of otherPart.Parent:FindFirstChild("Humanoid") ~= nil, since it allows you to have the player object be stored instead of having a reference to the hitpart. This is notable because the script is a little difficult to follow when it’s managing references to the player name, and also having a reference to the player would allow you to easily adapt into datastores or leaderstats.

As the others mentioned, you can also replace game.Workspace:WaitForChild("Map", 10):Destroy() with map:Destroy(), since earlier in the script you set the variable to the map object.

I also recommend switching otherPart.Parent:MoveTo(game.Workspace.SpawnPoint.Position) with just

for _,player in Players:GetPlayers() do
    player:LoadCharacter()
end

This is mostly so you can rely on the server for respawning players, and so that everybody gets respawned. This ensures that the entire lobby will be reset.

1 Like

There are no errors now, but when the next round starts, there is no map.

Code:

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")

local Text = game.Workspace:WaitForChild("Text")

local map = ReplicatedStorage:WaitForChild("Maps"):WaitForChild("Map1"):Clone()
map.Name = "Map"
map.Parent = game.Workspace

task.wait(4)

local function GameStart()
	ReplicatedStorage.Events.GameStart:FireAllClients()
	
	for i = 5, 1, -1 do
		Text.Value = "Game Starting In " .. i .. " Seconds!"
		task.wait(1)
	end
	Text.Value = "The Game Has Started!  GO!"
	SoundService.GameMusic:Play()
	SoundService.LobbyMusic:Stop()
	game.Workspace.Barrier.CanCollide = false
	game.Workspace.Barrier.Transparency = 1
end

game.Workspace.Finish.Touched:Connect(function(otherPart)
	local Player = game:GetService("Players"):GetPlayerFromCharacter(otherPart.Parent)
	SoundService.LobbyMusic:Play()
	SoundService.GameMusic:Stop()

	for _, Player in game:GetService("Players"):GetPlayers() do
		Player:LoadCharacter()
		ReplicatedStorage.Events.GameEnd:FireAllClients()
		game.Workspace.Barrier.CanCollide = true
		Text.Value = otherPart.Parent.Name .. "Has Won! Congrats!"
		task.wait(1)
		if map then
			map:Destroy()
			Text.Value = "Intermission"
			game.Workspace.Barrier.Transparency = 0.6
			task.wait(10)
			GameStart()
		end
	end	
end)

SoundService.LobbyMusic:Play()
task.wait(5)
GameStart()

I fixed it! I put the map variables inside the GameStart() function and then checked if the map is in workspace using FindFirstChild()!

Code:

local ReplicatedStorage = game:GetService("ReplicatedStorage")
local SoundService = game:GetService("SoundService")

local Text = game.Workspace:WaitForChild("Text")

task.wait(4)

local function GameStart()
	ReplicatedStorage.Events.GameStart:FireAllClients()
	
	local map = ReplicatedStorage:WaitForChild("Maps"):WaitForChild("Map1"):Clone()
	map.Name = "Map"
	map.Parent = game.Workspace
	
	for i = 5, 1, -1 do
		Text.Value = "Game Starting In " .. i .. " Seconds!"
		task.wait(1)
	end
	Text.Value = "The Game Has Started!  GO!"
	SoundService.GameMusic:Play()
	SoundService.LobbyMusic:Stop()
	game.Workspace.Barrier.CanCollide = false
	game.Workspace.Barrier.Transparency = 1
end

game.Workspace.Finish.Touched:Connect(function(otherPart)
	local Player = game:GetService("Players"):GetPlayerFromCharacter(otherPart.Parent)
	SoundService.LobbyMusic:Play()
	SoundService.GameMusic:Stop()
	
	for _, Player in game:GetService("Players"):GetPlayers() do
		Player:LoadCharacter()
		ReplicatedStorage.Events.GameEnd:FireAllClients()
		game.Workspace.Barrier.CanCollide = true
		Text.Value = otherPart.Parent.Name .. "Has Won! Congrats!"
		task.wait(3)
		
		if game.Workspace:FindFirstChild("Map") then
			local map = game.Workspace:FindFirstChild("Map")
			if map then
				map:Destroy()
				Text.Value = "Intermission"
				game.Workspace.Barrier.Transparency = 0.6
				task.wait(10)
				GameStart()
			end	
		end
	end	
end)

SoundService.LobbyMusic:Play()
task.wait(5)
GameStart()

Thank you to everybody who helped! Have a wonderful day! :slight_smile:

shut . I was typing it without script editor on my phone

1 Like

It’s just a joke, Technoblade :moyai:

1 Like

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.