Mystery/Random Box Purchase will sometimes randomly charge/grant more than what was purchased?

Help and Feedback Scripting Support
#1

Hello!

What are you attempting to achieve?
I have a “Mystery Crate” shop purchase system that when the button is clicked, will check if a player has enough coins, if so it will charge them and then choose a random number to determine what Item is being granted, and then will check if the player already has the Item, if the player already owns the Item it will grant another Item instead.

What is the issue?
Sometimes, randomly, it will charge multiple times for a single purchase, and award multiple items. This is undesirable as the player may only wish to purchase one item. But most of the time it works correctly, one click = one purchase/item granted.

What solutions have you tried so far? (Have you searched for solutions through the Roblox Wiki yet?)
I’ve re-configured my script a few times, but I don’t receive error outputs, and I can’t find a pattern for what triggers the multi-purchases.

Here is an abbreviated form of my script below, which is a local script inside a GUI:

--//Get Services
local ReplicatedStorage = game:GetService("ReplicatedStorage")
local Players = game:GetService("Players")
local Player = Players.localPlayer

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

--//Events 
-- This is the Remote Event which handles the purchases:
local UpgradesCoinsPurchase = Events:WaitForChild("UpgradesCoinsPurchase") 

item1_event = nil
item2_event = nil
--more events for more items.

math.randomseed(tick())

