Table and Arrays, error removing NPC correctly

I am just getting started with Tables and Arrays. In the workspace I have 4 NPCs, I want to insert them into a table called ‘NPCs’ and when one of the NPCs dies it will print the names of all the remaining NPCs.

Sometimes when I remove an NPC it still prints the NPC in the output.

I was told I should try iterating backwards, but I did this and still had the same problem.

The entire code:

--services
local workspace = game:GetService("Workspace")

--data
local workspaceItems = workspace:GetChildren()
local NPCs = {}

for index = 1, #workspaceItems do
	local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")
	if humanoid then
		table.insert(NPCs, workspaceItems[index])
	end
end

for index = 1, #NPCs do
	local humanoid = NPCs[index]:FindFirstChild("Humanoid")
	if humanoid then
		humanoid.Died:Connect(function()
			print(index)
			print(NPCs[index])
			table.remove(NPCs, index)    <-- I think the error is somewhere here
		end)
	end
end
3 Likes

So essentially what is happening here is that some NPC is removed because they died. This worked wonderfully and nothing went wrong. Let’s say this was NPC 3. So index 3 was removed from NPCs, what happens now?

You have 4 items in the list of

{1 = NPC1,
2 = NPC2,
3 = NPC3,
4 = NPC4}

After removing NPC 3, the list does not become

{1 = NPC1
2 = NPC2
3 = nil
4 = NPC4
}

it is instead

{1 = NPC1
2 = NPC2
3 = NPC4
4 = nil -- essentially not a part of the table.
}

And since you store the entry-id on the NPC, instead of it’s actual id, when NPC4 dies, it deletes the nil entry instead. (Effectively doing nothing.)

Well, that’s the gist of it, and how you fix it is entirely up to you.
My method would involve changing the table to a dictionary (like storing it under index = npc.Name)
But there are probably way better methods of doing this than that.

2 Likes

So, exactly what @mathymods said, but also your code will only insert the NPC one time. Even if the code did work as expected, you still have to allow for respawns of NPC to be added back in the table if you choose to do so.

1 Like

That makes sense. So how would I update the table when an NPC is removed? So instead of 1, 4 it’s 1, 3.

One possible option would be to make it re-run the whole loop by calling it as a function:

function loopThroughTable()
    for index = 1, #NPCs do
	local humanoid = NPCs[index]:FindFirstChild("Humanoid")
	if humanoid then
		humanoid.Died:Connect(function()
			print(index)
			print(NPCs[index])
			table.remove(NPCs, index)    <-- I think the error is somewhere here
            loopThroughTable() -- This will call it again
		    end)
	    end
    end
end

I’m not sure, this seems a little precarious.

Don’t do this.
You will now connect more and more functions, calling different iterations.
This will cause some nasty situations where NPC3 used to be in slot 3, but is now in 2, so when they now delete NPC3, it will delete index 2 and 3

I would rather use names as indexes.

for index = 1, #workspaceItems do
	local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")
	if humanoid then
		table.insert(NPCs, workspaceItems[index].Name, workspaceItems[index]) 
-- or store a number, that number is the ID and use tostring(ID) when setting names.
	end
end

for index, npc in pairs(NPCs) do
	local humanoid = npc:FindFirstChild("Humanoid")
	if humanoid then
		humanoid.Died:Connect(function()
			print(index)
			print(npc)
			table.remove(NPCs, index)
		end)
	end
end

Quick warning; This is not tested.

1 Like

Elephant in the room: two loops is unnecessary overhead. I’d also make a comment about iterating through the entire workspace but it seems that this is meant to be an arbitrary system so I’ll refrain from that. It’d probably be better if you put those NPCs in a folder and iterated through it though.

local NPCs = {}

for index = 1, #workspace:GetChildren() do
    -- The bottom will break if NPCs are not named a number
    local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")
    if humanoid then
        -- Don't re-look up the NPC
        table.insert(NPCs, humanoid.Parent)
        humanoid.Died:Connect(function ()
            table.remove(NPCs, index)
        end)
    end
