Math.Random giving more than it's supposed to

Hello.
Today I was trying to fix a few bugs, which got fixed by itself when afterwards I found a new bug.

Basically when you pick up a coin, it gives you a random amount between 5 and 20. But as I was picking them up and sometimes I would get 25, 58, 42, and all of these numbers above 20 and I don’t know why. I looked at the script and it was the same as I had left it a couple weeks ago so I have no idea what is causing this.

Here’s the script (I think I got the script from a tutorial a few years back and I have just always used this one since)

waittime = 5 -- Time Between each hit
amnt = math.random(5, 20)
function onTouched(part)
    local h = part.Parent:findFirstChild("Humanoid")
    if (h~=nil) then
        local thisplr = game.Players:findFirstChild(h.Parent.Name)
        if (thisplr~=nil) then
            local leaderstatsstats = thisplr:findFirstChild("leaderstats")
            if (leaderstatsstats~=nil) then
                local coins = leaderstatsstats:findFirstChild("Coins")
                if (coins~=nil) then
                    coins.Value = coins.Value + amnt
                end
            end
        end
    end
end

-- Ignore everything under here, that is for respawning the coin

local SS = game:GetService("ServerStorage")

function gen()
	local part = SS.CoinsWRLD1.Coin:Clone()
	part.Parent = workspace
	local x = math.random(130,366) 
	local z = math.random(-156,162) 
	part.Position = Vector3.new(x,1.5,z)
end

script.Parent.Touched:Connect(function(hit)
	local player = game:GetService("Players"):GetPlayerFromCharacter(hit.Parent)
	if player then gen()
		script.Parent:Destroy()
	end
end)

script.Parent.Touched:connect(onTouched)

If anyone could help that would be great. (:

Maybe you can try to use debounce here? (not sure tho)

1 Like

I agree with @aliaun125, it’s probably just firing the onTouched multiple times in rapid succession.

You could put a breakpoint in your code and step through it line by line to see for sure.

1 Like

You can try disconnecting the .Touched event so that it only fires once.

Also, why do you have two different .Touched events? You should merge them together and use only one event.

Here is your revised code. I don’t know how your game works, so please edit appropriately. To let activate multiple times, add a debounce = false after they pick up the coin so it can be picked up multiple times. I am not sure how your game works again so I do not know if this is applicable. It should fix your math.random problem though. Happy to help - CyberWizard

local debounce = false
waittime = 5 -- Time Between each hit
amnt = math.random(5, 20)
function onTouched(part)
    local h = part.Parent:findFirstChild("Humanoid")
    if (h~=nil) then
        local thisplr = game.Players:findFirstChild(h.Parent.Name)
        if (thisplr~=nil) then
            local leaderstatsstats = thisplr:findFirstChild("leaderstats")
            if (leaderstatsstats~=nil) then
                local coins = leaderstatsstats:findFirstChild("Coins")
                if (coins~=nil) then
                    coins.Value = coins.Value + amnt
                end
            end
        end
    end
end

-- Ignore everything under here, that is for respawning the coin

local SS = game:GetService("ServerStorage")

function gen()
	local part = SS.CoinsWRLD1.Coin:Clone()
	part.Parent = workspace
	local x = math.random(130,366) 
	local z = math.random(-156,162) 
	part.Position = Vector3.new(x,1.5,z)
end

script.Parent.Touched:Connect(function(hit)
if debounce == false then
debounce = true
onTouched(hit)
	local player = game:GetService("Players"):GetPlayerFromCharacter(hit.Parent)
	if player then gen()
		script.Parent:Destroy()
	end
end
end)


1 Like

It did not work. ):

Here is how the script is supposed to work:

local debounce = false
waittime = 5 -- Time Between each hit
amnt = math.random(5, 20)
function onTouched(part)
    local h = part.Parent:findFirstChild("Humanoid")
    if (h~=nil) then
        local thisplr = game.Players:findFirstChild(h.Parent.Name)
        if (thisplr~=nil) then
            local leaderstatsstats = thisplr:findFirstChild("leaderstats")
            if (leaderstatsstats~=nil) then
                local coins = leaderstatsstats:findFirstChild("Coins")
                if (coins~=nil) then
                    coins.Value = coins.Value + amnt
                end
            end
        end
    end
end

This first part is obviously the part that detects when it is touched, and gives the player the coins.

local SS = game:GetService("ServerStorage")

function gen()
	local part = SS.CoinsWRLD1.Coin:Clone()
	part.Parent = workspace
	local x = math.random(130,366) 
	local z = math.random(-156,162) 
	part.Position = Vector3.new(x,1.5,z)
end

script.Parent.Touched:Connect(function(hit)
	debounce = true
	onTouched(hit)
	local player = game:GetService("Players"):GetPlayerFromCharacter(hit.Parent)
	if player then gen()
		script.Parent:Destroy()
	end
end)

And this part is where the coin gets destroyed and respawns randomly within the place, I added this to the script because I was told it would be better to do so and it fixed an old bug, but I did it when I was still having this issue with the math.random. (didn’t know if that was the right spot to put it but it worked so I left it there, I don’t believe it has anything to do with this though…)

Edit: its connected to a different script as well, but as I said, this was happening before I put them together so it should be irrelevant.

You didn’t check if the debounce was false before making it true

The debounce is not implemented.
Add this line as the 1st line of your onTouched function:
if debounce then return end
then add these lines at the end of the if coins ~= nil statement:
debounce = true
task.wait(waittime)
debounce = false

