esx-framework / esx_core

Official Repo For core resources for esx-legacy
https://documentation.esx-framework.org/
GNU General Public License v3.0
368 stars 733 forks source link

Feat : Add uniqueId to class Player #1352

Closed enzo2991 closed 1 month ago

enzo2991 commented 5 months ago

Add id return to database for unique user identifier

tomiichx commented 4 months ago

I understand that it's a personal preference, but IMO it would make more sense to name it uniqueID instead of uniqueId, as the ID is an abbreviation for identifier. Also, esxID doesn't sound too bad.. 😄

enzo2991 commented 4 months ago

thank you for the feedback, I just modified with your request modification, but for the esxID, I prefer to keep the naming convention in camel case 😃

Gellipapa commented 4 months ago

@enzo2991 Hi! I don't quite understand what this is good for us, it doesn't link to anything, there is no conversion, even the playerId is used by the xPlayer table as an index, this uniqueId is not used by anything, it would be redundant to put it in the core.

But I'm curious what use-cases you can tell me for this.

If you really want to do this, you can do it in your own script so that when the player comes into action, you set the uniqueId with xPlayer.set.

Thekuca commented 4 months ago

@enzo2991 Hi! I don't quite understand what this is good for us, it doesn't link to anything, there is no conversion, even the playerId is used by the xPlayer table as an index, this uniqueId is not used by anything, it would be redundant to put it in the core.

But I'm curious what use-cases you can tell me for this.

If you really want to do this, you can do it in your own script so that when the player comes into action, you set the uniqueId with xPlayer.set.

A good example for the use case is Grand RP on RageMP.

On FiveM, player ids always change, so lets say for example id 50 does something that is bannable and then leaves the server. The admins need to go check his identifiers (steam, rockstar...) so they can offline ban him. This way he always has a unique id and the admin could just go /ban uniqueid ...

enzo2991 commented 4 months ago

@enzo2991 Hi! I don't quite understand what this is good for us, it doesn't link to anything, there is no conversion, even the playerId is used by the xPlayer table as an index, this uniqueId is not used by anything, it would be redundant to put it in the core.

But I'm curious what use-cases you can tell me for this.

If you really want to do this, you can do it in your own script so that when the player comes into action, you set the uniqueId with xPlayer.set.

As @Thekuca says, using the uniqueId would be useful for offline players, I find it easier to mark a number than a string, and I'm thinking of adding a GetPlayerFromUniqueId or GetPlayerFromEsxId function.

Gellipapa commented 4 months ago

@enzo2991 Hi! I don't quite understand what this is good for us, it doesn't link to anything, there is no conversion, even the playerId is used by the xPlayer table as an index, this uniqueId is not used by anything, it would be redundant to put it in the core. But I'm curious what use-cases you can tell me for this. If you really want to do this, you can do it in your own script so that when the player comes into action, you set the uniqueId with xPlayer.set.

As @Thekuca says, using the uniqueId would be useful for offline players, I find it easier to mark a number than a string, and I'm thinking of adding a GetPlayerFromUniqueId or GetPlayerFromEsxId function.

So now that you're making it include conversion methods you can make it work the previous version lacked them. Previously we introduced uniqueId in the database so this shouldn't cause any compatibility problems.

My concern is that adding a new table that stores xPlayer again is a pretty serious duplication. (We can do away with that because lua is fast and it's the least risk that something will be indexed incorrectly and break the system, so it's fine.)

The other is that the esxId should be written at the end because we don't want to break the old system principle that source is the first parameter, it should be written at the end because it becomes optional and won't cause any problems.

Gellipapa commented 4 months ago

There needs to be some logic to this because just because you can now index the esxPlayerId table, even if I submit a source it won't use it.

So there needs to be a logic that if a source is submitted it will convert to player unique id.

Because then by default you don't know what the player's unique id is, and if you want to make a system like this you want to handle every player by unique id.

Gellipapa commented 4 months ago

You need to think about this even more if you want to implement a GrandRP level unique ID logic in ESX.

enzo2991 commented 4 months ago

So now that you're making it include conversion methods you can make it work the previous version lacked them. Previously we introduced uniqueId in the database so this shouldn't cause any compatibility problems.

My concern is that adding a new table that stores xPlayer again is a pretty serious duplication. (We can do away with that because lua is fast and it's the least risk that something will be indexed incorrectly and break the system, so it's fine.)

