Expose arrestedPlayers table globally

[Aws0mee]

I would like to expose this table globally to allow for more efficient usage. Right now, if you want a table of all arrested players you must do one of the following:

1. Loop through all players and check if they're arrested. This obviously wastes performance when we can simply get a table with all the arrested players instantly.
2. Get a reference to arrestedPlayers using debug.getupvalue on a function where the table is an upvalue. This works fine for me, but I feel like most glua developers are not too familiar with the debug library or how to use it. So if we expose the table globally then everyone will have access to it.

Additionally, we can also make a function which returns the arrestedPlayers table and keep it local or rename it to something more unique like arrestedPlayersDarkRP or something. I'd be happy to make changes, just let me know.

[FPtje]

This should at least be a function that returns this value. Removing the local looks like a bug.

[FPtje]

Also, third option: use the hooks to keep your own table. Using this one is quite full of footguns: it works on steam ids, not on player objects, the players are the keys, not the values, modifying it will most likely cause things to break, it has its own behavior regarding players that disconnect.

Its only reason to exist is to be an optimisation. I think it's too dangerous to expose it. Maybe just a function that returns a count, or a function that builds a num:ply table.

[Kefta]

Just expose an iterator function like DarkRP.iterateArrestedPlayers()

[Aws0mee]

This should at least be a function that returns this value. Removing the local looks like a bug.

I think this is the best solution, what do you think it should be called?

[FPtje]

I'm afraid that doesn't address the concerns in my latest post. It would be better to implement my idea or @Kefta's

[Aws0mee]

I'm afraid that doesn't address the concerns in my latest post. It would be better to implement my idea or @Kefta's

That could work too, could we return a copy of the table instead? Or would that not be optimal?

[FlorianLeChat]

Just expose an iterator function like DarkRP.iterateArrestedPlayers()

I'm afraid the average DarkRP coder won't know how to use a function which returns an iterator, a table would be "simpler" to use and it's more typical in the gamemode codebase.

[Kefta]

Just expose an iterator function like DarkRP.iterateArrestedPlayers()

I'm afraid the average DarkRP coder won't know how to use a function which returns an iterator, a table would be "simpler" to use and it's more typical in the gamemode codebase.

A simple example on the wiki page showing for k,v in DarkRP.iterateArrestedPlayers() do would clear up any confusion (with better names for the keyvals but I don't remember how the table is structured). Iterators are a core idiom of Lua and custom ones are commonly used in pretty much every Lua landscape except GMod's.

[Aws0mee]

Just expose an iterator function like DarkRP.iterateArrestedPlayers()

I'm afraid the average DarkRP coder won't know how to use a function which returns an iterator, a table would be "simpler" to use and it's more typical in the gamemode codebase.

Yeah, I was thinking the same thing. I feel like just returning the table would be the most simple. However, I'll be happy to implement whatever @FPtje believes is best.

[FPtje]

Let's do the iterator with documentation, though this iterator should ideally give player objects, not the steam IDs and booleans that the original table has. So ideally, the usage would be for ply in DarkRP.iterateArrestedPlayers() do (note the lack of k.

The iterator should specifically skip over SteamIDs of players that are not currently in the server. Those Steam IDs can be in there, because rejoining should get them re-arrested.

With a player limit of 128, one can wonder if iterating over all players is not more efficient than iterating over the table and checking the SteamIDs, but I'll leave that as an optional exercise.

[FPtje]

Due to PR inactivity I decided to take a shot at this myself. @Aws0mee @Kefta what do you think?

I've added DarkRP.arrestedPlayers() and DarkRP.arrestedPlayerCount() to both show examples of the iterator and for ease of use.

[FPtje]

My local test for reference:

The functions indeed don't show arrested-but-disconnected players, until they come back.

[Kefta]

I generally agree with the functions you exposed, but with some refactoring of the arrestedPlayers structure, linear iteration can be achieved. Lemme write up a POC

[Aws0mee]

Due to PR inactivity I decided to take a shot at this myself. @Aws0mee @Kefta what do you think?

I've added DarkRP.arrestedPlayers() and DarkRP.arrestedPlayerCount() to both show examples of the iterator and for ease of use.

Sorry for my inactivity, I have been busy with classes and work. I think this should be sufficient for my use case and any others I can think of. I honestly still feel like adding these functions instead of just making the table global or giving a reference to it through a function is a little over-complicated, but I think this should still work fine. Either way, it's an improvement over having to use the debug library to get a reference to the table using upvalues which most people don't know how to do. Looks good to me so far, looking forward to what @Kefta has to show.

[FPtje]

I generally agree with the functions you exposed, but with some refactoring of the arrestedPlayers structure, linear iteration can be achieved. Lemme write up a POC

Thanks! if the functions have the right interface, then I think this PR is good to go, and improvements can be made later without affecting the interface.

I honestly still feel like adding these functions instead of just making the table global or giving a reference to it through a function is a little over-complicated, but I think this should still work fine.

I understand. My concern is mostly that I don't like addon authors being able to modify DarkRP internals. Having an addon write that datastructure can lead to very weird bugs that show up as errors in DarkRP code. With these functions, that mistake should not be easy.