search

Qbox-project / qbx_core

http://qbox-project.github.io
Other
46 stars 109 forks source link

config(server): add letter to citizen id generator #426

Closed jnccloud closed 3 months ago

jnccloud commented 3 months ago

Description

It adds a letter to the first character of the citizenid. This presents problems in some functions such as GetPlayerByCitizenId(cid).

image image

Statistically, this is an extreme edge case, but the RNG has dealt its cards among one of my players and this is the issue after further review. Maybe this would be better suited to enforce string conversion in the functions that accepts CID... but this at least stops the problem at the source.

Checklist

D4isDAVID commented 3 months ago

What kinds of problems are there without this change? Inside the SQL table, citizenid is already a string.

jnccloud commented 3 months ago

What kinds of problems are there without this change? Inside the SQL table, citizenid is already a string.

See the images attached. The function errors out and does not find the Player. I am not sure how widespread the problems are when this happens. I'll see if I can check the type.

jnccloud commented 3 months ago

What kinds of problems are there without this change? Inside the SQL table, citizenid is already a string.

Ok, so further research shows that the javascript inside the third party script doesn't declare the type for the citizenid when sending it thru the nui callback, so even though its a string when print(type(player.PlayerData.citizenid), "citizenid") is executed, running the same in JS shows it as a number console.log(typeof(citizenid))

image image

One could argue that the third parties should enforce proper typing of variables, but at the same time this is very simple to prevent at the core and could save others hours of time I wasted on debugging this. Its even worse if a third party script only provides compiled, minified or encrypted front-end code. Again, its an edge case, because most citizenid values are alphanumeric. But the RNG gave this player all integers.

solareon commented 3 months ago

Yep. Been wondering why I had been seeing a few random errors since a recent update.