NOVA-Team / NOVA-Monorepo

The core API of the NOVA voxel game modding system
https://nova-team.github.io
GNU Lesser General Public License v3.0
66 stars 23 forks source link

Data Improvements and JSON Serialization #247

Closed ExE-Boss closed 7 years ago

ExE-Boss commented 7 years ago

Ready to merge

⚠️️ Please, don’t squash the two commits, as they are there to show the reason as to why JSON Processing was chosen over GSON.

Original content

This PR contains two implementations of #244. One with JSON Processing and one with GSON.

From writing them, GSON has simpler serialization but can’t losslessly deserialize ints and longs (can only deserialize doubles), while JSON Processing has simpler deserialization but can losslessly deserialize all primitive types. I’m putting them both here so that we can choose which one to include in NOVA. After we choose which one, it will be added to Data and the other one will be removed, then we can merge.

Comparison

I’d say that JSON Processing is better for the NOVA use case, but I want a second opinion on this first. We could also use JSON Processing for deserialization and GSON for serialization

JSON Processing

Pros Cons
Simpler deserialization than GSON Serialization has to be written twice (for Object and for Array)
Lossless number (de)serialization (every number is a BigDecimal) Slightly slower than GSON (will improve, as Oracle wants JSON Processing to be the default JSON API in Java)
Unchecked exceptions
Can tell ints and longs apart from doubles

GSON

Pros Cons
Serialization only needs to be written once Number (de)serialization is lossy (every number is a double)
Faster deserialization on small files Deserialization requires more code
Easier serialization customization Checked exceptions
Can't tell ints and longs apart from doubles

I’ve also fixed an issue with saving and loading booleans to NBT. If you previously tried to save both a byte and a boolean, the last to save would overwrite the type of the rest, which would cause a ClassCastException on load.

I’ve also fixed DataConverter attempting to call Data.put("class", value), which would throw an AssertionError, as key in Data.put(key, value) can’t be "class".


Closes #244, Probably fixes #179

codecov-io commented 7 years ago

Current coverage is 13.94% (diff: 3.05%)

Merging #247 into master will decrease coverage by 0.23%

@@             master       #247   diff @@
==========================================
  Files           394        393     -1   
  Lines         11150      11245    +95   
  Methods           0          0          
  Messages          0          0          
  Branches       1586       1661    +75   
==========================================
- Hits           1581       1568    -13   
- Misses         9483       9576    +93   
- Partials         86        101    +15   

Powered by Codecov. Last update 1de7378...303f11d