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

An option to disable references on tables in toJSON() #1074

Open
dmi7ry opened this issue Aug 25, 2019 · 6 comments
Open

An option to disable references on tables in toJSON() #1074

dmi7ry opened this issue Aug 25, 2019 · 6 comments
Labels
enhancement New feature or request

Comments

@dmi7ry
Copy link

dmi7ry commented Aug 25, 2019

Describe the bug
If there are links to same table then toJSON() will insert the table only ocne and all other times a link will be used (^T^):

run a = { this = "that" }
run toJSON({ alpha = a, beta = a })
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": "^T^1" } ] [string]

It may be OK for some cases, but if we want to use it outside of MTA, it's not useful at all. Most cases we want the result would be like this:

run a = { this = "that" }
run toJSON({ alpha = a, beta = a }, true, "none", true ) -- last `true` to diable references on tables
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": { "this": "that" } } ] [string]

To reproduce

run a = { this = "that" }
run toJSON({ alpha = a, beta = a })
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": "^T^1" } ] [string]

Expected behaviour

run a = { this = "that" }
run toJSON({ alpha = a, beta = a }, true, "none", true ) -- last `true` to diable references on tables
[16:19:52] Command results: [ { "alpha": { "this": "that" }, "beta": { "this": "that" } } ] [string]

Screenshots
none

Version
1.5.6

Additional context
I'm not sure this is a bug. If it will be an option (toJSON's arument), it is also OK.

A little discussion about the issue there

@dmi7ry dmi7ry added the bug Something isn't working label Aug 25, 2019
@jushar
Copy link
Contributor

jushar commented Aug 31, 2019

That's one of MTA's weird behaviours and was made to prevent recursive table constructs.

Not sure what we do about that. Just changing it would break breakwards compatibility, so we would at least need a parameter to control that. But I generally agree with you that the behaviour you described is the expected behaviour.
For the meantime, you could use a pure Lua JSON library: https://github.com/rxi/json.lua

@dmi7ry
Copy link
Author

dmi7ry commented Aug 31, 2019

That's one of MTA's weird behaviours and was made to prevent recursive table constructs.

I'm pretty sure it's just to reduce the size (there is similar things on element data, afair). And I'm not
sure how it may prevent recursive table constructs.

Just changing it would break breakwards compatibility

fromJSON still will work and result will be same for both cases (old and new). The result of improved toJSON also will be valid. So, how it would break something? Can you give an example?

For the meantime, you could use a pure Lua JSON library: https://github.com/rxi/json.lua

Sure. But any lua's implementation reduces performance. And it may be critical in some cases.

@patrikjuvonen
Copy link
Contributor

patrikjuvonen commented Aug 31, 2019

I'm pretty sure it's just to reduce the size (there is similar things on element data, afair). And I'm not
sure how it may prevent recursive table constructs.

If you have a table that refers to itself in the table, this causes an issue. This JSON behavior was introduced to prevent infinite referencing although in JS such is prevented by actual error. Problem is it doesn't seem to work right.

fromJSON still will work and result will be same for both cases (old and new). The result of improved toJSON also will be valid. So, how it would break something? Can you give an example?

It will break backwards compatibility in toJSON as per your issue description. Some people may be relying on the old format.

Also isn't this a duplicate of #1073?

@dmi7ry
Copy link
Author

dmi7ry commented Aug 31, 2019

Also isn't this a duplicate of #1073?

No. 1073 is about {}

@patrikjuvonen
Copy link
Contributor

Ah, you're right. Seems like we had discussion about this issue in there so I got confused.

@dmi7ry dmi7ry changed the title References on tables in toJSON() An option to disable references on tables in toJSON() Oct 13, 2023
@dmi7ry
Copy link
Author

dmi7ry commented Oct 13, 2023

Could someone change type from bug to enhancement?

@Lpsd Lpsd added enhancement New feature or request and removed bug Something isn't working labels Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants