Code cleanup
Tab your code correctly. This lets you actually see which statements are run together in conditions and loops.
for i, v in pairs(game.Players:GetChildren()) do
local player = v:FindFirstChild("PlayerGui")
local screengui4 = Instance.new('ScreenGui', player)
screengui4.Name = "playercheckgui4"
local textLabel4 = Instance.new('TextLabel', player.playercheckgui4)
textLabel4.Position = UDim2.new(0.45, 0, 0.4, 0)
textLabel4.Size = UDim2.new(0, 250, 0, 100)
textLabel4.FontSize = "Size28"
textLabel4.BackgroundTransparency = "1"
textLabel4.TextStrokeTransparency = "0"
for i, f in pairs(game.Players:GetPlayers()) do
if f.TeamColor == BrickColor.new("Bright red") then
textLabel4.Text = (f.Name .. " is the chosen one!")
end
end
for i=1, 50 do
textLabel4.TextColor3 = Color3.new(math.random(), math.random(), math.random())
wait(0.1)
end
wait(2)
end
Use good variable names. You don't use i so use _ to indicate you don't use it. v doesn't mean anything, use player. Don't put unnecessary parenthesis around strings. Numbers aren't strings, don't put them in quotes. Use :GetPlayers() not :GetChildren() to get the sequence of players. Use one style of quotes consistently.
Don't use player.playercheckgui4. Just use the variable you have, screengui4.
After this cleanup, your code is much more readable while still doing the same things:
for _, player in pairs(game.Players:GetPlayers()) do
local gui = player:FindFirstChild("PlayerGui")
local screen = Instance.new("ScreenGui", gui)
screen.Name = "playercheckgui4"
local textLabel4 = Instance.new("TextLabel", screen)
textLabel4.Position = UDim2.new(0.45, 0, 0.4, 0)
textLabel4.Size = UDim2.new(0, 250, 0, 100)
textLabel4.FontSize = "Size28"
textLabel4.BackgroundTransparency = "1"
textLabel4.TextStrokeTransparency = "0"
for _, f in pairs(game.Players:GetPlayers()) do
if f.TeamColor == BrickColor.new("Bright red") then
textLabel4.Text = f.Name .. " is the chosen one!"
end
end
for i = 1, 50 do
textLabel4.TextColor3 = Color3.new(math.random(), math.random(), math.random())
wait(0.1)
end
wait(2)
end
Problem
Inside the main loop, you have waits. That means you process one player, wait a bit, then process the next player.
If you don't want to wait after processing each player, then that code doesn't belong in the for loop. That's why we tab our code, so that you can see what belongs where, easily.
It's not really the best that you have a for loop inside another for loop -- that's a code-smell that indicates something's not quite right. The f that you find should be the same for each one; you should just compute a chosenPlayer variable ahead of this loop.
It might look something like this:
local chosenPlayer
for _, player in pairs(game.Players:GetPlayers()) do
if player.TeamColor == BrickColor.new("Bright red") then
chosenPlayer = player
end
end
assert(chosenPlayer)
for _, player in pairs(game.Players:GetPlayers()) do
local gui = player:FindFirstChild("PlayerGui")
local screen = Instance.new("ScreenGui", gui)
screen.Name = "playercheckgui4"
local textLabel4 = Instance.new("TextLabel", screen)
textLabel4.Position = UDim2.new(0.45, 0, 0.4, 0)
textLabel4.Size = UDim2.new(0, 250, 0, 100)
textLabel4.FontSize = "Size28"
textLabel4.BackgroundTransparency = "1"
textLabel4.TextStrokeTransparency = "0"
textLabel14.Text = f.Name .. " is the chosen one!"
end
for i = 1, 50 do
for _, player in pairs(game.Players:GetPlayers()) do
local label = player.PlayerGui.playercheckgui4.textLabel14
label.TextColor3 = Color3.new(math.random(), math.random(), math.random())
end
wait(0.1)
end
wait(2)
The right way
You should not be manipulating each player's GUIs from a Script object.
You should be using FilteringEnabled, and all GUI manipulation should be done by a LocalScript.
Use RemoteEvents / RemoteFunctions to communicate state to the LocalScripts.
2 comments
Excuse you?
iandvdo mean something. They mean "index" and "value". Another variable name often used instead ofiisk, which stands for "key"; a synonym for "index".Fiesty o.o