Whenever I buy a sword, every single sword available is added to my inventory, please help

Help and Feedback Scripting Support
#1

If I buy a stone sword, it doesn’t just buy the stone sword, it also buys the iron sword and the stone sword again. If I buy the iron sword after the stone sword, it buys the iron sword twice and the stone sword twice. Also, the coin stat changes by 500 for the stone sword but then resets. I hope you can help me with this very confusing issue.

Server:

-- Services
local RS = game:GetService("ReplicatedStorage")
local PlayerS = game:GetService("Players")
local SS = game:GetService("ServerStorage")

-- Remote Event
local ShopEvent = RS:WaitForChild("ShopEvent")
local ToolGiven = RS:WaitForChild("ToolGiven")

-- Tools
local tools = SS:WaitForChild("Tools")

local function onEventRecieved(player,descendant)

if descendant:IsA("ImageButton") then
		local Coins = player.Coin_Stats.Coins.Value
		if Coins >= descendant.Price.Value then
			Coins -= descendant.Price.Value
			print(player.Name .. " has $" .. Coins .. " left")
			print(descendant.Name .." has been bought for $".. descendant.Price.Value .." by ".. player.DisplayName .." (@" .. player.Name .. ")")
		end
			local toolGiven = descendant
			
			local toolClone = toolGiven:Clone()
			
			toolClone.Parent = RS
			
			local changeP = toolClone:FindFirstChildOfClass("Tool")
			changeP.Parent = player.Backpack
			
			toolClone:Destroy()
			
			task.wait(1)
			
			else
				warn("Requested tool is not a tool")
			end
		end

ShopEvent.OnServerEvent:Connect(onEventRecieved)

Client:

-- Services
local RS = game:GetService("ReplicatedStorage")
local PlayerS = game:GetService("Players")

-- Coins
local Coins = PlayerS.LocalPlayer.Coin_Stats.Coins.Value

-- Remote Event
local ShopEvent = RS:WaitForChild("ShopEvent")
local ToolGiven = RS:WaitForChild("ToolGiven")

-- Player
local player = PlayerS.LocalPlayer
local bag = player.Backpack

-- GUI
local gui = script.Parent
local shop = gui.Shop
local desc = shop.ItemDescription
local toolSF = shop.ToolsScrollingFrame
local buyB = desc.BuyButton
local closeB = shop.CloseButtonF.CloseButton
local toolT = {}

-- Item Description content
local itemName = desc.ItemName
local itemDmg = desc.Damage
local itemPrice = desc.Price
local itemAbility = desc.Ability
local itemImg = desc.ItemImage

local function deleteValues()
	itemName.Text = "No Item Selected"
	itemDmg.Text = "Damage: N/A"
	itemPrice.Text =  "Price: N/A"
	itemAbility.Text = "Ability: N/A"
	itemImg.Image = ""
end

table.insert(toolT, toolSF:GetChildren())

for i, descendant in pairs(toolSF:GetDescendants()) do 
	if descendant:IsA("ImageButton") then
		print(descendant.Name)
		descendant.MouseButton1Click:Connect(function()
			print(descendant.Name .. " has been clicked")
			itemName.Text = descendant.ItemName.Value
			itemDmg.Text = "Damage: ".. descendant.Damage.Value
			itemPrice.Text = "Price: $".. descendant.Price.Value
			itemAbility.Text = "Ability: ".. descendant.Ability.Value
			itemImg.Image = descendant:FindFirstChildWhichIsA("Tool").TextureId
			buyB.MouseButton1Click:Connect(function()
				print(descendant.Name .. " has been bought")
				ShopEvent:FireServer(descendant)
				task.wait(1)
			end)
		end)
	end
end

closeB.MouseButton1Click:Connect(deleteValues)

Each tool button:
image
image

Thanks!

#2

You are connecting buy button every time you click any item in the shop. Move the buy button connection out of the loop and have a variable that holds which item is selected and send that variable with the event instead.

#3

Okay, so I have to move the buy button out of the for loop, and make a variable to fill in for which item is selected?

#4

Could I just use and object value to select which object is currently selected and use remote events to send a message to the server to set the object value?

#5

Yes, but the best way to do the shop usually is to just send the name of the item or the id of it.

#6

By sending the name, do you mean through the remote event?

#7 
ShopEvent:FireServer(descendant)

“descendant” is an instance, so it can’t be sent through remote events. You can send other things like the name of the descendant so you can find it on the server.

Also, in the server script, the tool is being given even if the player does not have enough coins.

for i, descendant in pairs(toolSF:GetDescendants()) do 
	if descendant:IsA("ImageButton") then
		print(descendant.Name)
		descendant.MouseButton1Click:Connect(function()
			print(descendant.Name .. " has been clicked")
			itemName.Text = descendant.ItemName.Value
			itemDmg.Text = "Damage: ".. descendant.Damage.Value
			itemPrice.Text = "Price: $".. descendant.Price.Value
			itemAbility.Text = "Ability: ".. descendant.Ability.Value
			itemImg.Image = descendant:FindFirstChildWhichIsA("Tool").TextureId
			buyB.MouseButton1Click:Connect(function()
				print(descendant.Name .. " has been bought")
				ShopEvent:FireServer(descendant)
				task.wait(1)
			end)
		end)
	end
end

Another thing is that you are listening for the mouse button click every time the descendant is pressed. This means that, on the first click, it will activate once, and on the second click, it will activate twice. It might actually activate way more than that because all of that is in a for loop.
Instead, you can make a new variable that keeps track of the selected descendant. Change this variable when the descendant is pressed, and create only one event listener outside of the for loop that sends the selected descendant to the server when buyB is pressed.

#8

Yes, basically on the client you would have just the images of the items you are selling, and when the player buys an item, they send the name (or id) of the item with remote event, server then checks if the item they are buying exists and if they have money for it, then clones the item from server storage (preferably) and gives it to the player.

#9

Alright I’ll try that, thanks for helping!

#10

Thanks for that, I tweaked it a bit and now it only buys once!

#11

This topic was automatically closed 14 days after the last reply. New replies are no longer allowed.