The other is that the esxId should be written at the end because we don't want to break the old system principle that source is the first parameter, it should be written at the end because it becomes optional and won't cause any problems.

I understand very well I think I will adapt to remove this duplication that I find is also deprived and useless, I will also modify to put the source in front.

tomiichx commented 4 months ago

The thing is that an unique identifier already exists, and it's the player license. The issue with the license is just that it's too long. However, this should by no means replace the already existing license identifier... Though I can see the esxId being used for the DB normalization. It's a much better approach to work with joins when fetching data 😅

Also @Gellipapa, I don't think the position of the argument shouldn't be an issue here. Afaik, the CreateExtendedPlayer() is just used in the es_extended, no?

tomiichx commented 4 months ago

Also, wouldn't it make more sense for every user to have the same length of the esxId? I'd say a 6-character alphanumeric string (or just a number) would make sense..?

enzo2991 commented 4 months ago

Also, wouldn't it make more sense for every user to have the same length of the esxId? I'd say a 6-character alphanumeric string (or just a number) would make sense..?

I don't think so, because to return a string is longer than to return a number, so it's true that in lua, is more speed

enzo2991 commented 4 months ago

The thing is that an unique identifier already exists, and it's the player license. The issue with the license is just that it's too long. However, this should by no means replace the already existing license identifier... Though I can see the esxId being used for the DB normalization. It's a much better approach to work with joins when fetching data 😅

Also @Gellipapa, I don't think the position of the argument shouldn't be an issue here. Afaik, the CreateExtendedPlayer() is just used in the es_extended, no?

It can't replace the license, but it will be possible with some modifications to put several licenses on 1 ID, but for that to happen, you'll have to review the entire use of the ID, or even minimize the license as you said

Gellipapa commented 4 months ago

The thing is that an unique identifier already exists, and it's the player license. The issue with the license is just that it's too long. However, this should by no means replace the already existing license identifier... Though I can see the esxId being used for the DB normalization. It's a much better approach to work with joins when fetching data 😅

Also @Gellipapa, I don't think the position of the argument shouldn't be an issue here. Afaik, the CreateExtendedPlayer() is just used in the es_extended, no?

Hi!

Yes but this would be a big change in core so I wouldn't put the esxId at the beginning but at the end to show that it was added on the way, the first parameter is always the source, that's the policy, in all cases.

In the current version I still don't see a solution to fully identify the player, the goal is to be able to identify the player with an ID, so either I need a conversion method or I need to write this ID on the player with a statebag and then retrieve it, but I need some solution, this is still a little bit too little at the moment.

UI: In the meantime I saw that the statebag is included, but it still doesn't work without modifying the scripts, I need to do this so that if I have 500 scripts I don't have to touch them but if they use a unique ID based on the config, it will be automatically returned by GetPlayerFromId, so probably the ID extracted from the state-bag.

If I would like to support the unique id at the formatting level then I would have to redesign the DB a bit, I think we should not do this in the first round, stick with the id column we are using now which is auto_increment and does not need db change.

Thekuca commented 3 months ago

Status?

Gellipapa commented 3 months ago

Status?

Hi! This needs more thought because it involves a core change and for such a legacy system it's not good to have such a big change so we are still thinking about this and looking at possible solutions.

Mycroft-Studios commented 2 months ago

just throwing this out there, this is pointless, and always has been since it was thought of it in 2017, and theres a reason nobody uses it this way. this ID is just pointless data that has no actual meaning in FiveM. The FiveM ID system is built into the game, and stems from GTA's code itself. this ID will have quite literally no use other than for offline banning something, which, tbh, just use txadmin if you want that, theres literally fleshed out tools already in the fxserver for that functionality, and ESX itself does not include banning functionality, at all, so its pointless to ESX itself. its not an ID that cannot be used in natives, its not an ID that can be used in normal commands, its just pointless data that serves no purpose to ESX itself

Arctos2win commented 1 month ago

just throwing this out there, this is pointless, and always has been since it was thought of it in 2017, and theres a reason nobody uses it this way. this ID is just pointless data that has no actual meaning in FiveM. The FiveM ID system is built into the game, and stems from GTA's code itself. this ID will have quite literally no use other than for offline banning something, which, tbh, just use txadmin if you want that, theres literally fleshed out tools already in the fxserver for that functionality, and ESX itself does not include banning functionality, at all, so its pointless to ESX itself. its not an ID that cannot be used in natives, its not an ID that can be used in normal commands, its just pointless data that serves no purpose to ESX itself

I completely agree