table.RemoveByValue issue

[Holywiremod]

Hello, I've been attempting to remove a player entity from a table I made with a loop and it doesn't seem to remove the entity. The goal is to remove a player from table when they die. This is pretty much what the code looks like:

function GM:PlayerDeath( ply, item, attacker )
...

       SomeName( ply ) 

...
end


table = {} --contains player entities

function SomeName( ply )
...
                --Initially table has one player stored in it.

                for _,v in pairs( table ) do
			if( ply == v ) then 
				table.RemoveByValue( table, ply )
                                print( table.Count( table ) ) -- Prints a 0, indicating the player was removed
			end
		end
		print( table.Count( table ) ) -- Prints a 1, indicating the table.RemoveByValue didn't actually remove the player.
...
end

Is this the proper way of removing an entry from the table. If not, can I do so without using table.Empty() and recreating the table without the selected player?

[xav]

Setting a table's entry to nil removes it from the table. Since you're already looping over your entries, there's no need to call table.RemoveByValue, which is a horrible function IMO. You can just do this:

for k, v in pairs(tbl) do
	if v == ply then
		tbl[k] = nil
	end
end

Or, if you want to simplify your task, just use table keys as entries for your set. This ensures that there will be no duplicates and makes it easier to iterate over players, looking up whether a player is in the set, removing / adding players from and to it. I personally use this approach extensively and I find that it is what works best for [I]me[/I]. Experiment around and see what method works best for you and stick to it.

--Add the player to the set
tbl[ply] = true

--Check whether the player is in the set
tbl[ply]

--Iterate over all the players in the set
for ply in pairs(tbl) do
	print(ply:Nick())
end

--Remove player from set 
tbl[ply] = nil

--Sanity check if you don't clean up properly on player disconnects
for ply in pairs(tbl) do
	if not IsValid(ply) then
		tbl[ply] = nil
	end
end

[Holywiremod]

Ah, that makes a lot of sense. Everything works good now. Thank you, xavier! Also, if interested, I made another solution which isn't as good but I thought I'd share.

local temptable = {}

for _,v in pairs( table ) do
	if( ply != v ) then 
		table.insert( temptable, v )
	end
end
table.Empty( table )
table.Merge( table, temptable )

[Mijyuoon]

[QUOTE=Holywiremod;47836270]

table.Empty( table )
table.Merge( table, temptable )

[/QUOTE]

Err, why? Why not do simply this:

table = temptable

[zerf]

Hey man, the source for most table functions is in lua/includes/extensions/table.lua. Notice that almost all iterate over the table; some multiple times. This kills performance. I recommend you read over the other posts in this thread more carefully.

[Holywiremod]

After much testing it looks like my method actually works better for what I am trying to do minus the whole "table = temptable" thing (thanks for that.)

tbl[ply] = nil --Does remove but does not bump up position in table

So when you try to iterate over table a second time after ply from tbl[0] has been removed you have an entity in position 1 when it is expected to be in position 0. This causes the following error:

[ERROR] .../.../blah.lua:#: attempt to index local 'player' <a nil value>

Unless there is another way to make a table with keys [1,3,6,etc.] to become [0,1,2,etc.] reconstructing the table with a temptable resets the keys to proper order in addition to removing the player desired. This is a technique I learned from a Doctor in Electrical Engineering with computer focus.

Just thought I'd share. Have a good one!

[Mista Tea]

If you use the method suggested by xaviergmail, you shouldn't be indexing [0] or [1] at all (Lua starts numeric indexes at 1 as a heads up).

Xavier's method indexes the table by the player entities themselves. With that method, you wouldn't be using a [I]for i = 1, #table do[/I] loop (from the sound of what you're doing). You'd be using [I]for ply, _ in pairs( table ) do[/I].

Perhaps there is some confusion?

Anyway, if you're going to stick with numerically indexing the table, you can do it in a more efficient manner.

table = {}

