How can I further more optimize my game module

Help and Feedback Code Review
1

Hi, this module script is very long but it basically handles everything for my round-based game.

I still consider this messy so any changes to this will help! Thanks for reading.

--] Variables
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

local Remotes = ReplicatedStorage.Remotes.Classic
local Values = ReplicatedStorage.Values
local Voting = workspace.Lobby.Voting

local Maps = ServerStorage.Maps:GetChildren()
local Choices = Voting:GetChildren()

local TimeFormat = require(ReplicatedStorage.Modules.TimeFormat)

local Settings = {
	Round = {
		Interval = 25,
		RoundTime = 125
	};
	
	Strings = {
		NotEnoughPlayers = "Waiting for more players...",
		Starting = "Starting in: ",
		Ending = false,
	}
}

local Teams = {
	Red = game.Teams.Killer,
	Blue = game.Teams.Runner,
	Lobby = game.Teams.Lobby
}

local Status = Values.Status
local InRound = Values.InRound

-- declarations

local isAnOption
local randomMap
local chosenMap
local mapClone = nil
local previousKiller = nil
local killer = nil

local BlueCount
local RedCount
local RemainderBlue
local RemainderRed

local Connections = {}
local Playerlist = {}
local Jukebox = {}
Jukebox.__index = Jukebox

-- functions

function TeamChanged(Player)
	local Killbrick = ServerStorage.Assets.Red.KillBrick
	local RoundEnd = Remotes.End
	if Player.Team == Teams.Red then
		local character = Player.Character

		if character then
			Player.CameraMode = "LockFirstPerson"
			local kb = Killbrick:Clone()
			wait(0)
			kb.Parent = character.Head
			kb.CFrame = (character.Head.CFrame + Vector3.new(0,0,-2))
			kb.Orientation = Vector3.new (90,0,0)
			local weld = Instance.new("WeldConstraint")
			weld.Parent = character.Head
			weld.Part0 = kb
			weld.Part1 = character.Head

			kb.IsInUse.Value = true
		end

		Connections[Player] = Player.CharacterAdded:Connect(function(character)
			local kb = Killbrick:Clone()

			task.wait(0)
			kb.Parent = character.Head
			kb.CFrame = (character.Head.CFrame + Vector3.new(0,0,2))
			kb.Orientation = Vector3.new (90,0,0)
			local weld = Instance.new("WeldConstraint")
			kb.Name = "Killbrick"
			weld.Name = "Weld"
			weld.Parent = character.Head
			weld.Part0 = kb
			weld.Part1 = character.Head
			Player.CameraMode = "LockFirstPerson"

			kb.IsInUse.Value = true

		end)
	else
		local connection = Connections[Player]

		if connection then
			connection:Disconnect()
			RoundEnd.OnServerEvent:Connect(function(Player)
				local Character = Players.character
				Character.killerBlock:Destroy()
				Character.WeldThingy:Destroy()
				Character:GetPlayerFromCharacter().CameraMode = "Classic"
			end)
		end

	end
end

