How can I improve my Shop purchase script further, utilizing Remote Events efficiently?


#1

Hello!

  • What are you attempting to achieve? I’m looking to improve my Purchase system to put more of the process on the server-side than the client-end to ensure that bad player experiences - such as having a purchase put your Coins in the negatives, does not occur.
  • What is the issue? I’m a bit shaky on my understanding of Remote Events, and am unsure if I’m using them in the most efficient way.
  • What solutions have you tried so far? (Have you searched for solutions through the Roblox Wiki yet?) I have indeed used the Wiki, and have edited my script many times as my understanding has grown. I’ve considered making a unique Remote Event for every single purchase, but I have over 700+ items and would like to avoid that many Remote Events if there’s a better way to do this,

So far, the script I have is working just fine, but I want to know if I can make it better to improve player experience and prevent potential bad things from happening - double purchases, overcharged, reduce the ability to exploit as much as possible.

Here are my Remote Events inside the Script under ServerScriptStorage:

--//Get Services
local ReplicatedStorage = game:GetService("ReplicatedStorage")
--//EventFolder
local Events = ReplicatedStorage:WaitForChild("Events")
--//Remote Events
local CoinsPurchase = Events:WaitForChild("CoinsPurchase")
local ItemOwned = Events:WaitForChild("ItemOwned")
local CanAffordItem = Events:WaitForChild("CanAffordItem")

 -- PURCHASE WITH COINS --
CoinsPurchase.OnServerEvent:Connect(function(Player, Value, Amount)
    if Player.leaderstats.Coins.Value >= Amount then
	          print('Server says player has enough Coins for the purchase.')
		Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - Amount 
              print(Value)
        Value.Value = true
    else
        print("Player does not have enough Coins for the purchase.")
    end
end)

--CHECK IF ITEM OWNED --
ItemOwned.OnServerEvent:Connect(function(Player,Value)
     if Value.Value == false then
	     print('Player does not own this item.')
     else
	     print('Player does own item')
	 end
	
end)

--CHECK IF PLAYER CAN AFFORD ITEM --
CanAffordItem.OnServerEvent:Connect(function(Player,Value, Amount)
     if Player.leaderstats.Coins.Value >= Amount then
	   print('Player CAN afford Item')
     else
	   print('Player Can NOT afford Item')
     end
	
end)

And this is a slightly abbreviated version of my shop purchase Local Script inside the Shop Gui. When the Item01 button is clicked, lines on that at the end of the script, it makes visible the BuyButton script and fills in the variables, specifically the Cost and the Value:

--//Get Services
local RS = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")
local Player = Players.LocalPlayer
local currencyCOINS = Player.leaderstats.Coins

--//EventFolder
local Events = RS:WaitForChild("Events")

--//Events
local ToolPurchase = Events:WaitForChild("ToolPurchase")
local CoinsPurchase = Events:WaitForChild("CoinsPurchase")
local ItemOwned = Events:WaitForChild("ItemOwned")
local CanAffordItem = Events:WaitForChild("CanAffordItem")

--//SHOP FRAME
local PurchaseFrame = Frame.PurchaseFrame
local BuyButton = PurchaseFrame.BuyButton
local ItemDesc = PurchaseFrame.ItemDesc
local Price = PurchaseFrame.Price

local ItemTrackName = RS:WaitForChild("ItemTrackNameShop")
local ItemPurchased = RS:WaitForChild("ItemPurchased")
local ItemPriceAmount = RS:WaitForChild("ItemPriceAmount")


