How could I improve this script?

Help and Feedback Code Review
#1

Hi, I’m looking for help on shortening and improving this script, any help is greatly appreciated.

local ReplicatedStorage = game:GetService("ReplicatedStorage")

local DoorMessage = ReplicatedStorage.Events:WaitForChild("DoorMessage")

local TimerSpeed = ReplicatedStorage.Values:WaitForChild("TimerSpeed")
local MultiplierVisible = ReplicatedStorage.Values:WaitForChild("MultiplierVisible")
local Multiplies = ReplicatedStorage.Values:WaitForChild("Multiplies")
local Minutes = ReplicatedStorage.Values:WaitForChild("Minutes")
local Seconds = ReplicatedStorage.Values:WaitForChild("Seconds")

local Winners = {}

script.Parent.Touched:Connect(function(Hit)
	
	local Player = game.Players:GetPlayerFromCharacter(Hit.Parent)
	
	if not Hit.Parent:FindFirstChild("Humanoid") then return end
	
	if not Player then return end
	
	if table.find(Winners, Player.UserId) then return end
	
	table.insert(Winners, Player.UserId)
	TimerSpeed.Value = TimerSpeed.Value / 2
	DoorMessage:FireAllClients(Player)
	MultiplierVisible.Value = true
	Multiplies.Value = Multiplies.Value * 2 
end)

Minutes.Changed:Connect(function()
	
	if Minutes.Value == 2 and Seconds.Value == 0 then
		
		if not Winners then return end
		
		for _, id in pairs(Winners) do
			
			local Player = game:GetService("Players"):GetPlayerByUserId(id)
			
			if not Player then continue end
			if not Player:WaitForChild("leaderstats") then continue end
			if not Player.leaderstats:WaitForChild("🍬 Candy") then continue end
			
			Player.leaderstats:WaitForChild("🍬 Candy").Value = Player.leaderstats:WaitForChild("🍬 Candy").Value + 80
			
			if Player.DoubleCandy.Value == true then
				Player.leaderstats:WaitForChild("🍬 Candy").Value = Player.leaderstats:WaitForChild("🍬 Candy").Value + 80
			end
		end
		Winners = {}
	end
end)
1 Like
#2

One question, is this a local or server script? You can avoid the wait for childs if the values are precreated or stored in replicated storage during game start up by using DataModel.Loaded. As explained by Sleitnick below:

Otherwise It’s pretty ok, however you can improve the neatness here by localizing that value.

local playerCandyValue = Player.leaderstats:WaitForChild("🍬 Candy")
local addedAmount = Player.DoubleCandy.Value and 160 or 80
playerCandyValue .Value +=  addedAmount
1 Like
#3

Alright thanks, it’s a regular script by the way.

#4

I’d suggest using attributes in place of -Value objects (except for the leaderstats values).

Also, not too important, but it’s good practice to declare local Players = game:GetService("Players") at the script’s beginning before using its functions later in the script (GetPlayerFromCharacter and GetPlayerByUserId in this case) even though the Players service is only used once or twice in your script.

2 Likes