Issue with MouseButton1Click firing twice?

[Panz0idz (ESPORTS3)]

Hi, so I’ve just ran into a problem with my client sending info to the server.

For some reason, the amount amount of players in the server equates to how many times the server is fired from the client.

Example:

Here, I dropped the money once, but it drops twice due to the client firing twice for some reason?

Client:

local Players = game:GetService("Players")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Events = ReplicatedStorage:WaitForChild("Events")
local LocalPlayer = Players.LocalPlayer
local Mouse = LocalPlayer:GetMouse()
local Money = LocalPlayer:WaitForChild("Money")
local Frame = script.Parent
local DropAmountBox = Frame:WaitForChild("DropAmount")
local DropButton = Frame:WaitForChild("DropButton")
local DropMoney = Events:WaitForChild("DropMoney")
local CloseButton = Frame:WaitForChild("CloseButton")
local ErrorMessage = Events:WaitForChild("ErrorMessage")

-- Settings

local DropTaxPercentage = 0.8 -- 0.1 = 10%

CloseButton.MouseButton1Click:Connect(function()
	Frame.Visible = false
end)


Frame.DropButton.MouseButton1Click:Connect(function()
	local DropAmount = tonumber(Frame.DropAmount.Text)
	if typeof(DropAmount) == "number" then
		print("Client-Fire")
		DropAmount = DropAmount * 0.8
		DropAmount = math.floor(DropAmount)
		Frame.Visible = false
		DropMoney:FireServer(tonumber(DropAmount)) -- This is where I fire the server
	else
		warn("DropMoney is not a valid number!")
		ErrorMessage:FireServer("Error","Not a valid number!")
	end
end)


DropMoney.OnClientEvent:Connect(function()
	Frame.Visible = true
end)

Server:

local Players = game:GetService("Players")
local Debris = game:GetService("Debris")
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local ServerStorage = game:GetService("ServerStorage")
local Events = ReplicatedStorage.Events
local Tool = script.Parent
local Handle = Tool.Handle
local MoneyGui = Handle.MoneyGui
local MoneyLabel = MoneyGui.MoneyLabel
local DropMoney = Events.DropMoney
local ServerErrorMessage = Events.ServerErrorMessage

-- Settings

local MoneyLifetime = 300 -- 5 minutes

local formatNumber = (function (n)
	n = tostring(n)
	return n:reverse():gsub("%d%d%d", "%1,"):reverse():gsub("^,", "")
end)


Tool.Equipped:Connect(function()
	local Character = Tool.Parent
	local Player = Players:GetPlayerFromCharacter(Character)
	local Money = Player.Money
	
	MoneyLabel.Text = formatNumber(tostring(Money.Value)).."$"
end)

Tool.Activated:Connect(function()
	local Character = Tool.Parent
	local Player = Players:GetPlayerFromCharacter(Character)
	local Money = Player.Money
	DropMoney:FireClient(Player)
end)