BuyButton.MouseButton1Click:Connect(function()
    MenuClickSound:Play()
	local amount = Cost
	local Value = ItemPurchased.Value
	ItemDesc.Text = 'Purchase '..ItemTrackName.Value..'?'
	Price.Text = ItemPriceAmount.Value

ItemOwned:FireServer(Player.Inventory[Value]) -- Checks if the Item is owned.
CanAffordItem:FireServer(Player.Inventory[Value], amount) -- Checks if the Player can afford the Item.	
	
    if Player.Inventory[Value].Value == true then
			ItemDesc.Text = 'You have already purchased this Item!'
			wait(3)
    else
	
		if Player.leaderstats.Coins.Value >= amount then
			CoinsPurchase:FireServer(Player.Inventory[Value], amount)
			wait(1)
			PurchaseClickSound:Play()
			ItemDesc.Text = 'Purchase Complete for '..ItemTrackName.Value..'! Equip this Item from your Inventory!'
			wait(3)		
			PurchaseFrame.Visible = false
			ItemDesc.Text = 'Purchase '..ItemTrackName.Value..'?'
			wait(1)
		else
			local coinsrequired = amount - currencyCOINS.Value		
			ItemDesc.Text = 'You need '.. coinsrequired ..' more Coins to buy this item!'
			wait(2)
			ItemDesc.Text = 'Purchase '..ItemTrackName.Value..'?'
			wait(5)
			PurchaseFrame.Visible = false
		end
	end
end)--Closes Purchase Frame

Item01.MouseButton1Click:Connect(function() 
     PurchaseFrame.Visible =  true 
     ItemTrackName.Value = 'Item01' 
     ItemDesc.Text = 'Purchase Item01?'
     ItemPurchased.Value = 'Item01' 
     Cost = 10 
     ItemPriceAmount.Value =  Cost 
end)		

--Additional Items: Item02, Item03, etc. that when their respective button is pressed, will change the variables in the BuyButton script above.

As you can see in the shop purchase script above, I first run the Remote Events to see if the Player already owns the item, and then if they can afford the item, before following it up with the local/client determining the same thing, and then the Remote Event fires that actually grants the Item, and deducts the amount/cost of the Item.

Am I using the Remote Events correctly here? Can I improve upon it further?

Thank you!


#2

You are technically using them correctly, but you have a massive security hole.

