What can I do to improve this Round module?

Help and Feedback Code Review
,
#1

Hello, I am very confused with how to organize stuff like rounds for games. I made a build battle round (module)script and it does not look good at all, and it’s all over the place. If there’s any tips on how to organize it, or if there’s any inefficient mistakes then please let me know. Thanks!

local Round = {}

Round.Settings = {
	RoundLength = 60*10,-- Building Time
	VotingTimeLength = 20, --Time to vote for each build
	WinnerShowcaseTimeLength = nil, --self.RandomSong.TimeLength + 5, --Time for showcasing the winner's build after the round ends
	VotingThemeTimeLength = 15, --Time to vote theme

	IntermissionLength = 15, --Time between rounds. Not beuing used
	PlayerCountNeeded = 2, --Minimum amount of players needed to start a round. Not being used yet
}



------------------------------------------------------------------------
------------------------------------Variables-----------------------------
------------------------------------------------------------------------

--General
local ServerStorage = game:GetService("ServerStorage")
local ServerScriptService = game:GetService("ServerScriptService")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")
local PhysicsService = game:GetService("PhysicsService")

--Add
local baseTemplate = ServerStorage.Round.BaseTemplate
local toolsGui = ServerStorage.Round.ScreenGuiRemake

local blocksSelection = game:GetService("ReplicatedStorage"):WaitForChild("BlocksSelection")
local OrderOfBlocks = require(blocksSelection.OrderOfBlocks)
local BlacklistForFloor = require(blocksSelection.OrderOfBlocks.BlacklistForFloor)

--Modules And Remotes
local PlayerModules = ServerScriptService.PlayerData
local PlayerManager = require(PlayerModules.PlayerManager)
local PlotManager = require(PlayerModules.PlotManager)
local PlayerData = require(ReplicatedStorage.Modules.PlayerData)
local VotingManager = require(script.VotingManager)
local PodiumManager = require(script.PodiumManager)
local Sounds = require(ServerScriptService.Sounds)
local ThemesManager = require(script.ThemesManager)
local CommandManager = require(PlayerModules.CommandManager)
local Chat = require(ReplicatedStorage.Modules.Chat)
local LeakyBucket = require(PlayerModules.LeakyBucket)
local PlacementValidator = require(ReplicatedStorage.Modules.PlacementValidator)
local PlayerDataStore = require(ServerScriptService.PlayerData.PlayerDataStore)

local remotesFolder = ReplicatedStorage.Remotes
local toolsRemote = remotesFolder.Tools
local roundDataRemote = remotesFolder.RoundData
local statusValue = roundDataRemote.Status
local timerValue = statusValue.Timer



local titleValue = statusValue.TitleText


--PhysicsService:RegisterCollisionGroup("Blocks")
--PhysicsService:CollisionGroupSetCollidable("players", "players", false)

------------------------------------------------------------------------
------------------------------------Script------------------------------
------------------------------------------------------------------------

Round.__index = Round

------------------------------------Private-----------------------------


------------------------------------Creating-----------------------------

function Round.new()
	local self = {}
	
	self.RandomSong = Sounds.GetRandomSound(Sounds.Selections.Sounds.LofiWinsAlbum)

	self.RoundLength = Round.Settings.RoundLength-- Building Time
	self.VotingTimeLength = Round.Settings.VotingTimeLength --Time to vote for each build
	self.WinnerShowcaseTimeLength = Round.Settings.WinnerShowcaseTimeLength or self.RandomSong.TimeLength + 5 --Time for showcasing the winner's build after the round ends
	self.VotingThemeTimeLength = Round.Settings.VotingThemeTimeLength --Time to vote theme
	
	self.IntermissionLength = 15 --Time between rounds. Not beuing used
	self.PlayerCountNeeded = 2 --Minimum amount of players needed to start a round. Not being used yet
	
	self.Leaderboard = {}
	
	self._toolConnection = nil
	self._playerAddedConnection = nil
	self.Connections = {}
	
	return setmetatable(self, Round)
end

function Round:Init()
	self:VoteTheme()
	
	self:CreatePlots()
	self:TpPlayersToOwnPlot()
	self:GivePlayerTools()
	self:TurnOnBuildRemotes()
	
	self.CountdownTimer(self.RoundLength)
	PlotManager.StopPlotPropertiesListener()
	self:DisconnectConnections()
	
	self:TakeAwayTools()
	self:VotingTime()
	self:TpAllToWinner()
	self:ShoutoutWinners()
	self:UpdateDataStats()
	
	self:EndRound()
