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

V2 : Issues with using JSON for serialization #502

Closed
z3dev opened this issue Jan 22, 2020 · 10 comments
Closed

V2 : Issues with using JSON for serialization #502

z3dev opened this issue Jan 22, 2020 · 10 comments
Labels

Comments

@z3dev
Copy link
Member

z3dev commented Jan 22, 2020

Expected Behavior

Serialization of arrays is correct.

Actual Behavior

Arrays are being serialized as objects with "0" to "N" attributes.

{"sides":[[{"0":78.9363021850586,"1":-17.02008819580078},{"0":85.4400405883789,"1":0}],[{"0":85.4400405883789,"1":0},{"0":78.9363021850586,"1":17.02008819580078}],[{"0":78.9363021850586,"1":17.02008819580078},{"0":60.41522979736328,"1":31.449024200439453}],[{"0":60.41522979736328,"1":31.449024200439453},{"0":32.69648742675781,"1":41.09012985229492}],[{"0":32.69648742675781,"1":41.09012985229492},{"0":5.231693493002029e-15,"1":44.47563552856445}],[{"0":5.231693493002029e-15,"1":44.47563552856445},{"0":-32.69648742675781,"1":41.09012985229492}],[{"0":-32.69648742675781,"1":41.09012985229492},{"0":-60.41522979736328,"1":31.449024200439453}],[{"0":-60.41522979736328,"1":31.449024200439453},{"0":-78.9363021850586,"1":17.02008819580078}],[{"0":-78.9363021850586,"1":17.02008819580078},{"0":-85.4400405883789,"1":5.446694595454167e-15}],[{"0":-85.4400405883789,"1":5.446694595454167e-15},{"0":-78.9363021850586,"1":-17.02008819580078}],[{"0":-78.9363021850586,"1":-17.02008819580078},{"0":-60.41522979736328,"1":-31.449024200439453}],[{"0":-60.41522979736328,"1":-31.449024200439453},{"0":-32.69648742675781,"1":-41.09012985229492}],[{"0":-32.69648742675781,"1":-41.09012985229492},{"0":-1.5695080055489613e-14,"1":-44.47563552856445}],[{"0":-1.5695080055489613e-14,"1":-44.47563552856445},{"0":32.69648742675781,"1":-41.09012985229492}],[{"0":32.69648742675781,"1":-41.09012985229492},{"0":60.41522979736328,"1":-31.449024200439453}],[{"0":60.41522979736328,"1":-31.449024200439453},{"0":78.9363021850586,"1":-17.02008819580078}]],"transforms":{"0":0.3511234521865845,"1":-0.936329185962677,"2":0,"3":0,"4":0.936329185962677,"5":0.3511234521865845,"6":0,"7":0,"8":0,"9":0,"10":1,"11":0,"12":130,"13":180,"14":0,"15":1}}

The geom2.isA() and geom3.isA() functions are failing because the objects are being brought back with transforms being an object, not an array.

Steps to Reproduce the Problem

  1. Load any design with a simple shape, like cube.
  2. There are no downloadable formats.

Specifications

  • Version: V2
  • Platform: WEB
  • Environment: All browsers
@z3dev
Copy link
Member Author

z3dev commented Jan 22, 2020

@kaosat-dev this is unexpected behaviour. the small Float32Array values are being converted into objects. even the vec2/vec3 arrays are being converted to objects.

@z3dev
Copy link
Member Author

z3dev commented Jan 22, 2020

JSON serializes simple arrays. So, one solution is to replace the Float32Array values with simple arrays. For example vec2.create() returns [0, 0]

@kaosat-dev
Copy link
Contributor

@z3dev I stumbled upon this problem when converting the web code to V2 and fixed it in some way (need to look it up): you are speaking about json.stringify(someJscadStuff) and not the weird old 'json serializer' right ?

@z3dev
Copy link
Member Author

z3dev commented Jan 22, 2020

I believe that JSON.stringify() is making object strings, not arrays strings.

@z3dev
Copy link
Member Author

z3dev commented Jan 22, 2020

I just read the specification for stringify(). It handles Arrays and typed arrays very different. Typed arrays are treated like objects, which enumerates across the attributes. UG

@kaosat-dev
Copy link
Contributor

Ugh indeed, so there are solutions (you can pass a function to stringify/parse to handle specific types, simple enough), but it is never going to be 'just' JSON.stringify

@kaosat-dev
Copy link
Contributor

then again @z3dev this is only needed when transfering data from the webWorker right ? (ie replace to/from compactBinary)

@z3dev
Copy link
Member Author

z3dev commented Jan 22, 2020

then again @z3dev this is only needed when transfering data from the webWorker right ? (ie replace to/from compactBinary)

Correct. Maybe we need the compact binary format again... did you add this to the list?

@kaosat-dev
Copy link
Contributor

yes and no
I am fine with a less optimal solution (custom function passed to serialize/parse) for now.
I can create a quick PR for now for this, and we can revisit later when needed.
(part of why I say this, is that I am looking for ways that would allow us to stay 100% inside web workers, so no more back & forth)

@z3dev z3dev added the V2 label May 15, 2020
@z3dev
Copy link
Member Author

z3dev commented Aug 4, 2020

With the move to arrays of Numbers, this is not longer an issue. Closing.

@z3dev z3dev closed this as completed Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants