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.