function removePlayer( ply )
    for k, otherPly in ipairs( table ) do
        if ( ply == otherPly ) then
            if ( k < #table ) then -- only swap when the entity we're removing isn't already the last index in the table
                table[ k ] = table[ #table ] -- swap the value from the last index into the one we're removing
            end

            table[ #table ] = nil -- clear the index we swapped from
            break
        end
    end
end

By swapping the value at the last index into the index you're removing, you save yourself from iterating over the remaining elements and moving them down in the array (i.e., what table.remove does).

[Holywiremod]

Yeah that would work better. PrintTable() does print out tables with indexes as such though:
[0] = ply1
[1] = ply2
[2] = ply3...
etc..

[Mista Tea]

[QUOTE=Holywiremod;47849315]Yeah that would work better. PrintTable() does print out tables with indexes as such though:
[0] = ply1
[1] = ply2
[2] = ply3...
etc..[/QUOTE]

It will only print out like that if you're indexing the table numerically.

E.g., using entities as indexes:

lua_run_cl PrintTable( { 
    [LocalPlayer()] = true, 
    [Entity(101)] = true, 
    [Entity(102)] = true, 
    [Entity(103)] = true,
    [Entity(104)] = true 
} )

Entity [101][prop_physics]    =    true
Entity [102][prop_physics]    =    true
Entity [103][prop_physics]    =    true
Entity [104][prop_physics]    =    true
Player [1][Mista Tea]         =    true

Using numeric indexes:

PrintTable( {
    LocalPlayer(), 
    Entity(101),
    Entity(102),
    Entity(103),
    Entity(104)
} )

1    =    Player [1][Mista Tea]
2    =    Entity [101][prop_physics]
3    =    Entity [102][prop_physics]
4    =    Entity [103][prop_physics]
5    =    Entity [104][prop_physics]

[Holywiremod]

Ah, okay. So I assume this is numerically then:

--Code:

table = { Name,Name1,Name2,Name3 }
PrintTable( table )

--Console:

0      =      Player [1][Name]
1      =      Player [2][Name1]
2      =      Player [3][Name2]
3      =      Player [4][Name3]

Also, that code you supplied is good in theory:

table = {}

function removePlayer( ply )
    for k, otherPly in ipairs( table ) do
        if ( ply == otherPly ) then
            if ( k < #table ) then -- only swap when the entity we're removing isn't already the last index in the table
                table[ k ] = table[ #table ] -- swap the value from the last index into the one we're removing
            end
            table[ #table ] = nil -- clear the index we swapped from
            break
        end
    end
end

but in practice it does not work. Here is what I got using it:

--Code:
table = { ply1, ply2}

function removePlayer( ply )
    for k, otherPly in ipairs( table ) do
        if ( ply == otherPly ) then
            if ( k < #table ) then -- only swap when the entity we're removing isn't already the last index in the table
                table[ k ] = table[ #table ] -- swap the value from the last index into the one we're removing
                print( "k: " .. k .. "|#table: " .. #table)
            end
            table[ #table ] = nil -- clear the index we swapped from
            break
        end
    end
end

removePlayer( ply1 )

--Table is Originally:

0        =        Player [1][ply1]
1        =        Player [2][ply2]

--Results:
0        =        Player [1][ply1]  -- removed the wrong player as the if ( k < #table ) got skipped for some reason

--When modifying the "if ( k < #table )" to "if ( k < #table+1 )" to check k and #table , Results:

k: 0|#table: 0

I believe the if ( k < #table ) statement is messing up the code. Not sure if this would work as I can't test it at the moment:

table = { ply1, ply2}

function removePlayer( ply )
    for k, otherPly in ipairs( table ) do
        if ( ply == otherPly ) then
            table[ k ] = table[ #table ] -- No harm in swapping the same slot for itself just incase
            table[ #table ] = nil
            break
        end
    end
end

What do you think?

[Scott]

His snippet works just fine. Stop zero-indexing your tables or change table[#table] to table[#table-1]. Considering everything else starts from 1 its probably just best to do that.

[Holywiremod]

For some reason it works now without modifying the code. I don't know what the issue was but it seems to have fixed itself after I restarted my computer and server and everything else. Zero indexing wasn't the issue.