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: qb hunger/thirst/stress item compatibility #1178

Merged
merged 13 commits into from
Mar 5, 2023

Conversation

Manason
Copy link
Contributor

@Manason Manason commented Mar 3, 2023

  • add qb compatibility for set player status for hunger, thirst, and stress changing items
  • replaced qb item conversion approach. Instead of hard coding the keys of an item table entry, the entry table itself is serialized. This has numerous benefits: table keys which are nil will not be included; the old approach required explicit nils making the item entries larger than needed. Additionally the serialization function does some optimizing of table structure such as replacing [""] with just the key names, or removing explicit key entries for an array. This also makes the code easier to extend as future writers need only to convert one table to another rather than concerning with how the table values will be serialized. The downside is chiefly that functions are not supported currently. To accomplish this a new Util function TableToString has been added, as a wrapper of Serpent

@thelindat
Copy link
Member

The serialisation thing you added seems somewhat unnecessary if it's just for the sake of removing explicit nil values and using short index notation; and I'd prefer the string notation for consistency when there are items with names that can't be expressed otherwise.

The item converter as it exists does need work and was acceptable enough up until I made the resource framework agnostic and qb support was added (a lot of code duplication). I'd rather just have a function tailored specifically for the current output instead of this serpent thingy.

@Manason
Copy link
Contributor Author

Manason commented Mar 3, 2023

The serialisation thing you added seems somewhat unnecessary if it's just for the sake of removing explicit nil values and using short index notation; and I'd prefer the string notation for consistency when there are items with names that can't be expressed otherwise.

The item converter as it exists does need work and was acceptable enough up until I made the resource framework agnostic and qb support was added (a lot of code duplication). I'd rather just have a function tailored specifically for the current output instead of this serpent thingy.

My issue is without removing explicit nil you'd end up with

client = { status = { hunger = nil, thirst = nil, stress = nil, } }

added to every item. I get that Serpent is an unsightly block of code to add, but the results are an improvement. The item names are still done using string notation, it's the item body that's converted serpent. I suppose I could hack up the serpent code to remove the features we aren't using and make it more readable, but it just doesn't seem worth it given that it just works as is now. Would you rather we just have default values for all config entries and make the config larger instead of using serialization?

@thelindat
Copy link
Member

thelindat commented Mar 3, 2023

My issue is without removing explicit nil you'd end up with

Which can be resolved without adding serpent, and I'm not saying you have to resolve this but it could probably be as simple as json.encode and then replacing json syntax with Lua table (hasn't been important enough for me to bother reworking or even thinking about solutions for).

client = { status = { hunger = nil, thirst = nil, stress = nil, } }

Does qb items even contain hunger/thirst/etc values to convert over?

Serpent is an unsightly block of code to add, but the results are an improvement

I just generally like to look for my own solutions to problems and dislike adding dependencies as an easy-solution (hello nodejs).

@Manason
Copy link
Contributor Author

Manason commented Mar 3, 2023

Which can be resolved without adding serpent, and I'm not saying you have to resolve this but it could probably be as simple as json.encode and then replacing json syntax with Lua table (hasn't been important enough for me to bother reworking or even thinking about solutions for).

It's not worth my time to re-invent the wheel here. I'd rather just take the explicit nils for now if you don't want to add a dependency. Let me know for sure and I'll change the PR to follow suit.

Does qb items even contain hunger/thirst/etc values to convert over?

The default items don't, but the server I develop for has a bunch of items that do have the values. Probably from Jim's stuff, if I had to guess.

@thelindat
Copy link
Member

thelindat commented Mar 3, 2023

It's not worth my time to re-invent the wheel here. I'd rather just take the explicit nils for now

nor would I expect you to; but a simple enough fix for nil values would be gsub to match and remove any nil fields, which I can setup later. Alternatively just concatenating each field when needed.

@Manason
Copy link
Contributor Author

Manason commented Mar 3, 2023

nor would I expect you to; but a simple enough fix for nil values would be gsub to match and remove any nil fields, which I can setup later. Alternatively just concatenating each field when needed.

There are edge cases such as removing the status key altogether if the child keys are all nil. In any case, I removed the serpent code and just used the existing string format approach with explicit nils.

@thelindat thelindat merged commit 600fd15 into overextended:main Mar 5, 2023
@Manason Manason deleted the eat-drink-compat branch March 5, 2023 08:55
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