You are–accidentally, I suppose–defining Click2 as Button.ClickDetector instead of Button2.ClickDetector. So, when you click on the rifle, nothing will happen. But because you set up two event listeners for the gas mask’s ClickDetector, both of the functions connected to those event listeners will execute.
You should structure your code in a way that it is secure against exploiters and makes your code easy to read and add onto or configure.
Do not forget the DRY principle: don’t repeat yourself. As scripters and programmers, our goal is to be as lazy as possible. Not lazy in the sense that we copy and paste to execute the same code more than once, but lazy in the sense that, for example, when we want to change a value that’s being used multiple times, we don’t need to search through the whole script to change every instance that the value is being used in–we would just need to change it once, and the code will automatically do the rest for us.
Here are examples of going by the DRY priniciple and not, respectively:
-- Let's say workspace.Parts is a folder.
local parts = workspace.Parts
local function connectTouched(part: BasePart)
part.Touched:Connect(function(...)
-- ...
end)
end
for _, part: BasePart in parts:GetChildren do
connectTouched(part)
end
parts.ChildAdded:Connect(connectTouched)
for _, part: BasePart in workspace.Parts:GetChildren() do
part.Touched:Connect(function(...)
-- ...
end)
end
workspace.Parts.ChildAdded:Connect(function(part)
part.Touched:Connect(function(...)
-- ...
end)
end)
In the second example, you can see that DRY has not been applied to the code.
Which means that if we wanted to change the code that executes when the BaseParts get touched, we would have to update both the code inside the for loop and the code inside the function connected to the Instance.ChildAdded event.
But in the first example, all we would need to do to change the code that executes when a BasePart gets touched is change the code inside the connectTouched() function.
And did you notice that in the second example I didn’t store the path to the Parts Folder in a variable? I typed workspace.Parts each time I wanted to use the folder.
So, instead of manually setting up ClickDetector.MouseClick event listeners for every object, let the code do it for you. All you need to do is tell the script what you want to happen for those objects.
If you have went to an online shopping website and selected an item you wanted to buy, you probably could have chosen the quantity of the item you wanted. Instead of having to order that item [x] times, you just had to tell the server that you wanted [x] of that item.
You should group the equippable items together, using a Folder or tagging them, iterate through those items, like I did in the first snippet above, and set up the ClickDetector.MouseClick events for each of those items.
for _, object in --[[Location here]] do
object.ClickDetector.MouseClick:Connect(function()
-- ...
end)
end
Also, if you want the server to detect that the player wants to pick up an item, you should set up the ClickDetector.MouseClick event on the server and update the player’s leaderstats there, too, then you call RemoteEvent:FireClient() and tell the player to play the sound and enable the necessary GUI.
In case you’re thinking, “The functions connected to my ClickDetector.MouseClick event listeners are not executing the exact same code.” I see that. But the differences are minor.
Let’s say we wanted to create three balls, but with different Names, BrickColors and Sizes. Instead of creating three functions for each ball, we would set the function up with parameters, then, when creating the balls, we would pass the arguments relative to every ball:
local function createBall(name: string, brickColor: BrickColor, size: number)
local ball = Instance.new("Part")
ball.Shape = Enum.PartType.Ball
ball.Name = name
ball.BrickColor = brickColor
ball.Size = Vector3.one * size
ball.Parent = workspace
return ball
end
createBall("Basketball", BrickColor.new("Deep orange"), 4)
createBall("Dodgeball", BrickColor.new("Bright red"), 4)
createBall("Tennis Ball", BrickColor.new("New yeller"), 1)
If there are many parameters, you can also use arrays instead:
type ballData = {name: string, brickColor: brickColor, size: number}
local function createBall(ballData: ballData)
local ball = Instance.new("Part")
ball.Shape = Enum.PartType.Ball
ball.Name = ballData.name
ball.BrickColor = ballData.brickColor
ball.Size = Vector3.one * ballData.size
ball.Parent = workspace
return ball
end