fix: RemoveKeys feat: admin command hooks #49

artur-michalak · 2024-05-11T13:50:48Z

Description

Admin commands moved to qbx_adminmenu.
fixed RemoveKeys function
Distance to vehicle based on maxDistance in lockpickDoor

Checklist

I have personally loaded this code into an updated Qbox project and checked all of its functionality.
My pull request fits the contribution guidelines & code conventions.

…dmin commands

Manason

I'm not sure it makes sense for admin menu to own vehicle key commands. Those feel like they should stay in vehicle keys, no?

artur-michalak · 2024-05-11T15:31:29Z

Based on the following information, I understood that moving the commands to qbx_adminmenu would be a good solution
Qbox-project/qbx_smallresources#99 (comment)

I don't see the difference between the command for thirst level and giving keys.

BerkieBb · 2024-05-11T15:33:43Z

Thirst level is built into the core whereas vehiclekeys is an optional resource

artur-michalak · 2024-05-11T15:44:06Z

So qbx_smallresurces should be treated as part of qbx_core?

artur-michalak · 2024-05-11T15:57:12Z

What about restricted = 'group.admin? This is less flexible than restricted = config.dev.

Manason · 2024-05-11T18:51:42Z

I have a philosophical concept of ownership. Each resource is modular, so may or may not be present on a server. While a resource can depend on other resources, dependencies limit the independence of a resource, and so should be minimized. We tend to use dependencies for cross-resource libraries and centralized handling of certain things.

So if one were to disable or remove qbx_vehiclekeys, it makes sense that the admin commands are also disabled. On a practical level, maintaining the admin functionality for a resource is easy to do inside the resource itself.

admin menu is a weird resource with a bit of an ownership problem. Similar to a resource like radial menu, ideally it would own nothing except the UI. So to add to the UI, other resources would call exports, creating a plugin like environment. In reality, resource owned functionality can be found in these UI type resources. Admin menu is extra tricky, as it currently contains some admin functionality in addition to its UI purpose. There's a good argument such functionality should be moved into core or separated out into other resources. Most of it should be able to be owned by txAdmin.

artur-michalak · 2024-05-11T19:26:21Z

I fully agree with the independence between modules. That's why I originally preferred to put the food-related commands into qbx_smallresources. How about applying the observer pattern?

server/commands.lua

server/main.lua

Manason · 2024-05-11T19:42:59Z

I fully agree with the independence between modules. That's why I originally preferred to put the food-related commands into qbx_smallresources. How about applying the observer pattern?

You mean use events? In what way are you thinking? I believe hunger/thirst data would be owned by core, so it would be appropriate to move the commands there.

Co-authored-by: Manason <clanerp@gmail.com>

artur-michalak · 2024-05-11T20:58:17Z

I had in mind a list of events or exports in the qbx_adminmenu, but maybe a simple factory in qbx_adminmenu called by resources would be a better solution.

…tations unification

Manason · 2024-05-11T22:39:43Z

I had in mind a list of events or exports in the qbx_adminmenu, but maybe a simple factory in qbx_adminmenu called by resources would be a better solution.

Ok. I think I understand you are talking about different ways to interface with adminmenu.

Here are the two options on the top of my mind:

adminmenu triggers events, other resources handle them to do things.
adminmenu exposes an export to include a new item on the menu with a callback export/event to trigger

artur-michalak · 2024-05-11T22:47:31Z

The first option seems more flexible. Some callback would have to be given there on the admin side.
The second option would be more compact and more consistent with the rest of the framework.

* fix: small issues - Added getVehicleByPlate function - Adjusted main client loop to check for player ownership when parked - Added plate to givekey - Fixed commands not grabbing name from locale - Made vehicleList be populated by the database on start * fix: distance check and mysql import * fix(server/commands): args inconsistenties * fix(server): invalid players * fix(client): speed up event to prevent driving without keys * fix: add notifications to commands * fix(server/main): add back extra callback * feat(locales): new entry * tweak(server/commands): add error notification types * tweak(client/main): flip ternary operator * fix(server/main): only load spawned vehicles * style(client/main): return if statement to multiline * fix(client/main): check ownership through state

…code/qbx_vehiclekeys into hedge-code-feat-admin-commands-hooks

…yLista state on server fix: lock of packend vehicles refactor: move server function to separate file

server/functions.lua

Manason

There's a lot more complexity with keys as items, recommending splitting that into its own subsequent PR

server/functions.lua

BerkieBb · 2024-05-14T22:14:52Z

client/main.lua

+    if not vehicle then return end
+    if not isBlacklistedVehicle(vehicle) then
+        if hasKeys(qbx.getVehiclePlate(vehicle)) or areKeysJobShared(vehicle) then
+


blank space can go

Improves readability by separating animation from the rest of the context

BerkieBb · 2024-05-14T22:15:18Z

client/main.lua

-                ClearPedTasks(cache.ped)
+            local lockstate
+            if state then
+                lockstate = state == true and 2 or 1


The == true part can be removed, it will function the same

The == true improves readability

That is how we write all of our ternary operators

I found what the problem was

server/commands.lua

server/functions.lua

BerkieBb · 2024-05-14T22:22:51Z

server/functions.lua