end

Just a warning that table.remove shifts the elements down by one, which is probably why table.remove isn’t functioning as you’d expect. I’d recommend switching over to a dictionary and using the NPC instances as keys instead.

local NPCs = {}

for index = 1, #workspace:GetChildren() do
    -- The bottom will break if NPCs are not named a number
    local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")
    if humanoid then
        -- Don't re-look up the NPC
        NPCs[humanoid.Parent] = humanoid -- Example?
        humanoid.Died:Connect(function ()
            NPCs[humanoid.Parent] = nil
        end)
    end
end
1 Like

Just so you know, you added an extra enclosing parenthesis after GetChildren(). This error might make it harder for OP to understand what you’re explaining.

Habit. I rarely use numeric for loops anymore because they aren’t really appropriate for scenarios like this. Thanks for pointing that out: corrected as such.

The whole function is error-bound, frankly. It indexes items by a number and assumes the assigned value is non-nil.

1 Like
--services
local workspace = game:GetService("Workspace")

--data
local workspaceItems = workspace:GetChildren()
local NPCs = {}

for index = 1, #workspaceItems do
	local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")
    if humanoid then
        NPCs[#NPCs + 1] = workspaceItems[index]
	end
end

for index = 1, #NPCs do
	local humanoid = NPCs[index]:FindFirstChild("Humanoid")
	if humanoid then
		humanoid.Died:Connect(function()
            table.remove(NPCs, index)
            for i = 1, #NPCs do
                print(NPCs[i].Name)
            end
		end)
	end
end

I don’t think your problem was anything in the script but yea what you wanted to do was print out the remaining NPCs and in order to do that you need to loop when the NPC dies. So this should get at what you want I believe.

Mind elaborating on this? As far as I’m concerned: GetChildren() places the children of a given instance in an array table… “It indexes items by a number and assumes the assigned value is non-nil.” what do you mean by this? As far as I know, instances that are ‘nil’ or destroyed are parented under nil and locked. So to claim that it has a chance of placing a destroyed instance wouldn’t work. But if you mean that the number index could be nil then that’s fine as long as it isn’t a sparse table… since indices that no longer exist are nil anyway.

This doesn’t make sense… if you don’t know how table.remove works, it makes sure that no sparse table is created when removing something from a table. In regular cases if I were to use Table[Index] = nil , this would work but depending on the position of the index you can no longer do a numerical for loop on the table since the famous for i = 1, #Table loop works in a way in which it stops after finding the first nil value of an array table. So for example if there were 12 values set in a table from 1 - 12 as their index, if for example the 6th index was set to nil and a #Table was performed on the table it would only count all the values leading up to the 6th index as the 6th index is set to nil and would stop further counting. I’m assuming that Techante wanted to not create a spare table hence why he used table. remove since it would shift the indexes so that a nil value wouldn’t be created between values.

Also, I don’t know if you are aware but since you did mention efficiency in the beginning of your post I’d like you to know that dictionaries are more costly than arrays in lua.

The function I wrote (by extension of OP’s code) assumes that every child of the workspace is named a number, hence the first two lines:

for index = 1, #workspaceItems do
	local humanoid = workspaceItems[index]:FindFirstChild("Humanoid")

As you can see, the following code uses the number of children in the Workspace to form a numeric for loop from 1 to n. In each iteration, as shown by the second statement, it goes

workspaceItems[index]:FindFirstChild("Humanoid")

This statement assumes that workspace[index] exists. Let’s pretend that there are 7 instances in the workspace, 5 of which are named 1-5 but the other two are just named “Model”. This loop will run 7 iterations. When it gets to 6, workspace[“6”] does not exist, therefore it will throw with attempt to index a nil value.

The only fix to this would be to use the actual instance table and use the index to perform a lookup using the number as a key, for which a value (the Instance in the workspace) will exist for.

local workspaceItems = workspace:GetChildren()

for index = 1, #workspaceItems do
    local humanoid = workspace[workspaceItems[index]]:FindFirstChild("Humanoid")

table.remove removes the item at the index n passed to the second argument and shifts all the elements down. It makes sense. There’s nothing more to understand about it.

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

for i, v in ipairs(tbl) do
    print(i, v)
end -- 1 A 2 B 3 C

table.remove(tbl, 2)

for i, v in ipairs(tbl) do
    print(i, v)
end -- 1 A 2 C

OP’s code doesn’t take into account element shifting, ergo the lack of a non-nil value at the index when Humanoid.Died fires and tries removing it from the NPC table.

If you don’t know what you’re talking about, please don’t reply to me saying I’m wrong or that what I’m saying is nonsense. Do your own testing.

This is an unsubstantiated criticism.

The len operator on a table doesn’t just stop counting at a nil index. When you add elements to the table, it reallocates and adds more slots. When you remove an element, the size is recalculated (providing you aren’t using a dictionary, which indices are added to the hash map, which then you need to count a different way).

local tbl = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11}