DropMoney.OnServerEvent:Connect(function(Player, DropAmount)
	print("Fire")
	local Money = Player.Money.Value
	local Character = Player.Character or Player.CharacterAdded:Wait()
	local moneymodules = ServerStorage.MoneyModules
	local moneybag_min = 2500
	local DropAmount = math.max(0,DropAmount)
	DropAmount = DropAmount
	if DropAmount == 0 then
		ServerErrorMessage:Fire(Player, "Error","Number cannot be negative!")
		return
	end
	if DropAmount >= moneybag_min and Money > DropAmount and Money > 0 then
		local moneyDebounce = false
		-- Moneybag B)
		print("moneybag", tostring(DropAmount).."$")
		MoneyLabel.Text = formatNumber(Player.Money.Value)
		local moneybag = moneymodules.Moneybag:Clone()
		moneybag.Parent = workspace
		moneybag.CFrame = Character.HumanoidRootPart.CFrame * CFrame.new(0,0,-5)
		Player.Money.Value -= DropAmount
		moneybag.MoneyGui.MoneyLabel.Text = formatNumber(DropAmount).."$"
		Debris:AddItem(moneybag, MoneyLifetime)
		moneybag.Touched:Connect(function(hitObject)
			pcall(function()
			if hitObject.Parent.Humanoid and not moneyDebounce then
					moneyDebounce = true
					local Character = hitObject.Parent
					local Player = Players:GetPlayerFromCharacter(Character)
					Player.Money.Value += DropAmount
					moneybag.MoneyPickup:Play()
					moneybag.MoneyPickup.Ended:Wait()
					moneybag:Destroy()
					moneyDebounce = false
				end
			end)
		end)
	elseif DropAmount < moneybag_min and Money > DropAmount and Money > 0 then
		local moneyDebounce = false
		-- Money
		print("money", tostring(DropAmount).."$")
		local money = moneymodules.Money:Clone()
		money.Parent = workspace
		money.CFrame = Character.HumanoidRootPart.CFrame * CFrame.new(0,0,-5)
		Player.Money.Value -= DropAmount
		money.MoneyGui.MoneyLabel.Text = formatNumber(DropAmount).."$"
		Debris:AddItem(money, MoneyLifetime)
		money.Touched:Connect(function(hitObject)
			pcall(function()
				if hitObject.Parent.Humanoid and not moneyDebounce then
					moneyDebounce = true
					local Character = hitObject.Parent
					local Player = Players:GetPlayerFromCharacter(Character)
					Player.Money.Value += DropAmount
					money.MoneyPickup:Play()
					money.MoneyPickup.Ended:Wait()
					money:Destroy()
					moneyDebounce = false
				end
			end)
		end)
	end
end)

Any help would be appreciated, thanks!

[DudusJestem]

The DropMoney remote’s OnServerEvent event is being connected to X (which is the players amount) amount of times because Its in duplicate scripts, I asssume that the server script is in a tool that is being cloned to every player which is why it fires multiple times. BTW you should not connect anything in events which run multiple times (like you did withe the moneybag.Touched event) because it may cause memory issues

[Blakary]

is the script on the client or server?

[Panz0idz (ESPORTS3)]

The one on the top is the ClientScript and the one on the bottom is the Serverside script.

[Blakary]

Ok well, I’ve had issues like these before, and I think the issue is the script in StarterPlayerScripts fires with the script in PlayerScripts, so disable the scripts in StarterPlayerScripts, and use another script to enable them if they’re in PlayerScripts. Here’s a simple one I made:

script.Parent.ChildAdded:Connect(function(inst)
	if inst.Parent ~= game.StarterPlayer.StarterPlayerScripts then
		if inst.ClassName == "Script" or inst.ClassName == "LocalScript" then
			inst.Enabled = true
			print("enabled "..inst.Name)
		end
	end
end)

This script, when inserted into PlayerScripts, will check if any scripts are added to it. It also checks if the parent is StarterPlayerScripts, to prevent the scripts from getting enabled outside of the player. The way this works is when it’s inserted into PlayerScripts, it will fire on any child added to it, and if the child is actually a script or local script, it will enable it.

I hope this helps!

[Panz0idz (ESPORTS3)]

This is most-likely the solution, but one thing, how would I connect every wallet from this server script? Because parenting it to the tool, I can just use script.Parent to reference it.

An alternative way would be using a for loop I think? To loop through every player and check for their wallets. But is that an efficient and reliable way? If not, what is the best way?

[DudusJestem]

A server script in the tool is very unnecessary imo, you should just make one server script in the server script service which just captures a remote in replicated storage (which every client has access to) being fired from the tool, then just evaluates (and does sanity checks on the client and server) the amount of money passed as argument with te :FireServer(moneyAmount) method and that’s all

[Panz0idz (ESPORTS3)]

Makes sense, thanks for the solution. One last thing, could you link me anything related or to any source about the the memory issue with Touched events, or how to fix it?

[DudusJestem]

Np, the memory issue is not with just the touched events, its with basically all of the RBXScriptSignals because you are using it wrong by connecting to the event multiple times while other event fires which just duplicates over time and clogs the memory