danini-the-panini / mittsu

3D Graphics Library for Ruby.
https://github.com/danini-the-panini/mittsu
MIT License
508 stars 33 forks source link

JSON export bugfixes #121

Open Floppy opened 6 months ago

Floppy commented 6 months ago

I was trying to debug my 3MF exporter using to_json, and found a few bugs that tripped it up. These changes fix those, so that running to_json on a loaded male02.obj file now works. No extra tests added, just a few quick tactical fixes that seem fairly obvious.

Incidentally, to_json in other contexts would normally produce a string, whereas this creates a hash which then needs JSON.generate() calling on it. Maybe something to reconsider down the line, what the "correct" output from to_json should be.

Floppy commented 6 months ago

The test failure here is because there is an extra level of object in the children array now. Rather than just containing objects, it contains complete to_json output from the child objects, including a metadata section. I'll update the code to pass, but I'm wondering, what should this be outputting? Should the metadata be in there? Or should it just be the contents of the "object" part of the child?

Floppy commented 6 months ago

Trying to phrase my question better. Which is the correct output?

{
  :metadata=>{
    :version=>4.3, 
    :type=>"Object", 
    :generator=>"ObjectExporter"
  }, 
  :object=>{
    :uuid=>"c52c90fd-8f3c-43ec-967f-4714b6a4a482", 
    :type=>"Object3D", 
    :matrix=>[1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0], 
    :children=>[
      {
        :uuid=>"cedd4b1e-e86d-4eb2-8591-314c5cec2e2a", 
        :type=>"Object3D", 
        :matrix=>[1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0]
      }
    ]
  }
}

or

{
  :metadata=>{
    :version=>4.3, 
    :type=>"Object", 
    :generator=>"ObjectExporter"
  }, 
  :object=>{
    :uuid=>"c52c90fd-8f3c-43ec-967f-4714b6a4a482", 
    :type=>"Object3D", 
    :matrix=>[1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0],
    :children=>[
      {
        :metadata=>{
          :version=>4.3, 
          :type=>"Object", 
          :generator=>"ObjectExporter"
        }, 
        :object=>{
          :uuid=>"cedd4b1e-e86d-4eb2-8591-314c5cec2e2a", 
          :type=>"Object3D", 
          :matrix=>[1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 0.0, 1.0]
        }
      }
    ]
  }
}
danini-the-panini commented 6 months ago

The first one, we don't need the metadata on every child object

Floppy commented 6 months ago

Great, thanks, I'll update the code :)