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?
--//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
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
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
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
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
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.
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.