Skip to content

Conversation

@137ElPro
Copy link

No description provided.

Signed-off-by: 137ElPro <103851494+137ElPro@users.noreply.github.com>
Refactor group rank checking to handle multiple requirements.

Signed-off-by: 137ElPro <103851494+137ElPro@users.noreply.github.com>
Signed-off-by: 137ElPro <103851494+137ElPro@users.noreply.github.com>
@Maximus7474
Copy link

What's the purpose for this ?
Description would be decent to explain why you made the changes and what it implies.

@137ElPro
Copy link
Author

The purpose of this change is to improve group-grade validation.
Currently, ox_inventory only supports checking a minimum grade, which means every higher grade automatically gets access — even in cases where that isn’t desired.

This PR adds support for a grade table, allowing access to be defined for specific grades instead of relying on the minimum-grade logic.
This gives more granular control and covers edge cases where certain higher grades should not inherit access automatically.

In short: it adds more precise permission handling by allowing explicit grade-based rules.

@cesarmr-github
Copy link

I've been testing it and it doesn't seem to have any problems

Copy link

@Maximus7474 Maximus7474 left a comment

Choose a reason for hiding this comment

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

Logic is sound and resolves the targeted issue.

Only change I'd do is have and extra if statement to check groupRank prior to checking the req variable type. I'd also consider renaming it for better readability, to maybe something like requiredRank.

At this stage, it also makes the group parameter of the function more complex having now multiple data types so adding some luadoc to define explicitly it's contents would also be a decent addition in my opinion.

This is personal feedback on the changed code, not a required change requested by COX to approve it's merge.

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.

3 participants