end

------------------------------------Round-----------------------------

function Round:VoteTheme()
	ThemesManager.ResetVotes()

	local randomThemes = ThemesManager.Get3RandomThemes()
	
	statusValue.Value = "VoteTheme"
	
	local themeVotes, randomThemes = ThemesManager.TurnOnRemote(randomThemes)
	
	self.CountdownTimer(self.VotingThemeTimeLength)

	local winningTheme = ThemesManager.GetWinningTheme(themeVotes, randomThemes)

	ThemesManager.AnnounceWinner(winningTheme)
	
	wait(1)
end

function Round:CreatePlots()
	statusValue.Value = "Building"
	
	Sounds.PlayRandomSound(Sounds.Selections.Songs.Lofi)

	PlayerManager.ForAllPlayersDo(function(player)
		local plotModel = PlotManager.SpawnPlot({player})
		
		PlayerManager.SetSpawnPoint(plotModel.Spawns:GetChildren(), player)
	end)
	
	self._playerAddedConnection = Players.PlayerAdded:Connect(function(player)
		PlayerManager.SetSpawnPointToLobby(player)
		PlayerManager.TeleportToSpawnpoint(player)
		
		wait(1)
		local plotModel = PlotManager.SpawnPlot({player})

		PlayerManager.SetSpawnPoint(plotModel.Spawns:GetChildren(), player)
		PlayerManager.TeleportToSpawnpoint(player)
		local tool = toolsGui:Clone()
		tool.Name = "BuildToollls"
		tool.Parent = player.PlayerGui

	end)
	
	self._playerRemovingConnection = Players.PlayerRemoving:Connect(function(player)
		local plotData = PlotManager.GetPlotData(player)
		if  plotData  then
			if #plotData.Players - 1 == 0 then
				PlotManager.RemovePlot(plotData)
			end
		end
	end)
end

function Round:TpPlayersToOwnPlot()
	task.wait(1)
	PlayerManager.ForAllPlayersDo(function(player)
		--player.Character.HumanoidRootPart.Position = plot.Plot.Region.PrimaryPart.Position
		PlayerManager.TeleportToSpawnpoint(player)
	end)
end

function Round:GivePlayerTools()
	CommandManager.TurnOnCommand("Fly")

	PlayerManager.ForAllPlayersDo(function(player)
		local tool = toolsGui:Clone()
		tool.Name = "Tools"
		tool.Parent = player.PlayerGui
		
		PlayerManager.OnCharacterDeath(player, "GiveTools", function()
			local tool = toolsGui:Clone()
			tool.Name = "Tools"
			tool.Parent = player.PlayerGui
		end)
	end)
end

local buildDebounce = false

