How to make cleaner (not repetitive) scripts

Help and Feedback Scripting Support
#1

So I’ve recently been getting into scripting and I’m having a lot of fun learning new things to make and new tools to use
One thing I am really struggling with, is a clean/organized script.

My first example of one of my unorganized repetitive scripts is

local CollectionService = game:GetService("CollectionService")
local YellowCoins = CollectionService:GetTagged("Ycoin")
local GreenCoins = CollectionService:GetTagged("Gcoin")
local BlueCoins = CollectionService:GetTagged("Bcoin")
local RedCoins = CollectionService:GetTagged("Rcoin")

for _, Part in pairs(YellowCoins) do
	Part.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			local plr = game.Players:GetPlayerFromCharacter(hit.Parent)
			Part:Destroy()
			plr.leaderstats.Coins.Value = plr.leaderstats.Coins.Value + 1
		end
	end)
end

for _, Part in pairs(GreenCoins) do
	Part.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			local plr = game.Players:GetPlayerFromCharacter(hit.Parent)
			Part:Destroy()
			plr.leaderstats.Coins.Value = plr.leaderstats.Coins.Value + 5
		end
	end)
end

for _, Part in pairs(BlueCoins) do
	Part.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			local plr = game.Players:GetPlayerFromCharacter(hit.Parent)
			Part:Destroy()
			plr.leaderstats.Coins.Value = plr.leaderstats.Coins.Value + 10
		end
	end)
end

for _, Part in pairs(RedCoins) do
	Part.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			local plr = game.Players:GetPlayerFromCharacter(hit.Parent)
			Part:Destroy()
			plr.leaderstats.Coins.Value = plr.leaderstats.Coins.Value + 50
		end
	end)
end

As you can see, I repeated the same script multiple times with little changes

I thought that I would have to create a table, but I don’t know how I would do that if I also need to change certain values depending on the instance. (example being that the yellow coin gives +1 score and the green coin gives +5 score)

What way would I be able to shorten this script down so that it doesn’t look so messy and repetitive?

1 Like
#2

You can use local functions to help shorten this.

New code:

--//Services
local Players = game:GetService("Players")
local CollectionService = game:GetService("CollectionService")

--//Functions
local function InitializePart(part, coinIncrease)
	part.Touched:Connect(function(hit)
		local Player = Players:GetPlayerFromCharacter(hit.Parent)
		
		if not Player then
			return
		end
		
		part:Destroy()
		
		Player.leaderstats.Coins.Value += coinIncrease
	end)
end

for _, Part in ipairs(CollectionService:GetTagged("Ycoin")) do
	task.spawn(InitializePart, Part, 1)
end

for _, Part in ipairs(CollectionService:GetTagged("Gcoin")) do
	task.spawn(InitializePart, Part, 5)
end

for _, Part in ipairs(CollectionService:GetTagged("Bcoin")) do
	task.spawn(InitializePart, Part, 10)
end

for _, Part in ipairs(CollectionService:GetTagged("Rcoin")) do
	task.spawn(InitializePart, Part, 50)
end
2 Likes
#3

Could simplify it even more like this:

local CollectionService = game:GetService("CollectionService")

local function addCoinType(tagName, value)
    local taggedParts = CollectionService:GetTagged(tagName)
    for _,v in pairs(taggedParts) do
        v.Touched:Connect(function(hit)
    		if hit.Parent:FindFirstChild("Humanoid") then
    			local player = game:GetService("Players"):GetPlayerFromCharacter(hit.Parent)
    			if player then
    			    v:Destroy() 
    			    plr.leaderstats.Coins.Value += value
    			end
    		end
    	end)
    end
end

addCoinType("Ycoin", 1)
-- so on
2 Likes
#4 
local CS = local CollectionService = game:GetService("CollectionService")
local coins = {
	table.unpack(CS:GetTagged("Ycoin"),
	table.unpack(CS:GetTagged("Gcoin"),
	table.unpack(CS:GetTagged("Bcoin"),
	table.unpack(CS:GetTagged("Rcoin")
}

for _, Part in pairs(coins) do
	Part.Touched:Connect(function(hit)
		if hit.Parent:FindFirstChild("Humanoid") then
			local plr = game.Players:GetPlayerFromCharacter(hit.Parent)
			Part:Destroy()
			local coinval = CS:HasTag(Part, "Ycoin") and 1 or CS:HasTag(Part, "Gcoin") and 5 or CS:HasTag(Part, "Bcoin") and 10 or CS:HasTag(Part, "Rcoin") and 50
			plr.leaderstats.Coins.Value += coinval
		end
	end)
end
2 Likes
#5

If you want to stay consistent with CollectionService then we could create a dictionary table that assigns the Tag name with the Value.

local CollectionService = game:GetService("CollectionService")
local CoinConfig = {
Ycoin = 1,
Gcoin = 5,
Bcoin = 10,
Rcoin = 50
}

for TagName, CoinValue in pairs(CoinConfig) do
    local CoinList = CollectionService:GetTagged(TagName)
    for _, CoinPart in pairs(CoinList) do
        CoinPart.Touched:Connect(function(Hit)
            if Hit.Parent and Hit.Parent:FindFirstChild("Humanoid") then
                local Player = game.Players:GetPlayerFromCharacter(Hit.Parent)
                CoinPart:Destroy()
                Player.leaderstats.Coins.Value += CoinValue
            end
        end
    end
end
3 Likes
#6

You should also probably add a task.spawn so it sets all the functions instantly. You also forgot to make it add the actual value set in, instead of 1

New code:

--//Services
local Players = game:GetService("Players")
local CollectionService = game:GetService("CollectionService")

--//Functions
local function addCoinType(tagName, value)
	local taggedParts = CollectionService:GetTagged(tagName)
	
	for _, part in pairs(taggedParts) do
		task.spawn(function()
			part.Touched:Connect(function(hit)
				local Player = Players:GetPlayerFromCharacter(hit.Parent)
				
				if not Player then
					return
				end
				
				part:Destroy()
				
				Player.leaderstats.Coins.Value += value
			end)
		end)
	end
end

task.spawn(addCoinType, "Ycoin", 1)
-- so on
1 Like
#7

Touched does not delay a script, therefore, task.spawn is useless and not needed.

2 Likes
#8

Thank you, to everyone who replied, I’m gonna learn from these.
I see now how I can use a table to shorten it, or an alternative to a table like in this reply!
again thank you! I appreciate all the replies.

1 Like
#9

One more thing I want to add is that instead of doing:

value = value + 1

You can do

value += 1

They mean the same exact thing and one can save you a whole lot of space as seen above!

1 Like
#10

You should most likely use the table solution as shown in the reply from @GetGlobals. Not only can you use the table for other purposes (eg. getting all the available coin types), but you can also get rid of the addCoinType function call, which will become quite repetitive, something you want to avoid.

1 Like