Code review on my round script

Help and Feedback Code Review
#1 
local functionsModule = require(script.ModuleScript)

local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")
local ReplicatedStorage = game:GetService("ReplicatedStorage")

local gameFolder = ReplicatedStorage.Game
local status = gameFolder.Status

local CONSTANTS = {
	requiredPlayers = 4;
}

while wait(1) do
	
	local players = {}
	
	repeat wait(1)
		
	for _, player in ipairs(Players:GetPlayers()) do
		if player and not table.find(players, player) then
			table.insert(players, player)
			print(string.format("Player was added to Queue: %s", player.Name))
	    end
	end

	status.Value = string.format("Waiting for enough players ( %s / %s )", #players, CONSTANTS.requiredPlayers)
		
	until #players >= CONSTANTS.requiredPlayers
	
	for intermission = 10, 0, -1 do
		status.Value = string.format("Intermission ( %s )", intermission)
		wait(1)
	end
	
end
#2

Can I ask what you’re trying to do I’m new to scripts so can you pls tell what you’re trying to do?

#3

The purpose of this code is strange. It’s an infinite loop, so even once all the players are present, it will still (1 second later) update the status to say it’s waiting for 4 / 4 players. It will then add all of these to the players table again, and print for each that they are added to the queue.

Is this the intent of the script? It doesn’t seem like you’d want to be caught in this loop forever, always stating that it’s waiting for players even though you’ve reached the goal already.

Aside from that, there’s no point having constant strings be formatted separately like this. You also end up with a double space before the parenthesis close due to this.

status.Value = string.format("Waiting for enough players ( %s %s %s %s", #players, "/", CONSTANTS.requiredPlayers, " )")

It makes better use of the function to just have the variable parts be unknown, and it also makes sure you aren’t doing double spaces where you just wanted one.

status.Value = string.format("Waiting for enough players ( %s / %s )", #players, CONSTANTS.requiredPlayers)
1 Like
#4

It isn’t a infinite loop, it’s unfinished at the moment of course. I prefer string.format rather than concatenation.

#5

It may be unfinished but it literally is an infinite loop with no break and the wait() will never return nil or false, therefore it will never stop.

You didn’t provide any description or any constraints on what we were and weren’t allowed to review so I reviewed the whole code.

And at no point did I suggest concatenation. Please look at the two comparing code snippets I included in my reply and read the accompanying text descriptions.

3 Likes