function Round:TurnOnBuildRemotes()
	toolsRemote.OnServerInvoke = function(player, request, ...)
		
		local  team, base, buildFolder, region, floor = PlayerData.GetData(player)

		if request == "Place" then
			
			buildDebounce = true
			
			local blockRequest, data = ...
			local block = nil
			for _, category in OrderOfBlocks do
				local blockExists =  table.find(category, blockRequest)
				if blockExists then
					block = category[blockExists]
				end
			end
						
			if block then
				local newblock: Model = block:Clone()
				newblock:PivotTo(data.CFrame)
				newblock.PrimaryPart.Anchored = true --data.Anchored
				
				if PlacementValidator.IsInsideOtherPart(newblock) == true then newblock:Destroy() return "Success" end
				if not PlacementValidator.WithinBounds(PlotManager.GetPlotData(player).Model.Region, newblock) then return "Success" end

				local blockOffsets = newblock:FindFirstChild("Offsets") 

				if blockOffsets then
					local ceilingOffsets = blockOffsets:FindFirstChild("Ceiling")
					local floorOffsets = blockOffsets:FindFirstChild("Floor")
					local normalOffsets = blockOffsets:FindFirstChild("Normal")
					
					if data.Offset == "Ceiling" then
						for _, offset in ceilingOffsets:GetChildren() do
							offset.Effected.Value.Position = newblock.Hitbox.Position + offset.Value
						end

					elseif data.Offset == "Floor" then
						for _, offset in floorOffsets:GetChildren() do
							offset.Effected.Value.Position = newblock.Hitbox.Position + offset.Value
						end

					else
						for _, offset in normalOffsets:GetChildren() do
							offset.Effected.Value.Position = newblock.Hitbox.Position + offset.Value
						end
					end
				end
				
				for _, part in newblock:GetDescendants() do
					if part:IsA("BasePart") then
						part.Anchored = true

						--Allows player cameras to see through
						if part.Transparency == 0 and part.CanCollide == true then
							part.CanCollide = false
							local playerCollider = part:Clone()
							playerCollider.CanCollide = true
							playerCollider.Transparency = 1
							playerCollider.CFrame = part.CFrame
							playerCollider.Parent = part.Parent

							coroutine.wrap(function()
								local texture = playerCollider:FindFirstAncestorWhichIsA("Texture")
								if texture then
									for _, texture in part:GetDescendants() do
										if texture:IsA("Texture") then
											texture:Destroy()
										end
									end
								end
							end)()
						
						end
					end
					--part.CollisionGroup = ""
				end
				
				newblock.Parent = buildFolder
				
			end
			

			buildDebounce = false

			return "Success"
		elseif request == "Delete" then
			local clickedBlock = ...
			if not (clickedBlock.Parent == buildFolder) then return end
			
			clickedBlock:Destroy()

			return "Success"
		elseif request == "SetFloorColor" then
			--local color = ...
			--floor.Color = color	
			
			local block = ...
			local blockExists
			for _, category in OrderOfBlocks do
				blockExists =  table.find(category, block)
				if blockExists then break end
			end
			if not blockExists or table.find(BlacklistForFloor, block) then return end


			floor.Color = block.PrimaryPart.Color
			floor.Transparency = block.PrimaryPart.Transparency
			
			local blockTexture: Texture =  block.PrimaryPart:FindFirstChild("Texture")
			if blockTexture then
				for _, texture in block.PrimaryPart:GetDescendants() do--Find the top texture
					if texture:IsA("Texture") then
						if texture.Face == Enum.NormalId.Top then
							blockTexture = texture
						end
					end
				end
				
				if floor:FindFirstChild("Texture") then floor.Texture:Destroy() end
				
				local texture = Instance.new("Texture")
				texture.Texture = blockTexture.Texture
				texture.StudsPerTileU = blockTexture.StudsPerTileU
				texture.StudsPerTileV = blockTexture.StudsPerTileV
				texture.Face = Enum.NormalId.Top
				texture.Name = 'Texture'
				
				texture.Color3 = blockTexture.Color3
				texture.Transparency = blockTexture.Transparency
				texture.Parent = floor
			end

			
			
		end
		

	end
	
	local deleteAllBucket = LeakyBucket.new("DeleteAll", 1, 1, 60*10)

	table.insert(self.Connections, toolsRemote.RemoteEvent.OnServerEvent:Connect(function(player, request)		
		if request == "DeleteAll" then
			if deleteAllBucket:CheckIsFull() == true then return end
			deleteAllBucket:AddWater()
			
			local plotData = PlotManager.GetPlotData(player)
			if plotData then
				local built = plotData.Model.Build
				for _, block in built:GetChildren() do
					wait()
					if not block:IsA("Model") then continue end
						
					block:Destroy()
				end
			end
		end
	end))
	
	PlotManager.ConnectPlotPropertiesListener()
end