+function public.removeKeys(source, plate)
+    local citizenid = getCitizenId(source)
+
+    if not citizenid then return end
+
+    local keys = Player(source).state.keysList or {}
+
+    if not keys[plate] then return end
+    keys[plate] = nil
+
+    Player(source).state:set('keysList', keys, true)
+
+    exports.qbx_core:Notify(source, locale('notify.keys_removed'))
+
+    return true
+end
+
+function public.hasKeys(source, plate)
+    return Player(source).state.keysList[plate]
+end
+
+---Gives the user the keys to the vehicle
+---@param source number ID of the player
+---@param plate string The plate number of the vehicle.
+function public.giveKeys(source, plate)
+    local citizenid = getCitizenId(source)
+
+    if not citizenid then return end
+
+    local keys = Player(source).state.keysList or {}
+
+    if keys[plate] then return end
+    keys[plate] = true
+
+    Player(source).state:set('keysList', keys, true)
+
+    exports.qbx_core:Notify(source, locale('notify.keys_taken'))
+
+    return true
+end


These should also update the keys lists

These shouldn't.

this keeps the state of the keys only log off characters

artur-michalak added 2 commits May 11, 2024 15:32

fix: RemoveKeys feat: removeKeys and giveKeys command hooks, remove a…

6d3d3d2

…dmin commands

chore: RemoveKeys summary

bfebf18

Manason requested changes May 11, 2024

View reviewed changes

feat: restore admin commands

707e419

artur-michalak force-pushed the feat-admin-commands-hooks branch from eebd943 to 707e419 Compare May 11, 2024 16:01

artur-michalak mentioned this pull request May 11, 2024

feat: qbx_vehiclekeys admin commands Qbox-project/qbx_adminmenu#56

Closed

2 tasks

artur-michalak requested a review from Manason May 11, 2024 16:06

fix: admin commands locales

c8700e4

Manason requested changes May 11, 2024

View reviewed changes

artur-michalak and others added 2 commits May 11, 2024 22:02

refactor: remove semicolon at commands.lua GetVehiclePedIsIn

f9c14b3

Co-authored-by: Manason <clanerp@gmail.com>

refactor: Null-coalescing operator at playerId in command

fd82c7e

Co-authored-by: Manason <clanerp@gmail.com>

refactor: single responsibility of getPlayersVehiclePlate chore: anno…

b9cff55

…tations unification

feat: remove relic net events

bb68347

Manason approved these changes May 12, 2024

View reviewed changes

BerkieBb and others added 4 commits May 12, 2024 14:33

Merge branch 'feat-admin-commands-hooks' of https://github.com/hedge-…

e4ef778

…code/qbx_vehiclekeys into hedge-code-feat-admin-commands-hooks

fix: AddEventHandlers to RegisterNetEvent

51be3c9

feat: commands translations, restore optional arguments and remove ke…

6d1c201

…yLista state on server fix: lock of packend vehicles refactor: move server function to separate file

artur-michalak requested a review from Manason May 13, 2024 17:29

artur-michalak commented May 13, 2024

View reviewed changes

server/functions.lua Outdated Show resolved Hide resolved

Manason reviewed May 13, 2024

View reviewed changes

server/functions.lua Outdated Show resolved Hide resolved

server/functions.lua Outdated Show resolved Hide resolved

server/functions.lua Show resolved Hide resolved

server/functions.lua Show resolved Hide resolved

artur-michalak added 5 commits May 14, 2024 17:31

refactor: remove keys item related functions

0f56e45

chore: restore fpid locale key

ee4acc5

chore: tabs to spaces

4573eb6

feat: logout characters keys memory

8d824b2

chore: remove debug print from cron

bb05905

BerkieBb suggested changes May 14, 2024

View reviewed changes

artur-michalak added 2 commits May 15, 2024 16:52

feat: ox cron, playerId in commands as mandatory

223d71c

fix: lockstate if state nil

aad28a4

Manason merged commit 37b2a82 into Qbox-project:main May 16, 2024
1 of 2 checks passed

artur-michalak deleted the feat-admin-commands-hooks branch June 10, 2024 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: RemoveKeys feat: admin command hooks #49

fix: RemoveKeys feat: admin command hooks #49

artur-michalak commented May 11, 2024

Manason left a comment

artur-michalak commented May 11, 2024

BerkieBb commented May 11, 2024

artur-michalak commented May 11, 2024

artur-michalak commented May 11, 2024 •

edited

Loading

Manason commented May 11, 2024

artur-michalak commented May 11, 2024

Manason commented May 11, 2024

artur-michalak commented May 11, 2024 •

edited

Loading

Manason commented May 11, 2024

artur-michalak commented May 11, 2024 •

edited

Loading

Manason left a comment

BerkieBb May 14, 2024

artur-michalak May 15, 2024

BerkieBb May 14, 2024

artur-michalak May 15, 2024

BerkieBb May 15, 2024

artur-michalak May 15, 2024 •

edited

Loading

BerkieBb May 14, 2024

artur-michalak May 15, 2024

BerkieBb May 15, 2024

artur-michalak May 15, 2024 •

edited

Loading

fix: RemoveKeys feat: admin command hooks #49

fix: RemoveKeys feat: admin command hooks #49

Conversation

artur-michalak commented May 11, 2024

Description

Checklist

Manason left a comment

Choose a reason for hiding this comment

artur-michalak commented May 11, 2024

BerkieBb commented May 11, 2024

artur-michalak commented May 11, 2024

artur-michalak commented May 11, 2024 • edited Loading

Manason commented May 11, 2024

artur-michalak commented May 11, 2024

Manason commented May 11, 2024

artur-michalak commented May 11, 2024 • edited Loading

Manason commented May 11, 2024

artur-michalak commented May 11, 2024 • edited Loading

Manason left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artur-michalak May 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

artur-michalak May 15, 2024 • edited Loading

Choose a reason for hiding this comment

artur-michalak commented May 11, 2024 •

edited

Loading

artur-michalak commented May 11, 2024 •

edited

Loading

artur-michalak commented May 11, 2024 •

edited

Loading

artur-michalak May 15, 2024 •

edited

Loading

artur-michalak May 15, 2024 •

edited

Loading