Help with math.random()

This is my code. I’m trying to fire all clients every 60 seconds so there is a message in chat by the server. Error is

ServerScriptService.Stats:251: invalid argument #2 to 'random' (interval is empty)

local messages={
	"Give the game a thumbs up on the game page!",
	"Join the group OBBLOXIANS! to stay tuned.",
	"Explore this place, you might find something!",
	"Invite your friends and play together!",
	"Drink strange potion to get interesting results.",
	"Push the robots using Push tool!",
}
local m={}
while wait(60) do
	if m=={}then m=messages end
	local msg = m[math.random(1,#m)]
	R.SE:FireAllClients(msg)
	for i,v in pairs(m)do
		if v==msg then
			table.remove(m,i)
		end
	end
end

The error message basically implies that #m is either exactly 1 OR #m is nil

To fix this, simply replace your math.random() function with math.random(#m). Hope this helps! :wink:

Oh that is a thing? I was putting 1 when I didn’t even need to. Thanks lemme test.

1 Like

Yep! math.random() really only needs one arguement in your usage case, in a more specific situation where you absolutely have to set a minimal number, only then should you actually implement the first AND second arguement(s).

1 Like

Doesn’t fix the error though. m isn’t nil so idk why it shows #m to be nil. Printed this when i added print statement before random line. table: 0x47075f097527feea

I did a more in depth fix in my own studio and found that there were more issues than just the #m issue.

The issue here was that m was being set as your messages array and for some reason, m wouldnt actually be set to said table. Alternatively I used the next() function to determine if m was an empty table and it worked!

As for your second issue, since you’re actually setting m as your messages array you would eventually run out of values because you’re essentially still using the same table just under a different variable. To fix this I added a CloneTable function that would duplicate your array so you’d never run out of values!

Heres the improved code:

local messages = {
	"Give the game a thumbs up on the game page!",
	"Join the group OBBLOXIANS! to stay tuned.",
	"Explore this place, you might find something!",
	"Invite your friends and play together!",
	"Drink strange potion to get interesting results.",
	"Push the robots using Push tool!",
};

local m = {};

--// Functions
function CloneTable(Array) -- Returns a copy of X array
	local Clone = {};
	
	for Key, Value in pairs(Array) do
		Clone[Key] = Value
	end
	
	return Clone
end

--// Main
while task.wait(1) do
	if (not next(m)) then m = CloneTable(messages); end
	
	local Query = math.random(#m);
	local msg = m[Query];
	
	table.remove(m, Query);
	print(m, msg);
end

Hope this helped! :grinning_face_with_smiling_eyes:

I don’t understand what’s wrong with my code. m=messages should copy messages to m. when i printed it, it printed a table value though not the table itself.

The issue was pretty much just the way you were detecting if m was empty or not, I printed m and it was always shown as an empty table.

does m=={} doesn’t give true for m={} ?

Not exactly, your logic is right but the method itself isnt.

Tables are each given unique identifiers so than your scripts can identify them apart so with this being said, m and {} would both have different identifiers because they’re 2 seperate tables which are not the same.

PS: This is also why you cant compare tables with ==

What does next(m) return? roblox roblox

next(m) returns the first value it can find in your array, it wont return a boolean but it will still pass your if statement.

1 Like

yeah but

	if not next(m) then m=messages end

is not working. it doesn’t put m=messages for some reason that’s why after all strings have been removed it says invalid argument #1 to ‘random’ (interval is empty)

My guess is that it’s not detecting whether m is equal to {}. The easiest fix I THINK is to simply change m = messages

local messages = {
	"Give the game a thumbs up on the game page!",
	"Join the group OBBLOXIANS! to stay tuned.",
	"Explore this place, you might find something!",
	"Invite your friends and play together!",
	"Drink strange potion to get interesting results.",
	"Push the robots using Push tool!"
}
local m = messages
while wait(60) do
	if m=={}then m=messages end
	local msg = m[math.random(1,#m)]
	R.SE:FireAllClients(msg)
	for i,v in pairs(m)do
		if v==msg then
			table.remove(m,i)
		end
	end
end

You also had a comma at the end of the final message (I don’t know if that would change anything since I haven’t tested it), but it’s better if its not there as it’s indicating that it’s the last message.

I think it’s better asked: what are you trying to do? Both the original code and the proposed solutions seem strangely convoluted for something that looks simple.

If I had to wager a guess based on the existing code and proposed solutions, you’re trying to make a random message from a table show up every time period but you don’t want any of those messages repeated until all of them have at least shown up once, yes?

In the instance that the above case is correct, then four other ways to solve this (not an exhaustive list, there’s probably even more ways to solve this easily):

  • Use table.move to copy elements of your messages table into the queue table and then work with that instead as you go.

  • Track used indices when selecting a random message and keep randomising until you get an unused indice then select the message at that indice as the one to send. Keep doing this until all indices have been used, then clear the table.

  • Use the length operator to check for table emptiness. This is the problem with your original attempt as well: {} creates a table with a different memory address and no two tables are equal to each other. This is not including special circumstances (e.g. __eq present in metatable).

  • Just iterate your array in order.

I’m more a fan of table.move myself. The length operator check would be a simple fix to your comparison problem but not an overall fix because tables are passed by reference so it’s not simply enough to declare another variable, they’ll just point to the same table.

local messages = {...}
local messageQueue = table.create(#messages)

-- Don't use wait as the condition of a while loop
while true do
    -- Refill the messageQueue if it is empty
    if #messageQueue == 0 then
        -- Move all indices from messages to messageQueue
        table.move(messages, 1, #messages, 1, messageQueue)
    end

    -- Select an indice of available queued messages; keep the number
    -- variable so you can remove it from the queue easily later rather
    -- than needing to perform an unnecessary loop
    local index = Random.new():NextInteger(1, #messageQueue)
    -- You can pick out the message in a subsequent variable
    local message = messageQueue[index]

    -- For the sake of your own sanity, please use names that are more
    -- than a single character. And for the rest of ours too.
    R.SE:FireAllClients(message)

    -- We can now remove the queued message with table.remove since we saved
    -- the index from the randomisation earlier, instead of using a loop
    table.remove(messageQueue, index)

    -- NOW you can wait and using the task library's wait, not the global
    task.wait(60)
end

Like others have pointed out, the variable m points to the same table that the variable messages points to. m == messages but m ~= {} when the messages table becomes empty. You work on m, but because m is the same table as messages, changes apply to the table in memory. That if statement will never be true and when all messages have been sent and removed there will not be any messages to send from that point on, meaning no more tips.

The rest is all just practice problems of different assortments. Using wait as the condition of a while loop is bad. As for removing messages from your queue, you’re performing a highly unnecessary for loop to find out which indice to remove. You already get the indice when you perform the randomisation so separate those variables and then you can use table.remove right away the same way I do it in the code sample I provided above. And please, for your own sake, strive to use names that span more than one or two characters. It makes your code more readable and maintainable.

2 Likes

If you would’ve looked at my code closely, I added an extra function to prevent this issue from happening. I assume that after a few loops you encountered this error or even before finishing a loop at all you encountered it.

In my code I added a function that clones your Array because when you use m = messages, you’re just telling your Script that m is the same as your messages variable.

With this being said, try using my CopyTable() function and see how it works! :grinning_face_with_smiling_eyes:

2 Likes

Yes!

So, I should replace all wait() with task.wait()? The ones which are not even in loop?

is it better than math.random()?

task.wait superseded wait, the latter of which will eventually face deprecation. task’s members should be used for new work. The thread I linked here has to do with a practice problem using wait as the conditional part of a while loop; using task.wait instead presents the same problem. Use a proper condition for a while loop, even just true is fine.

The Random object and math.random share the same algorithm but there are some benefits to using the former. If you aren’t seeding then Random and math.random will be the exact same but if you ever need to seed then you should pick Random because the seed is local to that object whereas math.random needs to use math.randomseed to seed and that affects the seed for all calls of math.random across your code, which may produce undesirable results if you’re dependent on that seed. I also personally prefer Random over math.random so I use that whenever I want randomisation.

2 Likes