function Round:VotingTime()
	task.wait(2)
	game:GetService("Lighting").ExposureCompensation = 0.2
	game:GetService("Lighting").ExposureCompensation = 0.3

	coroutine.wrap(function()
		CommandManager.TurnOffCommands({"Fly"})
	end)()
	
	statusValue.Value = "VotingBuild"
	
	if self._playerAddedConnection then
		self._playerAddedConnection:Disconnect()
	end
	if self._playerRemovingConnection then
		self._playerRemovingConnection:Disconnect()
	end
	LeakyBucket.DestroyByName("DeleteAll")
	
	Sounds.PlayRandomSound(Sounds.Selections.Songs.Lofi)
	
	PlotManager.RemoveShadowBlocksForClients()
	
	
	PlayerManager.ForAllPlayersDo(function(player)
		if not player then return end
		VotingManager.GiveVotingTools(player)
	end)
	
	self._playerAddedConnection = Players.PlayerAdded:Connect(function(player)
		VotingManager.GiveVotingTools(player)
	end)
	
	VotingManager.TurnOnVoteRemotes()
	
	PlotManager.RandomizePlotTableOrder()
	
	for _, plot in PlotManager.Plots do
		CommandManager.TurnOnCommand("Fly")

		PlayerManager.ForAllPlayersDo(function(player)
			PlayerManager.SetSpawnPoint(plot.Model.Spawns:GetChildren(), player)
			PlayerManager.TeleportToSpawnpoint(player)
		end)

		roundDataRemote.RemoteEvent:FireAllClients("NewBuildToVote", unpack(plot.Players))

		VotingManager.InitVotesForNewPlot(plot)
		self.CountdownTimer(self.VotingTimeLength)
		VotingManager.AddVotesToPlotManager()
		
		CommandManager.TurnOffCommands({"Fly"})
	end
	
	VotingManager.SortPlotManagerByVotes()
	local winningPlot = PlotManager.Plots[1]
	local secondPlace = PlotManager.Plots[2]
	
	if secondPlace then
		if winningPlot.Stars == secondPlace.Stars then--If more than one plot has tie, then...
			local tiedPlots = VotingManager.GetAllOfSamePropertyAmount(function(plot)
				return plot.Stars == winningPlot.Stars
			end)

			winningPlot = VotingManager.TiebreakerVote(tiedPlots)
			print("Tiebreaker")
		end
	end

	self.Leaderboard = PlotManager.Plots
	
	print(winningPlot.Players[1], " Won with ", winningPlot.Stars, "Stars!")
	PlotManager.PrintPlots()

	--local mostVotedPlotName, mostVotedNum = VotingManager.NewPlotVotes()
	--if typeof(mostVotedPlotName) == type(table) then
	--	mostVotedPlotName = VotingManager.TiebreakerVote(mostVotedPlotName)
	--end
	
	VotingManager.ResetVotesForNewRound()

	self:TakeAwayTools()
end

function Round:TpAllToWinner()
	CommandManager.TurnOnCommand("Fly")
	
	statusValue.Value = "WinnersDrumroll"
	self.CountdownTimer(3)
	
	roundDataRemote.RemoteEvent:FireAllClients("Winners", self.Leaderboard[1].Players, self.Leaderboard[1].Model)
	
	local winner = self.Leaderboard[1]
	PlayerManager.ForAllPlayersDo(function(player)
		PlayerManager.SetSpawnPoint(winner.Model.Spawns:GetChildren(), player)
		PlayerManager.TeleportToSpawnpoint(player)
	end)
	Sounds.PlaySong(self.RandomSong)
	coroutine.wrap(function()
		task.wait(self.RandomSong.TimeLength - 1)
		Sounds.PlaySFX(Sounds.Selections.Sounds.Radio.Static, false)
	end)
	
	self:AddWinnersToChat()

	self.CountdownTimer(self.WinnerShowcaseTimeLength)
end

function Round:AddWinnersToChat()
	local message = "-------------------- \n <b>Leaderboard: \n"

	for i, plotData in PlotManager.Plots do
		local isTop3 = false
		
		if i == 1 then
			isTop3 = true
			message = message .. [[ <font color="#fff9a9">]] .. tostring(i) .. ": "
			Color3.new(1, 0.976471, 0.662745)
		elseif i == 2 then
			isTop3 = true
			message = message .. [[ <font color="#bcffff">]] .. tostring(i) .. ": "

		elseif i == 3 then
			isTop3 = true
			message = message .. [[ <font color="#ffd497">]] .. tostring(i) .. ": "
			
		else
			message = message ..  [[ <font color="#ffefff">]] .. tostring(i) .. ": "
		end

		for _, player in plotData.Players do
			message = message .. player.Name .. " - " .. tostring(plotData.Stars) .. " Stars"
		end
		
		--message = isTop3 and message .. "</font> \n" or message
		message ..= "</font>"
		message = if i == #PlotManager.Plots then message .. "</b>"  else message		
	end
	message = message .. " \n --------------------"
	Chat.AddChat({Text = message, Color = Color3.new(1, 1, 1)})
end

function Round:ShoutoutWinners()
	PodiumManager.ReskinRigs(self.Leaderboard)
	PodiumManager.AddBuildsToLobby(self.Leaderboard)