CoinsPurchase.OnServerEvent:Connect(function(Player, Value, Amount)

With this line, it is possible to pass in a bad amount, like the incorrect number or a negative number. In this case, you are trusting the client not to send incorrect amounts for what they owe. Instead, you should pass in just the value and have the server determine the cost.

I would also recommend adding a client side coin validation, rather than pinging the server. Some users will notice the 50ms-100ms delay when trying to purchase an item they know they can’t afford. It also will decrease server load. Obviously, you need to keep the server check since exploiters can bypass the client check.


#3

Thanks for the info!

How would I go about with the server determining the cost check? Would I make adjust that Remote Event like so:

 -- PURCHASE WITH COINS --
CoinsPurchase.OnServerEvent:Connect(function(Player, Value, Amount)
    if Player.leaderstats.Coins.Value >= Amount then
	          print('Server says player has enough Coins for the purchase.')
		Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - 100 --Instead of Amount?
              print(Value)
        Value.Value = true
    else
        print("Player does not have enough Coins for the purchase.")
    end
end)

Would I then need to use multiple Remote Events for the different item being purchased?

I’m not sure I follow the coin validation you reference, would that be a line in my local script to ensure the Shop Gui is open?

BuyButton.MouseButton1Click:Connect(function()
    MenuClickSound:Play()
	local amount = Cost
	local Value = ItemPurchased.Value
	ItemDesc.Text = 'Purchase '..ItemTrackName.Value..'?'
	Price.Text = ItemPriceAmount.Value

if Player.ShopGui then --

ItemOwned:FireServer(Player.Inventory[Value]) -- Checks if the Item is owned.
CanAffordItem:FireServer(Player.Inventory[Value], amount) -- Checks if the Player can afford the Item.	
	
    if Player.Inventory[Value].Value == true then
--etc.

		end
	end
end -- Added for the If Player.ShopGui line
end)--Closes Purchase Frame

Thank you!!


#4

In this case, the following line:

CoinsPurchase:FireServer(Player.Inventory[Value], amount)

would be changed to:

CoinsPurchase:FireServer(Player.Inventory[Value])

The server function would need some work, but here is the idea:

local function DetermineCost(Value)
	--TODO: Return cost
end

CoinsPurchase.OnServerEvent:Connect(function(Player, Value)
    Amount = DetermineCost(Value)

    if Player.leaderstats.Coins.Value >= Amount then
	          print('Server says player has enough Coins for the purchase.')
		Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - 100 --Instead of Amount?
              print(Value)
        Value.Value = true
    else
        print("Player does not have enough Coins for the purchase.")
    end
end)

Also, you may want to look into RemoteFunctions so they can return something, like if the purchase was successful or not.


#5

In short, use a Module Script to Store Item Data.

:FireServer(‘BuyItem’,ItemName)

that’s all you need.

and check everything on the server

you can also check it in the client too just so that the server isn’t bothered all the time cause people can spam your remote.

But you must always check on the server no matter what.

You can also use ONLY one RemoteEvent for the entire game too and use Tables to store functions inside your Script.


#6

If your script works but you want to improve on it, hop this thread over to the new category, #development-support:requests-for-code-feedback. The Development Support categories are for if you need help fixing bugged code.

You can move the thread by clicking the pencil beside the title.


#7

Thank you for this! I’m getting the idea, but botching the execution. Here’s what I did:

local function DetermineCost(Value)
	   if Value.Value == Item01 then return "100"
       elseif Value.Value == Item02 then return "125"
          --etc Item03 through Item799
       elseif Value.Value == Item800 then return "350"
       end        
end

On the line:

if Player.leaderstats.Coins.Value >= Amount then

I receive the following error marked below: attempt to compare nil with number

CoinsPurchase.OnServerEvent:Connect(function(Player, Value)
  Amount = DetermineCost(Value)
        print(Amount) -- which is printing out as 'nil'.
    if Player.leaderstats.Coins.Value >= Amount then --Error's on this line: 'attempt to compare nil with number'
	    print('Server says player has enough Coins for the purchase.')
		Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - Amountthe player
        print(Value)
        Value.Value = true
    else
        print("Player does not have enough Coins for the purchase.")
    end

end)

I’m certain I’m messing up the formatting DetermineCost function. >.<


#8

That error means the DetermineCost function returned nil.

It must mean Value.Value is never equal to any of the variables you are comparing it against in each of your conditions. Could this be because you’re comparing Value.Value with a variable, are you meant to be comparing it with a string?
Currently it’s Value.Value == Item01
Should it be Value.Value == "Item01"?

Another thing I notice is that you’re returning a string value instead of a number, remove the quotation marks around the number in each of the return statements, otherwise you’ll get another error for trying to compare a number with a string.
Currently it’s return "350"
It should be return 350

Hopefully this helps.


#9

Thanks for the response! I’m still getting a nil value though and the script breaks with the “attempt to compare nil with number” on the “if Player.leaderstats.Coins.Value >= Amount then” line

I edited it to this:

local function DetermineCost(Value)
	print(Value)
	  if Value.Value == "Item01" then return 100
      elseif Value.Value == "Item02" then return 125
      --etc Item03 through Item799
      elseif Value.Value == "Item800" then return 350
	  end
	  print(Value)
end

The Prints are showing the correct value of “Item01”, or whichever Item I’ve clicked on, but the other print for the RemoteEvent for the actual purchase continues to print out “nil”.

CoinsPurchase.OnServerEvent:Connect(function(Player, Value)
Amount = DetermineCost(Value) --print says "nil" and the script breaks here."attempt to compare nil with number"
	print(Amount)
	if Player.leaderstats.Coins.Value >= Amount then
		print('Server says player has enough Coins for the purchase.')
		Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - Amount --Subtract money from the player
        print(Value)
        Value.Value = true
    else
    	print("Player does not have enough Coins for the purchase.")
	end

end)

Thoughts? :frowning:


#10

Just looked back on your code, and from what I can tell, you’re passing a BoolValue object as the parameter to this function, try comparing Value.Name like I have done so here:

local function DetermineCost(Value)
	print(Value)
	  if Value.Name == "Item01" then return 100
      elseif Value.Name == "Item02" then return 125
      --etc Item03 through Item799
      elseif Value.Name == "Item800" then return 350
	  end
	  print(Value)
end

#11

Thank you!! :smiley:
This appears to work! Your solution along with TheNexusAvenger has done it! :smiley: So excited! Now to add in all 800+ items, but it works!


#12

To add on to this. The golden rule is to never allow clients to hold / manage data. The client should be used just to show the data, like displaying the amount of coins that person has. The server will be dealing with interactions with that data.