Why does this for loop stop half way and only scramble half of the words in the table?

[jamespringles]

My goal is to make a new order for the words table in a scrambled table, but the output shows it only goes half way. I can’t figure out what’s making this for loop stop half way?

local words = {"hello","world","good","morning","test"}
local scrambledWords = {}
function scramble()
	scrambledWords = {}
	for i,v in ipairs(words) do
		local RandomItem = words[math.random(1, #words)]
		print(RandomItem)
		table.insert(scrambledWords, 1, RandomItem)
		table.remove(words, table.find(words, RandomItem))
		print(#words)
		print(#scrambledWords)
	end
	words = scrambledWords
	print(scrambledWords)
end
scramble()

[Ziffix]

It’s because you’re removing values from the array as you traverse it. When you remove a value from an array, all the values to the right of that value will to move one index to the left:

{"A", "B", "C"}
  1    2    3

-- Remove B:

{"A", "C"}
  1    2

-- C shifted one index to the left.

To make this easier to comprehend, I’ll demonstrate this with a loop that instead removes the value at the current index:

local letters = {"A", "B", "C"}

for index = 1, #letters do
    table.remove(letters, index)
end

print(table.concat(letters, ", ")) --> B

Like ipairs, the goal of this loop is to travel through each value of the target array, one-by-one, left to right:

-- Loop pointer = ^

{"A", "B", "C"}
  1    2    3
  ^

As this loop removes A, the values in the array shift to the left. The loop will then attempt to remove the next value at index #2,

{"B", "C"}
  1    2 
       ^

You can see that “B” gets skipped. This will continue to happen with every other value in the array.

A good analogy to associate with this behaviour is the removal of blocks from a Jenga tower. Removing blocks from below the top will cause all blocks above that block to fall down one layer. The only way to avoid this is to remove blocks solely from the top until no blocks are left. We can express this as a backwards loop:

-- Loop pointer = ^

{"A", "B", "C"}
  1    2    3
            ^

-- Remove C:

{"A", "B"}
  1    2 
       ^

-- Remove B

{"A"}
  1  
  ^

local letters = {"A", "B", "C"}

for index = #letters, 1, -1 do
    table.remove(letters, index)
end

print(table.concat(letters, ", ")) --> 

In order to use this method, you must forego ipairs, as it can only iterate from left to right. However, we can use an entirely different approach altogether. Your current algorithm is quite inefficient. You can scramble an array without creating a new one or removing elements. This is done through the Fisher-Yates algorithm:

local function shuffle(array: {})
    for index = #array, 2, -1 do
        local randomIndex = math.random(index)

        array[index], array[randomIndex] = array[randomIndex], array[index]
    end
end

local letters = {"A", "B", "C"}

shuffle(letters)

print(table.concat(letters, ", ")) --> C, A, B

However, even this is unnecessary. Roblox has provided us the exact same algorithm through its Random class’ “Shuffle” function:

local letters = {"A", "B", "C"}

Random.Shuffle(letters)

print(table.concat(letters, ", ")) --> A, C, B

[NyrionDev (Nyrion)]

Never edit an array that you’re looping over, that’s a recipe for unintended behavior. Also what you’re trying to do can be easily done through Random.Shuffle:

local scrambledWords = table.clone(words) --to avoid changing the original table
Random.Shuffle(scrambledWords) --shuffle the words
print(scrambledWords)

or if you want to directly shuffle the words:

Random.Shuffle(words)
print(words)