end

function Round:EndRound()
	CommandManager.TurnOffCommands({"Fly"})

	self._playerAddedConnection:Disconnect()
	self:DisconnectConnections()
	
	--self:TakeAwayTools()
	PlotManager.RemoveAllPlots()
end

function Round:UpdateDataStats()
	for i, plotData in PlotManager.Plots do

		for _, player in plotData.Players do

			local leaderstats = player:FindFirstChild("leaderstats")
			local wins = leaderstats:FindFirstChild("Wins")
			local averageRating = leaderstats:FindFirstChild("Average Rating")

			local otherData = player:FindFirstChild("OtherData")
			local numberOfTimesPeopleRated = otherData:FindFirstChild("NumberOfTimesPeopleRated")
			local totalRatings = otherData:FindFirstChild("TotalRatings")
			
			if i == 1 then
				wins.Value += 1
			end

			if leaderstats then
				if #PlotManager.Plots == 1 then return end
				--local playerTo15Multiplier = 15 / #PlotManager.Plots / 1.7
				
				
				numberOfTimesPeopleRated.Value = numberOfTimesPeopleRated.Value + #PlotManager.Plots
				totalRatings.Value = totalRatings.Value + plotData.Stars --* playerTo15Multiplier

				
				

				averageRating.Value = totalRatings.Value / numberOfTimesPeopleRated.Value
			
				
				PlayerDataStore:Update(player, "Leaderstats", function()
					return {Wins = wins.Value, ["Average Rating"] = averageRating.Value}
				end)
	
				PlayerDataStore:Update(player, "NumberOfTimesPeopleRated", function()
					return numberOfTimesPeopleRated.Value
				end)
				
				PlayerDataStore:Update(player, "TotalRatings", function()
					return totalRatings.Value
				end)
				
			end

		end
	end
end

------------------------------------Methods-----------------------------

function Round:TakeAwayTools()

	PlayerManager.TakeAwayTools()
	
	VotingManager.ResetVotesForNewRound()

	PlayerManager.ForAllPlayersDo(function(player)
		PlayerManager.UnbindToCharacterAdded(player, "GiveTools")
	end)
end

function Round:TpAllPlayers(Location: Vector3)
	PlayerManager.ForAllPlayersDo(function(player)
		player.Character:PivotTo(CFrame.new(Location))
	end)
end

function Round.CountdownTimer(timeLength: number)
	timerValue.Value = timeLength
	
	while timerValue.Value > 0 do
		task.wait(1)
		timerValue.Value -= 1
	end
	--for i=timeLength, 0, -1 do
	--	wait(1)
	--	timerValue.Value = i
	--end
	
	
	task.wait(2)
end

function Round:DisconnectConnections()
	toolsRemote.OnServerInvoke = nil
	for _, connection in self.Connections  do
		connection:Disconnect()
	end	
end

return Round
2 Likes
#2

Use Clear and Consise Dividers

Big blocks to seperate things can make code look crowded, instead of using something like;

------------------------------------------------------------------------
------------------------------------Variables---------------------------
------------------------------------------------------------------------

try using something more subtle, that stills serves the same purpose, for example:

----- Variables -----

or,

-- Variables

More Descriptive Comments

Some of your comments/titles don’t provide much context. For example, instead of using -- General, I would use something like -- Services

No More Empty Sections!

Having sections that have no content may confuse beginner coders, and do nothing but clutter your code. For example your “Private” title should not be there, as it has no content.

Code Placement

For better readability, add code that are relative to each other, closer together. For instance, I would move Round.__index = Round right below local Round = {}

More Descriptions!

Lastly, I would recommend putting descriptions before your functions, like this:

-- Constructor function
-- @return class Maid
function Maid.new()
   -- code
end

Resources

Here’s evaera’s guide to clean code: Guide to Clean Code By Evaera (I believe she has a more in-depth guide somewhere, but I was unable to find it :sweat_smile:)
I would also take a peek at public code, written by “professionals”, like Nevermore Engine By Quenty.

Note: I noticed in multiple parts of your code, you use multiple comments to make a hidden code block, you can instead use --[[ ]].
3 Likes
#3

Thanks. Can I ask another thing? Is this a good way to handle Rounds? As in, should I not use stuff like VotingManagers, PlotManagers, or whatever? What do you think is the best framework for something like this?

2 Likes