function PickRandomMap() -- Self Explanatory
	local randomNumber = math.random(1, #Maps)
	randomMap = Maps[randomNumber]
	return randomMap.CanBeVoted
end

function PickKiller()
	for _, player in ipairs(game.Players:GetChildren()) do  
		if player ~= previousKiller then
			table.insert(Playerlist, player)
		end
	end
	
	local SelectedKiller = Playerlist[math.random(1, #Playerlist)]
	previousKiller = SelectedKiller
	Playerlist = {}
	return SelectedKiller
end

function InitilizeVoteSystem()
	for i, choice in pairs(Choices) do
		local name = choice.label.SurfaceGui.TextLabel
		local picture = choice.Image.SurfaceGui.ImageLabel
		isAnOption = PickRandomMap()
		if isAnOption.Value == true then
			repeat 
				isAnOption = PickRandomMap()
			until isAnOption.Value == false
			name.Text = randomMap.Name
			picture.Image = randomMap.Image.Value
			randomMap.CanBeVoted.Value = true
		else
			name.Text = randomMap.Name
			picture.Image = randomMap.Image.Value
			randomMap.CanBeVoted.Value = true		
		end					
	end	
end

function RoundChanged()
	if InRound == true then -- round is in progress
		local Choice1Votes = #Voting.Choice1.button.Votes:GetChildren()
		local Choice2Votes = #Voting.Choice2.button.Votes:GetChildren()
		local Choice3Votes = #Voting.Choice3.button.Votes:GetChildren()
		
		if Choice1Votes >= Choice2Votes and Choice1Votes >= Choice3Votes then
			chosenMap = Voting.Choice1.label.SurfaceGui.TextLabel.Text
		elseif Choice2Votes >= Choice1Votes and Choice2Votes >= Choice3Votes then
			chosenMap = Voting.Choice2.label.SurfaceGui.TextLabel.Text
		else
			chosenMap = Voting.Choice3.label.SurfaceGui.TextLabel.Text
		end
		
		Status.Value = "Map: ".. chosenMap
		
		for _, map in pairs(Maps) do
			if chosenMap == map.Name then
				mapClone = map:Clone()
				mapClone.Parent = game.Workspace
			end
		end	
		
		task.wait(3)
		
		for i, choice in pairs(Choices) do
			choice.label.SurfaceGui.TextLabel.Text = " "
			choice.Image.SurfaceGui.ImageLabel.Image = " "
			choice.button.Votes:ClearAllChildren()
			ReplicatedStorage.GameEvents.VoteReset:FireAllClients(choice.button)
		end
		
		local BlueTeamCount = {}
		local RedTeamCount = {}
		local killer = PickKiller()
		
		if killer then
			local RedSpawns = mapClone.RedSpawns:GetChildren()
			local BlueSpawns = mapClone.BlueSpawns:GetChildren()
			for i, plr in ipairs(game.Players:GetChildren()) do
				local char = plr.Character
				local humanRoot = char:WaitForChild("HumanoidRootPart")
				-- picks a random spawn from each team
				local randomRedSpawn = RedSpawns[math.random(1,#RedSpawns)]
				local randomBlueSpawn = BlueSpawns[math.random(1,#BlueSpawns)]
				-- will put the current player into a team with the less amount of players
				if plr ~= killer then
					plr.Team = Teams.Blue
					humanRoot.CFrame = randomBlueSpawn.CFrame + Vector3.new(math.random(1,3),math.random(1,3),math.random(1,3))
					plr.CameraMode = Enum.CameraMode.Classic
					plr.CameraMaxZoomDistance = 9
					table.insert(BlueTeamCount, plr.Name)
				elseif plr == killer then
					plr.Team = Teams.Red
					plr.CameraMode = Enum.CameraMode.LockFirstPerson
					plr.CameraMaxZoomDistance = 9
					humanRoot.CFrame = randomRedSpawn.CFrame + Vector3.new(math.random(1,3),math.random(1,3),math.random(1,3))
					table.insert(RedTeamCount, plr.Name)
				end
			end
		end
	else -- not in progress
		for _, player in pairs(Players:GetPlayers()) do
			local Character = player.Character or player.CharacterAdded:Wait()
			if Character then -- just incase
				local HumanoidRootPart = Character:FindFirstChild("HumanoidRootPart")
				HumanoidRootPart.CFrame = (workspace.Lobby.lobbySpawn.Position + Vector3.new(0,5,0))
			end
		end
		-- voting system
		mapClone:Destroy()
		for i, map in pairs(Maps) do
			map.CanBeVoted.Value = false
		end

		for i, choice in pairs(Choices) do
			local name = choice.label.SurfaceGui.TextLabel
			local picture = choice.Image.SurfaceGui.ImageLabel
			isAnOption = PickRandomMap()
			if isAnOption.Value == true then
				repeat 
					isAnOption = PickRandomMap()
				until isAnOption.Value == false
				name.Text = randomMap.Name
				picture.Image = randomMap.Image.Value
				randomMap.CanBeVoted.Value = true
			else
				name.Text = randomMap.Name
				picture.Image = randomMap.Image.Value
				randomMap.CanBeVoted.Value = true		
			end	
		end
	end
end

-----
function Jukebox.new()
	local self = setmetatable({},Jukebox)
	
	return self
end

function Jukebox:Round()
	while true do
		
		repeat 
			Status.Value = Settings.Strings.NotEnoughPlayers
			task.wait(1)
		until #Players:GetChildren() >= 2
		
		InRound.Value = false
		
		for i = Settings.Round.Interval,0,-1 do
			Status.Value = Settings.Strings.Starting..i
			--if #Players:GetChildren() >= 2 then
			--	repeat 
			--		Status.Value = Settings.Strings.NotEnoughPlayers
			--		task.wait(1)
			--	until #Players:GetChildren() >= 2
			--end
		end
		
		BlueCount = {}
		RedCount = {}
		
		task.wait(3)
		for i = Settings.Round.RoundTime,0,-1 do
			task.wait(1)
			local Time = TimeFormat:Convert(i, "Default", true)
			if Settings.Strings.Ending == false then
				Status.Value = Time
			else
				Status.Value = Settings.Strings.Ending..Time
			end
			

			local RemainderBlue = {}
			local RemainderRed = {}

			for _, player in pairs(Players:GetPlayers()) do
				if player.Team == Teams.Blue then
					if i == Settings.Round.RoundTime then
						table.insert(BlueCount,player.Name)
					end
					table.insert(RemainderBlue,player.Name)
				elseif player.Team == Teams.Red then
					if i == Settings.Round.RoundTime then
						table.insert(RedCount,player.Name)
					end
					table.insert(RemainderRed, player.Name)
				end	
			end
			
			if #RemainderRed == 0 and i == 0 then -- both teams lost
				break
			end
			
			if #RemainderRed == 0 or i == 0 then -- blue won
				break
			end
			
			if #RemainderBlue == 0 then -- red won
				break
			end
		end
	end
end

-- connections
Values.InRound.Changed:Connect(RoundChanged)

game.Players.PlayerAdded:Connect(function(Player)
	script.Loading:Clone().Parent = Player.PlayerGui -- loads game
	
	Player:GetPropertyChangedSignal("Team"):Connect(TeamChanged)
	Player.CharacterAdded:Connect(function(Character)
		Character.Humanoid.Died:Connect(function()
			Player.Team = Teams.Lobby
			Player.CameraMode = Enum.CameraMode.Classic
			Player.CameraMaxZoomDistance = 9
		end)
	end)
end)

-- initilizing gamemode
InitilizeVoteSystem()
Jukebox:Round()


return Jukebox
2

I have suggested some minor improvements to your code below. Hope this helps.

1: I have noticed you’ve used wait() and task.wait() in a part of your script:

I highly suggest using task.wait() instead of wait() as its more efficient and slightly more faster than wait().

2: Instead of:

You can just set them all to nil or just leave them blank like so:

local isAnOption
local randomMap
local chosenMap
local mapClone
local previousKiller
local killer

3:

Instead of checking every one second, you can just check if the amount of players in-game is 2 every time a player has been added:

repeat
	Status.Value = Settings.Strings.NotEnoughPlayers
	Players.PlayerAdded:Wait()
until #Players:GetPlayers() >= 2

4: I also want to point out that whenever you use a for loop and you never use the i (index) variable, you should replace it with an _ which basically is known as a placeholder variable that replaces a variable you won’t use:

for _, v in pairs(myTable:GetChildren()) do
    print(v)
end
1 Like
3

I took a quick read and here are some suggestions to make this code better.

  1. Split the code into smaller functions and classes: The code seems to do a lot of things in one big script. You can break it down into smaller functions or classes, which will make it easier to read and maintain. For example, you can have a class for handling the vote system, a class for handling the rounds, and so on.
  2. Use modules: It’s a good idea to split the code into different modules and require them when needed. This will make it easier to manage and organize the code.
  3. Use constants: Instead of hardcoding values like “Red” and “Blue” throughout the code, you can define them as constants at the beginning of the script. This will make it easier to change them if needed.
  4. Avoid unnecessary repetition: There are some parts of the code that are repeated unnecessarily. For example, the code for picking a random map can be refactored into a separate function that can be reused.
  5. Simplify the logic: The code has some complex logic that can be simplified. For example, the code for picking a killer can be simplified by using a shuffle algorithm. You can shuffle the list of players and pick the first player as the killer.
  6. Use event-driven programming: The code can benefit from using event-driven programming instead of polling. For example, instead of checking the number of players every second, you can use a player added/removed event to keep track of the number of players.
  7. Use standard coding conventions: The code should follow standard coding conventions, such as using camelCase for function names and using meaningful variable names. This will make it easier for other developers to read and understand the code.
4 Likes