Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Add discord to players DB table #1135

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

Cocodrulo
Copy link
Contributor

Description

Nowadays most servers using QBCore uses discord whitelist, so the player will have their discord linked to FiveM. To ease integration with webapges I think discord information in main qbcore database table (players) should be kept to easy locate characters linked to a player.

Checklist

  • I have personally loaded this code into an updated qbcore project and checked all of its functionality.
  • My code fits the style guidelines.
  • My PR fits the contribution guidelines.

Copy link

@nick-perry14 nick-perry14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how necessary this is, but if you really want it merged you have to provide a schema update with the PR.

@@ -437,10 +438,11 @@ function QBCore.Player.Save(source)
local pcoords = GetEntityCoords(ped)
local PlayerData = QBCore.Players[source].PlayerData
if PlayerData then
MySQL.insert('INSERT INTO players (citizenid, cid, license, name, money, charinfo, job, gang, position, metadata) VALUES (:citizenid, :cid, :license, :name, :money, :charinfo, :job, :gang, :position, :metadata) ON DUPLICATE KEY UPDATE cid = :cid, name = :name, money = :money, charinfo = :charinfo, job = :job, gang = :gang, position = :position, metadata = :metadata', {
MySQL.insert('INSERT INTO players (citizenid, cid, license, discord, name, money, charinfo, job, gang, position, metadata) VALUES (:citizenid, :cid, :license, :discord, :name, :money, :charinfo, :job, :gang, :position, :metadata) ON DUPLICATE KEY UPDATE cid = :cid, name = :name, money = :money, charinfo = :charinfo, job = :job, gang = :gang, position = :position, metadata = :metadata', {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the corresponding Schema update?

@@ -97,6 +97,7 @@ function QBCore.Player.CheckPlayerData(source, PlayerData)
if source then
PlayerData.source = source
PlayerData.license = PlayerData.license or QBCore.Functions.GetIdentifier(source, 'license')
PlayerData.discord = PlayerData.discord or QBCore.Functions.GetIdentifier(source, 'discord'):sub(9)
Copy link

@nick-perry14 nick-perry14 Jul 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to add. What if this is nil? calling :sub on a null may cause an issue. Requires testing.

qbcore.sql Outdated
@@ -3,6 +3,7 @@ CREATE TABLE IF NOT EXISTS `players` (
`citizenid` varchar(50) NOT NULL,
`cid` int(11) DEFAULT NULL,
`license` varchar(255) NOT NULL,
`discord` varchar(255) NOT NULL,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be null.

@Cocodrulo
Copy link
Contributor Author

Just updated, I dont really know if QBCore.Functions.GetIdentifier does retrieve null because if we follow the documentation of the function GetPlayerIdentifierByType and it is right, it always return a string so, the sub will happen in a string, but change it if the docs are not correct

@nick-perry14
Copy link

QBCore.Functions.GetIdentifier can very much return nil.

image

@Cocodrulo
Copy link
Contributor Author

Cocodrulo commented Aug 4, 2024

You will be correct if you are talking about old qbcore, now it uses the native:

function QBCore.Functions.GetIdentifier(source, idtype)
if GetConvarInt('sv_fxdkMode', 0) == 1 then return 'license:fxdk' end
return GetPlayerIdentifierByType(source, idtype or 'license')
end

@Irishstevie
Copy link
Contributor

Maybe add something to the config.lua to turn this on and off

@Cocodrulo
Copy link
Contributor Author

Well, because the SQL file was change, I feel that it shouldn't be optional

Copy link

github-actions bot commented Nov 7, 2024

This PR has had 60 days of inactivity & will close within 7 days

@github-actions github-actions bot added the Stale label Nov 7, 2024
@Cocodrulo
Copy link
Contributor Author

What about this?

@github-actions github-actions bot removed the Stale label Nov 12, 2024
@Qwerty1Verified
Copy link
Contributor

Could you possibly elaborate on the use case example you mentioned?

@Cocodrulo
Copy link
Contributor Author

Could you possibly elaborate on the use case example you mentioned?

Yes, for example if u want to get have an admin menu to see all players data (no matters they are online or not) it is always easier, mostly for RP servers, to locate players based on discord Id.

Also if u want to integrate your sv with a discord bot or maybe a Webpage with discord login this will be helpful.

Discord is the main way gaming communities get along right now, and a better integration with this programme providing more critical data storing can become handy.

@GhzGarage
Copy link
Member

The idea is fine but needs more thought instead of just saving additional information

@GhzGarage GhzGarage self-assigned this Nov 13, 2024
@GhzGarage GhzGarage added enhancement New feature or request help wanted Extra attention is needed Needs Testing/Reviews labels Nov 13, 2024
@Cocodrulo
Copy link
Contributor Author

As I said before, is not only for just saving data, is for future scripts functionality and for easier Web integration

@Cocodrulo
Copy link
Contributor Author

I think it really should be added. This will allow new scripts with new innovations

@Cocodrulo Cocodrulo changed the title Add discord to players DB table Feat; Add discord to players DB table Dec 9, 2024
@Cocodrulo Cocodrulo changed the title Feat; Add discord to players DB table Feat: Add discord to players DB table Dec 9, 2024
@RojinxChhetri
Copy link
Contributor

I think it really should be added. This will allow new scripts with new innovations

I agree that this addition is essential. It opens the door for creators to develop innovative scripts and introduces opportunities for further enhancements. It would be a valuable contribution to the project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed Needs Testing/Reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants