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

Code Maintenance: Remove references to JSONObject / JSONArray #1355

Open
Merudo opened this issue Mar 4, 2020 · 8 comments
Open

Code Maintenance: Remove references to JSONObject / JSONArray #1355

Merudo opened this issue Mar 4, 2020 · 8 comments
Assignees
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.

Comments

@Merudo
Copy link
Member

Merudo commented Mar 4, 2020

Is your feature request related to a problem? Please describe.
Now that 90%+ of the JSON has been converted to gson, it may be a good time to remove all references to JSONObjects / JSONArray.

I found some references in AppUpdate.java, MTWebClientManager, MTWebAppServer, WebAppInitiative, WebTokenInfo, and JSONMacroFunctions.

@Phergus Phergus added the code-maintenance Adding/editing javadocs, unit tests, formatting. label Mar 4, 2020
Merudo added a commit to Merudo/maptool that referenced this issue Mar 6, 2020
Merudo added a commit to Merudo/maptool that referenced this issue Mar 6, 2020
- Replace JSONObject / JSONArray with gson elements in WebApp
- Discussed in RPTools#1355
@Merudo
Copy link
Member Author

Merudo commented Mar 7, 2020

The JSONArray / JSONObject imports left are:

JSONMacroFunctions (for json.indent)
JSONMacroFunctionsOld
TestJSONMacroFunctions

@Phergus
Copy link
Contributor

Phergus commented May 10, 2020

@Merudo So it appears that we could replace the calls in jsonIndent() with:

    Gson gson = new GsonBuilder().setPrettyPrinting().create();
    return gson.toJson(json);

The limitation being that it uses a fixed 2 character indent. We would have to create our own JsonPrintFormatter that accepted an indent level. Personally, I'm fine with a fixed 2 char indent.

Thoughts? @cwisniew @JamzTheMan

Another issue is that it doesn't seem to be handling arrays of objects well but that is perhaps an issue with how the json info is being put together by getInfo("campaign").

  {
	"name": "Candle - 5",
	"max range": 10.0,
	"type": "NORMAL",
	"light segments": [
	  "{\"facingOffset\":0.0,\"radius\":5.0,\"arcAngle\":360.0,\"isGM\":false,\"ownerOnly\":false}",
	  "{\"paint\":{\"color\":1677721600},\"facingOffset\":0.0,\"radius\":10.0,\"arcAngle\":360.0,\"isGM\":false,\"ownerOnly\":false}"
	]
  },

How it looks currently. (Which is actually pretty ugly just without the escaping of quotes.)

    {
"name": "Candle - 5",
"max range": 10,
"type": "NORMAL",
"light segments":         [
				{
		"facingOffset": 0,
		"radius": 5,
		"arcAngle": 360,
		"isGM": false,
		"ownerOnly": false
	  },
				{
		"paint": {"color": 1677721600},
		"facingOffset": 0,
		"radius": 10,
		"arcAngle": 360,
		"isGM": false,
		"ownerOnly": false
	  }
	]
  },

@JamzTheMan
Copy link
Member

Im ok with a 2 char indent limitation. I dont see that "breaking" anything major.

I almost wish json indenting was the default when ever we print out a var that was a json to be honest...

@Phergus
Copy link
Contributor

Phergus commented May 10, 2020

It is something weird about how getInfo(campaign) or the functions it calls is assembling the lights data.

This:

[h: myJson = '{"name": "Candle - 5","max range": 10,"type": "NORMAL","light segments":[{"facingOffset": 0,"radius": 5,"arcAngle": 360,"isGM": false,"ownerOnly": false},{"paint": {"color": 1677721600},"facingOffset": 0,"radius": 10,"arcAngle": 360,"isGM": false,"ownerOnly": false}]}']
  
<pre>[r: json.indent(myJson)]</pre>

Works fine with the gson formatter.

{
  "name": "Candle - 5",
  "max range": 10,
  "type": "NORMAL",
  "light segments": [
    {
      "facingOffset": 0,
      "radius": 5,
      "arcAngle": 360,
      "isGM": false,
      "ownerOnly": false
    },
    {
      "paint": {
        "color": 1677721600
      },
      "facingOffset": 0,
      "radius": 10,
      "arcAngle": 360,
      "isGM": false,
      "ownerOnly": false
    }
  ]
}

@JamzTheMan
Copy link
Member

There is an extra comma at the end if the original or did you not paste everything?

@Phergus
Copy link
Contributor

Phergus commented May 10, 2020

It's just a snippet of the whole thing.

Found that the light segments are being returned from getInfo() like this:

"light segments":[
"{\"facingOffset\":0.0,\"radius\":5.0,\"arcAngle\":360.0,\"isGM\":false,\"ownerOnly\":false}",
"{\"paint\":{\"color\":1677721600},\"facingOffset\":0.0,\"radius\":10.0,\"arcAngle\":360.0,\"isGM\":false,\"ownerOnly\":false}"
]

So that's a separate issue entirely.

@Phergus
Copy link
Contributor

Phergus commented May 12, 2020

See #1799 for light segments fix.

@cwisniew cwisniew added this to the 1.8.0 milestone May 24, 2020
@Phergus Phergus removed this from the 1.8.0 milestone Feb 7, 2021
@Phergus Phergus self-assigned this May 22, 2021
@Phergus
Copy link
Contributor

Phergus commented May 31, 2021

Moving to next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code-maintenance Adding/editing javadocs, unit tests, formatting.
Projects
None yet
Development

No branches or pull requests

4 participants