Skip to content

Conversation

@Jordan2139
Copy link
Member

Description

Motivation and Context

How Has This Been Tested?

Contributor License Agreement

By submitting this pull request, you certify that you have read and agree to the project’s Contributor License Agreement (CLA) under the PolyForm Noncommercial 1.0.0 license.

I, , hereby agree to license my contributions to SonoranCADFiveM under the PolyForm Noncommercial License 1.0.0.

(Replace <GitHub Handle> with your GitHub username.)

Checklist

  • My code follows the project’s coding style and conventions
  • I have updated documentation where necessary
  • I have read, signed, and agreed to the Contributor License Agreement (CLA)

@Jordan2139
Copy link
Member Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 56815ad35c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 330 to 334
AddEventHandler("playerDropped", function()
local changed = false
for vehNet, owner in pairs(displayOwners) do
if owner == source then
displayOwners[vehNet] = nil

Choose a reason for hiding this comment

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

P2 Badge CAD display ownership never released after owner leaves vehicle

Ownership of a vehicle’s CAD display is stored per vehicle and only cleared on player disconnect (see playerDropped handler). Because takeover in ClaimDisplay requires the current owner to approve, if a player claims a display and simply exits the vehicle, they remain owner indefinitely; takeover requests from new occupants will keep timing out and the display will never update screenshots for them. Ownership needs to be released when the vehicle is vacated or the owner leaves so subsequent occupants can control the display.

Useful? React with 👍 / 👎.

@Jordan2139
Copy link
Member Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8d88403af

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines 765 to 768
end

TriggerServerEvent("SonoranCAD::caddisplay::ClaimDisplay", vehNet)
notify("You are now controlling this CAD display.")

Choose a reason for hiding this comment

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

P1 Badge Send seat when claiming display control

The client sends SonoranCAD::caddisplay::ClaimDisplay with only the vehicle net ID and never passes the player’s seat (TriggerServerEvent("...", vehNet)), so when a display already has an owner the server sees claimedSeat == nil and rejects the takeover with “Only the driver or front passenger can take control.” (see sv_caddisplay.lua lines 249‑276). As a result, even the driver/front passenger cannot request control from an existing owner, making the handoff flow unusable whenever a display is already claimed.

Useful? React with 👍 / 👎.

@Jordan2139
Copy link
Member Author

@codex review

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 683b2970c2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +59 to +63
local function checkPermissions(src)
if not pluginConfig.commands.restricted then
return true, true
end

Choose a reason for hiding this comment

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

P1 Badge Default permission check grants admin to all users

When commands.restricted is false (the default in caddisplay_config.dist.lua), checkPermissions returns (true, true), marking every caller as an admin. That means any player can call SavePlacement/DeletePlacement and persistently add or remove CAD display placements without ACE/framework/custom approval. This effectively bypasses the intended admin-only guard as soon as the feature is enabled.

Useful? React with 👍 / 👎.

Comment on lines 780 to 783
local seat = getSeatIndexForPed(veh, ped)
TriggerServerEvent("SonoranCAD::caddisplay::ClaimDisplay", vehNet, seat)
notify("You are now controlling this CAD display.")
claimedOnce[tostring(vehNet)] = true

Choose a reason for hiding this comment

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

P2 Badge Client shows control success before server approval

The interact command always notifies "You are now controlling this CAD display" and marks the vehicle as claimed immediately after sending SonoranCAD::caddisplay::ClaimDisplay, without waiting for the server response. If the server rejects or defers the claim (e.g., another owner exists and you are not the driver/front passenger, or the current owner denies), the player still sees a success message and the prompt is suppressed even though control was not granted.

Useful? React with 👍 / 👎.

@Jordan2139 Jordan2139 merged commit fa64cf8 into master Jan 13, 2026
2 checks passed
@Jordan2139 Jordan2139 deleted the feat-caddisplay branch January 13, 2026 23:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants