Is this the best way to do this?

Help and Feedback Code Review
#1

This is a round system script recently reworked, and I would like to know if this is the approach. have not finished the round(), but the other code works. I need to know if I use this number of events or if there is a better way to implement a round system.

I send the player ID to put the profile picture of the player into the map they voted for.


local players = game:GetService("Players")
local serverStor = game:GetService("ServerStorage")
local mapFolder = serverStor:FindFirstChild("Maps")
local replicated = game:GetService("ReplicatedStorage")
local RoundEvents = replicated:FindFirstChild("Round Sys Events")

local updateTime = RoundEvents:FindFirstChild("UpdateTimer")
local voteUpdate = RoundEvents:FindFirstChild("VoteUpdate")
local voteCast = RoundEvents:FindFirstChild("VoteCast")
local voteBegin = RoundEvents:FindFirstChild("VoteBegin")

local serverScript = game:GetService("ServerScriptService")
local RandomMapModule = require(serverScript:FindFirstChild("Voting Module"):FindFirstChild("Map Randomiser"))

local InterTime = 5
local votingTime = 5
local RoundTime = 120

local selectedMaps = {}
local mapNames = {}

local function Intermission()
	selectedMaps = {}
	mapNames = {}
	for  i = InterTime, 0 , -1 do
		task.wait(1)
		updateTime:FireAllClients(i)
	end
end
local function Voting()
	RandomMapModule:getMap(mapFolder, selectedMaps, mapNames) -- Gets 3 Random Maps From the folder
	print(selectedMaps)
	voteBegin:FireAllClients(selectedMaps, votingTime, mapNames)
	
	for i = votingTime, 0, -1 do
		task.wait(1)
		updateTime:FireAllClients(i)
		
	end
end
voteCast.OnServerEvent:Connect(function(player, value, ID)



		voteUpdate:FireAllClients(ID, value)
	
end)
while true do
	task.wait(1)
	Intermission()
	Voting()
end
1 Like
#2 
for i = votingTime, 0, -1 do
	task.wait(1)
	updateTime:FireAllClients(i)
	
end

i would probably avoid this and just send the client the tick that the voting ends, for example:

local function Voting()
	RandomMapModule:getMap(mapFolder, selectedMaps, mapNames) -- Gets 3 Random Maps From the folder
	print(selectedMaps)

	votingTimeEnd = tick() + votingTime
	voteBegin:FireAllClients(selectedMaps, votingTimeEnd, mapNames)
	
	task.wait(votingTimeEnd - tick())
end

im not sure if the script spends time waiting to fire all clients for it to go to the next line, but if it does the votingTimeEnd - tick() corrects it. it also makes so it doesnt have to fire an event to all clients every second. :+1:

i like the organization, but i would make the constant values like this:

local INTER_LENGTH = 5
local VOTING_LENGTH = 5
local ROUND_LENGTH = 120

i prefer so you really know what variables are suposed to be constant and which aren’t. also i named them “length” since i use “time” for variables that use or compare to the tick()

#3

I use the update time remote event to send current time in local script to update the time in the Timer GUI that the game has, so it does not appear different in every player.

I really like your way but I am yet to learn tick(), I will see how I can use tick () to do this, But is it better to use remote events for this or should I use some kind of attribute which keeps changing