CRATESButton.MouseButton1Click:Connect(function()

    if item1_event then item1_event:disconnect() end
    
    item1_event = Item1.MouseButton1Click:Connect(function()

        local Item1Coinsbutton = script.Parent.Frame.CRATESFrame.Item1Frame.BuyCoins
        script.Parent.Frame.CRATESFrame.Item1Frame.CoinsPrice.Text  = 100
        
        Item1Coinsbutton.MouseButton1Click:Connect(function()

            local amount  = 100
            local descText = script.Parent.Frame.CRATESFrame.Item1Frame.DescriptionFrame.ItemDescription
            local random  =  math.random(1,200)
            local debounce = false

            if debounce == false and  Player.leaderstats.Coins.Value >= amount then
                debounce = true
                if random >= 1 and random <= 10 then 
                    local Item01 =  Player.Inventory.HasItem01Blue
                    if Item01.Value  ==  false then
                        UpgradesCoinsPurchase:FireServer(Item01, amount)
                        descText.Text = 'You received  Item01Blue! Check your Inventory!'
                    elseif Item01.Value  ==  true then
                        descText.Text = 'You received  Item01Blue! You already own this item. Try again!'
                    end
                    wait(3)
                elseif  random >=  11 and random <=  20 then 
                    local Item01 = Player.Inventory.HasItem01Green
                    if Item01.Value == false then
                        UpgradesCoinsPurchase:FireServer(Item01, amount)
                        descText.Text = 'You received  Item01Green! Check your Inventory!'
     	            elseif Item01.Value == true then
                        descText.Text = 'You received  Item01Green! You already own this item. Try again!'
                    end
                    wait(3)
                -- an additional 18 options below that mirror the above elseif statement except for the random numbers and the Item received being adjusted.
            
end
        end
    end)
end)
     
--repeat with additional Item events that mirror the above, except instead of item1_event its item2_event, etc.

This is my Remote Event script inside ServerScriptStorage:

--//Get Services
local ReplicatedStorage = game:GetService("ReplicatedStorage")

--//Event
local Events = ReplicatedStorage:WaitForChild("Events")
local UpgradesCoinsPurchase = Events:WaitForChild("UpgradesCoinsPurchase")
     
UpgradesCoinsPurchase.OnServerEvent:Connect(function(Player, Value, Amount)
    Player.leaderstats.Coins.Value = Player.leaderstats.Coins.Value - Amount --Subtract money from the player
    print(Value)
    Value.Value = true
end)

The print of the Value purchase does confirm that more than one item was granted and charged for.

Any insight or directions to go would be much appreciated. :slight_smile: Thanks!

1 Like
#2

I am having a difficult time posting that in studio, you may want to make sure that you are using [code] blocks so there is proper formatting. One thing I will say skimming over the code is you are heavily relying on the client to be truthful with requests (ex: not giving a negative amount to subtract in the remote event).

1 Like
#3

I can barely parse your script. I noticed this, though:

if Item01.Value == false then
    UpgradesCoinsPurchase:FireServer(Item01, amount)
    descText.Text = ‘You received Item01Blue! Check your Inventory!’
elseif Item01.Value == true then
    descText.Text = ‘You received Item01Blue! You already own this item. Try again!’
end

It seems like you’re checking if the player has already bought the item on the client but not on the server. I’m guessing that if the player clicks the button too fast, they’ll send multiple purchase requests and the server just processes it without checking that they’re valid first.

Besides that, right now, you’re putting too much trust on the client:

  • I could fire the UpgradesCoinsPurchase remoteevent with a negative Amount and give my self a lot of money.
  • I also could change the Value parameter to be any item I want.

You should probably start doing the following:

  • Generate the random numbers on the server.
  • Check if the player already owns the item on the server.
  • Check if the player has enough money on the server.
  • Instead of passing both the Value (item id) and the Amount (price) to the remote event, just pass the item id. The server can check the price itself.
1 Like
#4

Thank you for taking a look! :slight_smile: I’ll fix my post to be in code. I could certainly move more things to the server it appears, but as of right now, would anything in my random giver code, cause it to charge/award multiple times for a single press of the button? It doesn’t seem to matter if I’m clicking the Purchase button fast or slow, and its not often that it does, but randomly it will purchase/award multiple times for that single press.

I’m assuming the problem is happening here:

(...)
elseif random >= 11 and random <= 20 then
    local Item01 = Player.Inventory.HasItem01Green
    if Item01.Value == false then
        UpgradesCoinsPurchase:FireServer(Item01, amount)
        descText.Text = 'You received Item01Green! Check your Inventory!'
    elseif Item01.Value == true then
        descText.Text = 'You received Item01Green! You already own this item. Try again!'
    end
(...)
#5

Thank you for the response! I went back and edited the first post to be in code block, apologies for it not being so before.

I’ve tested it many times - the button click speed - it doesn’t seem to make a difference if I hit the button too fast or not, its just a random occurrence where it awards/charges multiple times for the single press. And it’s not awarding duplicate items, my purchase script there specifically ensures that does not happen:

 if  random >=  1 and random <=  10 then 
 local   Item01 =  Player.Inventory.HasItem01Blue
 	if Item01.Value  ==  false then
 	UpgradesCoinsPurchase:FireServer(Item01, amount)
 	 descText.Text  =  'You received  Item01Blue! Check your Inventory!'
 elseif Item01.Value  ==  true then
 	 descText.Text  =  'You received  Item01Blue! You already own this item. Try again!'
 end

I can certainly look at moving requests from the local to the server, this bit of code originates from my earliest scripting attempts, and I know just enough to know what I don’t know. :slight_smile: I can certainly try to add a few remote events to take the place of the processes going on in the local script there to see if it makes a difference.

Forgive my ignorance, how could you fire the remote event without pressing the purchase button, or change the Value parameter to anything you want? I’m unfamiliar with this workaround process. The way my Item giving system is set up is via BoolValues. A purchase is made, remote event makes the specific BoolValue inside the player’s folder to True, which makes an equip button in player’s inventory appear. Are you, the player, able to inject code through non-explotive ways to do the same?

Thank you again for your time and response!

#6

I’ve amended the formatting of the code in your first post here.

Please format your code in a readable way when posting in this category. This will make it a lot easier for people to understand your code and help you. A couple of examples to improve your formatting can be found below.

Do not use multiple consecutive spaces between operators and values like this: (click to open) 
descText.Text    =   '...'

Use a single space on both sides instead:

descText.Text = '...'

This also applies to guards:

if random  >=  11 then

Again, use a single space on both sides instead:

if random >= 11 then
Please also keep in mind your indentation at the start of the lines. Increase indentation when you start a new control structure, and decrease it when you end a control structure. (click to open) 
if ... then
    while ... do
        ...
    end
    if ... then
        ...
        if then
            ...
        end
    end
end
#7

Forgive my ignorance, how could you fire the remote event without pressing the purchase button, or change the Value parameter to anything you want?

Always assume that an exploiter can execute whatever code they want locally. This means that they can fire remote events with whatever values they want.

1 Like
#8

Thank you for that! Apologies for the unpleasant format, I personally don’t use indention when scripting, doesn’t bother me, but I can certainly see how most would rather not and I’ll be sure to do so if I post future help questions. :slight_smile:

#9

Since my game is Filtering Enabled, then the exploiter can only affect things on their end on the local side, right? I see your point though, wearing a raincoat to avoid the storm and such.Exploiters gonna exploit. :frowning:

But exploiting aside, do you see anything here that could cause the script to fire multiple times? It doesn’t always do so, and the speed of pressing the purchase button doesn’t seem to make a difference on when it occurs:

 if  random >=  1 and random <=  10 then 
 local   Item01 =  Player.Inventory.HasItem01Blue
 	if Item01.Value  ==  false then
 	       UpgradesCoinsPurchase:FireServer(Item01, amount)
 	       descText.Text  =  'You received  Item01Blue! Check your Inventory!'
    elseif Item01.Value  ==  true then
 	       descText.Text  =  'You received  Item01Blue! You already own this item. Try again!'
 end

It’s possible it’s happening elsewhere, but this would seem to be the most likely cause of where it triggers, that, or my remote event since those are the only scripts that are going at the time.

buildthomas was kind enough to edit the format of my original post to make it clearer to read too, if that helps? :slight_smile:

Thank you again for your assistance!

#10

This is a connection created every time Item1 is clicked. Is this ever disconnected?

1 Like
#11

If I’m understanding correctly, the connection should end on the second to last end) . I marked it in the script below:

  Item1Coinsbutton.MouseButton1Click:Connect(function()

            local amount  = 100
            local descText = script.Parent.Frame.CRATESFrame.Item1Frame.DescriptionFrame.ItemDescription
            local random  =  math.random(1,200)
            local debounce = false

            if debounce == false and  Player.leaderstats.Coins.Value >= amount then
                debounce = true
                if random >= 1 and random <= 10 then 
                    local Item01 =  Player.Inventory.HasItem01Blue
                    if Item01.Value  ==  false then
                        UpgradesCoinsPurchase:FireServer(Item01, amount)
                        descText.Text = 'You received  Item01Blue! Check your Inventory!'
                    elseif Item01.Value  ==  true then
                        descText.Text = 'You received  Item01Blue! You already own this item. Try again!'
                    end
                    wait(3)
      elseif  random >=  11 and random <=  20 then 
                        local Item01 = Player.Inventory.HasItem01Green
                        if Item01.Value == false then
                            UpgradesCoinsPurchase:FireServer(Item01, amount)
                            descText.Text = 'You received  Item01Green! Check your Inventory!'
         	            elseif Item01.Value == true then
                            descText.Text = 'You received  Item01Green! You already own this item. Try again!'
                        end
                        wait(3)
                 -- an additional 18 options below that mirror the above elseif statement except for the random numbers and the Item received being adjusted.
                    
                  end
                end
            end) -- MouseButton1Click:Connect ends here.
        end)

Is this what you’re referring to? or am I missing a line of code I should insert?

#12

I think your code is alright, just move everything to the Server and also Add a Debounce.

Never trust the client to do important stuff.

Only use the client to send a request to the server and let it do the rest.

Also a small tip,

if not Debounce then
– Code
end

if Debounce then
–Code
end

No need to do

X == true , X == false

1 Like
#13

You’re creating a new connection to Item1Coinsbutton.MouseButton1Click every time Item1 is clicked. You can make multiple connections to the same event.

Disconnect the old connection to Item1Coinsbutton.MouseButton1Click before creating a new one.

1 Like