A Round system review

Help and Feedback Code Review
#1

I would like to recibe opinions and able to change and improve my code.
I am not satisfied with my code because I feel that __newindex is a mess with a lot of if statements

local PlayerUtility = require('../Utility/PlayerUtility')

---| Services |---
local Players = game:GetService('Players')
local workspace = game:GetService('Workspace')
local ServerStorage = game:GetService('ServerStorage')
local ReplicatedStorage = game:GetService('ReplicatedStorage')

---| Variables |---
local Remotes = ReplicatedStorage.Remotes
local Events = ServerStorage.Events

---| Main Round |---
local Round = {}
Round.Teachers = {}

local Phases = {}
Phases.PhaseInOrder = {'Waiting', 'Intermission'}

---| Round Variables |---
Round.Current = 0
Round.Thread = nil
Round.PhaseIndex = 0

--[[
__Index = reference to Round
__Newindex = Recive calls when Variable Changes
Current = Send to client update of timer
Thread = Type of threads Start,end,cancel.
PhaseIndex = Send What phase we are currently. --Maybe Ill change to server side only..
]]
local RoundSettings = setmetatable({},
{
	__index = Round,
	__newindex = function(_,Key,Value) --Find out if theres a better way to do this
		if Key == 'Current' then
			Remotes.CurrentTime:FireAllClients(Round[Key])
		elseif Key == 'Thread' then
			if Value == nil then
				Events.TimeCancel:Fire()
			elseif not Value then
				Events.TimeEnd:Fire()
			elseif coroutine.status(Value) == 'suspended' then
				Events.TimeStart:Fire()
			end
			
		elseif Key == 'PhaseIndex' then
			Remotes.RoundPhase:FireAllClients(Value)
		end
		Round[Key] = Value
	end,
})

---| Round Functions |---

--Start the whole game system
function Round.Init()
	Round.NextPhase()
end

--Go to next phase, Round.PhaseIndex and Phases.PhaseInOrder
function Round.NextPhase()
	if RoundSettings.PhaseIndex >= #Phases.PhaseInOrder then Round.PhaseIndex = 0 end
	RoundSettings.PhaseIndex += 1
	
	local GetPhase = Phases.PhaseInOrder[RoundSettings.PhaseIndex]
	
	if GetPhase and Phases[GetPhase] then
		Phases[GetPhase]()
	end
end

--Countdown, will cancel the last countdown if ran again
function Round.CountDown(Duration: number)
	if RoundSettings.Thread and coroutine.status(RoundSettings.Thread) == 'suspended' then
		task.cancel(RoundSettings.Thread)
		RoundSettings.Thread = nil
	end
	
	RoundSettings.Current = Duration
	
	RoundSettings.Thread = task.spawn(function()
		while RoundSettings.Current >= 0 do
			RoundSettings.Current -= 1
			task.wait(1)
		end
		RoundSettings.Thread = false
	end)
end

--Adds or removes time
function Round.TimeShift(Amount: number)
	if not (RoundSettings.Thread and Amount ~= 0) then return end
	
	RoundSettings.Current += Amount
	Events.TimeShift:Fire(Amount)
end

--Is there enough players to start?
function Round.EnoughPlayers()
	return #Players:GetPlayers() >= 2
end

--Select Teachers
function Round.ChooseTeachers()
	table.clear(Round.Teachers)

	local Current = PlayerUtility:FetchPlayerChances()
	local Max = 1

	if #Players:GetPlayers() >= 6 then
		Max = 3
	elseif #Players:GetPlayers() >= 3 then
		Max = 2
	end

	for Index = 1, Max do
		local Weight = 0

		for _,Value in pairs(Current) do
			Weight += Value
		end

		Weight *= math.random()

		for Player, Value in pairs(Current) do
			Weight -= Value

			if Weight <= 0 then
				table.insert(Round.Teachers, Player)
				Current[Player] = nil
				break
			end
		end
	end
	print(Round.Teachers)
end


---| Round Phases |---

function Phases.Waiting()
	if Round.EnoughPlayers() then return Round.NextPhase() end

	local PlayerAdded:RBXScriptConnection
	
	PlayerAdded = Players.PlayerAdded:Connect(function()
		if not Round.EnoughPlayers() then return end

		PlayerAdded:Disconnect()
		return Round.NextPhase()
	end)
	
end

function Phases.Intermission()
	Round.CountDown(10)
	Events.TimeEnd.Event:Wait()
	Round.NextPhase()
end

return Round
#2

I would lean towards using specific “setter” functions It makes the code easier to read and understand for you and anyone else who might look at it later

1 Like