qbcore-framework / qb-core

FiveM RP Framework Core :muscle:
GNU General Public License v3.0
590 stars 960 forks source link

Duplicate license check: By MrCrowley #1035

Closed gononono64 closed 1 year ago

gononono64 commented 1 year ago

Description

Duplicate license check just...doesnt work Because of that this happens: https://www.youtube.com/watch?v=U1Pxpxuj7-o If you bother to look into this you will realize you should merge this pretty much asap

PS I got lazy and rewrote what i did through the github editor. Its just a few minor modifications that i did. I also did these modifications on another server that was having the same issue. Person who brought it to my attention.

Please make sure i didnt miss anything. AKA make sure to test it

Checklist

gononono64 commented 1 year ago

There, next time dont approve things the linter doesnt accept...her der

iboss21 commented 1 year ago

we personally tested it on our server and fixed it now. thanks to MrCrowley

gononono64 commented 1 year ago

tested and working the commands.lua and functions.lua part is not needed for merge

commands is needed for linter, functions there was a redundant check

Qwerty1Verified commented 1 year ago

What part of the original code stops it from working? Is it the retrieving of the licenses before waiting that's stopping it?

ChristianBDev commented 1 year ago

What part of the original code stops it from working? Is it the retrieving of the licenses before waiting that's stopping it?

This is where it is stopping it,

-- if GetConvarInt('sv_fxdkMode', false) then
    --     license = 'license:AAAAAAAAAAAAAAAA' -- Dummy License
    -- end

    if not license then
        deferrals.done(Lang:t('error.no_valid_license'))
    elseif QBConfig.Server.CheckDuplicateLicense and QBCore.Functions.IsLicenseInUse(license) then
Qwerty1Verified commented 1 year ago

Checking that convar should be doable, this is the normal documented usage though local fxdkMode = GetConvarInt('sv_fxdkMode', 0) == 1

gononono64 commented 1 year ago

Checking that convar should be doable, this is the normal documented usage though local fxdkMode = GetConvarInt('sv_fxdkMode', 0) == 1

For what purpose?

Qwerty1Verified commented 1 year ago

Its purpose is to allow the usage of the Cfx.re Development Kit (FxDK). The edit on line 50 of the events.lua file also seems unnecessary.

gononono64 commented 1 year ago

Its purpose is to allow the usage of the Cfx.re Development Kit (FxDK). The edit on line 50 of the events.lua file also seems unnecessary. QBConfig.Server.CheckDuplicateLicense is definitely needed as its checking the config as it should. The previous was a typo

Qwerty1Verified commented 1 year ago

It was previously checking the config through QBCore.Config

gononono64 commented 1 year ago

It was previously checking the config through QBCore.Config

image image

Qwerty1Verified commented 1 year ago

https://github.com/qbcore-framework/qb-core/blob/a866b1a0492a1d01678bc765b3beda5f70bc3ebf/server/main.lua#L2 QBCore.Config is a variable alias defined by QBConfig

gononono64 commented 1 year ago

https://github.com/qbcore-framework/qb-core/blob/a866b1a0492a1d01678bc765b3beda5f70bc3ebf/server/main.lua#L2

QBCore.Config is a variable alias defined by QBConfig

Well i can tell you from experience, being a developer and having to figure out what links where without any search options, is simply bad design. Makes it much harder to decode. At this point take it or leave it

Qwerty1Verified commented 1 year ago

It's usually just a matter of best practice, hence why getters and setters exist to prevent access to the original data structure. People write code differently, but this is usually the practice that is followed. When most of the other code uses QBCore.Config, it can be assumed that it works if it hasn't been modified, and then of course a quick search for QBCore.Config would reveal that it is a real variable and has a role

gononono64 commented 1 year ago

It's usually just a matter of best practice, hence why getters and setters exist to prevent access to the original data structure. People write code differently, but this is usually the practice that is followed. When most of the other code uses QBCore.Config, it can be assumed that it works if it hasn't been modified, and then of course a quick search for QBCore.Config would reveal that it is a real variable and has a role

image

Qwerty1Verified commented 1 year ago

Yeah that's why I said different people write code differently and that most of the other code uses QBCore.Config. Simply trying to follow best practice, and that's just a few occasions where it's using QBConfig. I'm glad you found the cause of the bug though

Qwerty1Verified commented 1 year ago

Has it been tested since the convar check was added back in? It'll be nice to see a bug like this fixed

iboss21 commented 1 year ago

I m glad its fixed and servers owners as myself can be relaxed. Thank you guys and thanks to QBCore Community for this fast response and fixing it.

Truly appreciated.

gononono64 commented 1 year ago

Has it been tested since the convar check was added back in? It'll be nice to see a bug like this fixed

yes. have not tested the convar itself tho

GhzGarage commented 1 year ago

https://github.com/qbcore-framework/qb-core/commit/d7323ba147c6e135aa4df1910a3c4ff7eedac192