1 Like

now the script doesn’t work at all, the coin just gets deleted.

What exactly about the coin?
This is the script I was referring to:

Instead of giving you the random amount of coins when you touch it it doesn’t give you anything. Though it does get deleted and the rest of the script works just fine. (if that’s what you’re asking)

What does your code look like now? Also, try changing if debounce then return end to if debounce==true then return end

local debounce = false
waittime = 5 -- Time Between each hit
amnt = math.random(5, 20)
function onTouched(part)
	if debounce then return end
	local h = part.Parent:findFirstChild("Humanoid")
	if (h~=nil) then
		local thisplr = game.Players:findFirstChild(h.Parent.Name)
		if (thisplr~=nil) then
			local leaderstatsstats = thisplr:findFirstChild("leaderstats")
			if (leaderstatsstats~=nil) then
				local coins = leaderstatsstats:findFirstChild("Coins")
				if (coins~=nil) then
					coins.Value = coins.Value + amnt
					debounce = true
					task.wait(waittime)
					debounce = false
				end
			end
		end
	end
end

-- Ignore everything under here, that is for respawning the coin

local SS = game:GetService("ServerStorage")

function gen()
	local part = SS.CoinsWRLD1.Coin:Clone()
	part.Parent = workspace
	local x = math.random(130,366) 
	local z = math.random(-156,162) 
	part.Position = Vector3.new(x,1.5,z)
end

script.Parent.Touched:Connect(function(hit)
	debounce = true
	onTouched(hit)
	local player = game:GetService("Players"):GetPlayerFromCharacter(hit.Parent)
	if player then gen()
		script.Parent:Destroy()
	end
end)

Will try that right now!

still doesnt work

Weird. Your coding style seems a bit messy. I recommend using FindFirstChild instead of findFirstChild as that one is deprecated.

I think I found the solution. What the others have been saying is correct. The reason why the coin value was incrementing more than 20 at a time was because it was running more than once before the coin was destroyed.

However, the reason why the other solutions are not working is because the coin is getting destroyed before the other touch function that handles adding coins runs. So, my solution is to add the two touch functions into one and add a debounce (a bool value) to keep it from running more than once.

Here’s the code (I updated it to more of my style to help me understand the code better)

local waittime = 5 -- Time Between each hit
local amnt = math.random(5, 20)
local hit = false

function onTouched(part)
	local humanoid = part.Parent:FindFirstChild("Humanoid")
	if humanoid then
		local thisplr = game.Players:GetPlayerFromCharacter(part.Parent)
		if thisplr then
			local leaderstatsstats = thisplr:FindFirstChild("leaderstats")
			if leaderstatsstats then
				local coins = leaderstatsstats:FindFirstChild("Coins")
				if coins and not hit then
					hit = true
					coins.Value = coins.Value + amnt
					gen()
					script.Parent:Destroy()
				end
			end
		end
	end
end

-- Ignore everything under here, that is for respawning the coin

local SS = game:GetService("ServerStorage")

function gen()
	local part = SS.CoinsWRLD1.Coin:Clone()
	part.Parent = workspace
	local x = math.random(130,366) 
	local z = math.random(-156,162) 
	part.Position = Vector3.new(x,1.5,z)
end

script.Parent.Touched:connect(onTouched)

I hope this helps! Let me know if it doesn’t work for you.

local debounce = false
waittime = 5 -- Time Between each hit
amnt = math.random(5, 20)
function onTouched(part)
	if debounce then return end
        debounce = true
	local h = part.Parent:FindFirstChild("Humanoid")
	if (h~=nil) then
		local thisplr = game.Players:FindFirstChild(h.Parent.Name)
		if (thisplr~=nil) then
			local leaderstatsstats = thisplr:FindFirstChild("leaderstats")
			if (leaderstatsstats~=nil) then
				local coins = leaderstatsstats:FindFirstChild("Coins")
				if (coins~=nil) then
                                        amnt = math.random(5, 20)
					coins.Value = coins.Value + amnt
				end
			end
		end
	end
        wait(0.05)
        debounce = false
end

[/quote]

Here is how I did it.

Hmm… just tested it and it still seems so be giving more than the math.random…

Weird, works on my test case. By chance are there two places where this script exists, like one coin that starts in the workspace and one in the coin in ServerStorage? I’ll look more into possible reasons this could be happening.

Try this:

--//Services
local Players = game:GetService("Players")
local ServerStorage = game:GetService("ServerStorage")

--//Variables
local Part = script.Parent

--//Controls
local waitTime = 5
local amount = Random.new():NextInteger(5, 20)

--//Functions
local function gen()
	local part = ServerStorage.CoinsWRLD1.Coin:Clone()
	part.Position = Vector3.new(Random.new():NextInteger(130, 366), 1.5, Random.new():NextInteger(-156, 162))
	part.Parent = workspace
	
	part.Touched:Connect(function(hit)
		local Player = Players:GetPlayerFromCharacter(hit.Parent)

		if Player then
			part:Destroy()

			Player.leaderstats.Coins.Value += amount
			gen()
		end
	end)
end

Part.Touched:Connect(function(hit)
	local Player = Players:GetPlayerFromCharacter(hit.Parent)
	
	if Player then
		Part:Destroy()
		
		Player.leaderstats.Coins.Value += amount
		gen()
	end
end)

My bad. I forgot to check if debounce == false. I corrected it now. Tell me if it still does not work.