print(#tbl) -- 11

for k, v in pairs(tbl) do
    print(k, v)
end -- 6 is non-nil

tbl[6] = nil

print(#tbl) -- 11
-- size only becomes 10 with table.remove(tbl, 6)

for k, v in pairs(tbl) do
    print(k, v)
end -- 6 is nil

What are you talking about? Arrays and dictionaries are literally the same in Lua. One uses numeric indices and one uses arbitrary keys. There’s no difference. Even if there is one, it’s negligible. If you want to talk about real performance involving Lua tables, what’s worth looking at is constructing a table with a predefined size (essentially templating the table) over a table with no elements, making it’s size arbitrary and fickle depending on your implementation.

Yea I see where I went wrong with this one.

It’s a micro-opt but there is a difference and since you do want to point out efficiency, arrays are faster in lua.

The remarks of efficiency between having two loops (which also extends to a practice problem) and how you construct a table are irrelevant in their entirety. Please do show me a source or repro of how a table with arbitrary keys is more costly than without.

Regardless of whether you choose to or not, this has no relevance being brought up. What you choose to use is use case dependent, as well as preferential.

The gist of it is that in arrays the indices are direct while in dictionaries it’s a bit more of a process. And that logically means less efficient because the more you do the less efficient.

That’s nothing more than a speculative assumption, which is untrue. What process are you talking about? Table lookups are the same either way whether you use a numerical indice or an arbitrary one. There’s no difference. One uses numbers, one doesn’t.

It… uh that’s not how it works… arrays in Lua are direct and don’t require resizing basically with dictionaries you’re hashing and moving elements and checking multiple times (aka hasher will come up with indices and put them in the hash part array. based on the hash and if it runs out of space to put new elements it resizes)

Seriously. Stop cherry-picking my responses and disagreeing with me for no reason. If you’re going to say something, then have something to back it up with. You’re making far too many assumptions and failing to appropriately address the points of my thread.

Regardless of what kind of table you have, a table lookup is the same.

local tbl = {"A"}
local dict = {[1] = "A"}

print(tbl[1]) -- A
print(dict[1]) -- A

There’s no difference in this table lookup. At all. This harkens right back to my original point asking for a source which one has not been produced as of yet. You still haven’t replied to any of the initial comments either:

  • How is the creation of a dictionary more costly than an array? No source has been provided, just a speculative “gist” that doesn’t say anything.

  • What “process” is involved with dictionaries? What are you talking about by “direct” indices? A table lookup by index, regardless of what you use as the key, is the same either way.

Yes they do. Untemplated tables reallocate when you add more slots to them. Lua constructs tables with a size dependent on how many elements you specify during its construction. In addition, creating a table with a predefined size is less costly than not.

Sources:
Lua as Fast as Possible - Addressing table creation
Lua PiL 3.6 - Tables
Lua PiL 11.1 - Arrays

Do your research. Stop throwing around assumptions and false facts.

1 Like