UnderwaterApps / overlap2d

Overlap2D Game development toolkit for UI and Level design
Other
780 stars 224 forks source link

PrettyPrint MainScene.dt andproject.dt #270

Open dcman opened 9 years ago

dcman commented 9 years ago

PrettyPrint MainScene.dt andproject.dt so that if you have to look at them it is not a single line of text.

dcman commented 9 years ago

I fixed it and will submit a pull request.

dsaltares commented 9 years ago

I don't think this is a good idea. Speed and low file size is better than readability for saved files. You can always open the JSON with an editor and quickly format it with a shortcut.

dcman commented 9 years ago

Write speed shouldn't be an issue with the two files. From what I can see they are only generated once at export. Also the code for the PP operation was already there and was run each time you exported the file anyways. There was just a bug in the code preventing it from saving the file that way.

dsaltares commented 9 years ago

Write speed shouldn't be an issue with the two files

I meant read speed and final file size. Maybe it could be an advanced option on export that's sticky, so they user doesn't need to keep setting it.

azakhary commented 9 years ago

There are mainly 2 reasons I see for pretty printing the JSON 1) This might (or might not) make it easier for VCS to merge scene commits. If it is worked by a team. 2) If something went wrong with scene due to overlap2d bug, it's easier to fix it "by hand"

Is there 3rd reason I am missing?

number 1 - I am not really sure about it, sometimes even with pretty print it gives weird merge conflicts that can't be fixed.

and number 2 - we should work towards eliminating this bugs if they ever happen which did not happen a lot. (I do not see issue reports about corrupt scene files)

So, given both reasons to pretty print are not so much good points, the only reason against it is - why not keep files small?

dsaltares commented 9 years ago

This might (or might not) make it easier for VCS to merge scene commits. If it is worked by a team.

If we really want to achieve this, the best way is to split a project in multiple files. We'd have to decide on the granularity.

In CryEngine (from my time at Crytek), level files are split in layers, each layer was a separate XML file. Several designers could work on the same level and check files into source control without conflicts because each one of them would work on a separate layer.

I guess this is worth a separate ticket and a more in depth discussion. Definitely, this is the way to support multiple team members in Overlap2D.

azakhary commented 9 years ago

Makes sense. Maybe some other time we can separate layers into files. There is also library items that can be in both either scene file, or in one parent project file. I'll open a separate ticket about overall data structure discussion I guess, as for this one. There is a pull request waiting in runtime to pretty print, but I guess we should not do it. Other solutions are required, but there is no practical reason to pretty print, unless 2 reasons written above that require different solutions. So, if @dcman is ok with this, I will close this issue maybe?

dcman commented 9 years ago

If pretty print isn't going to get pulled into run time then you can close this. Side note did some testing... File sizes for an object saved in pretty print format and standard

PPtext.txt 263864 Jsontext.txt 259291 Run times in milliseconds PP Json string to object: 0: 17 1: 14 2: 14 3: 16 4: 33 5: 21 6: 14 7: 13 8: 14 9: 10 Average: 16Ms Standard Json string to object: 0: 12 1: 12 2: 15 3: 14 4: 12 5: 18 6: 15 7: 17 8: 8 9: 10 Average: 13Ms Object to json standard string: 0: 18 1: 17 2: 22 3: 19 4: 19 5: 25 6: 31 7: 22 8: 18 9: 15 Average: 20Ms Object to PP json string: 0: 48 1: 41 2: 48 3: 77 4: 50 5: 36 6: 35 7: 33 8: 37 9: 44 Average: 44Ms

On Mon, Oct 26, 2015 at 3:59 AM, Avetis notifications@github.com wrote:

Makes sense. Maybe some other time we can separate layers into files. There is also library items that can be in both either scene file, or in one parent project file. I'll open a separate ticket about overall data structure discussion I guess, as for this one. There is a pull request waiting in runtime to pretty print, but I guess we should not do it. Other solutions are required, but there is no practical reason to pretty print, unless 2 reasons written above that require different solutions. So, if @dcman https://github.com/dcman is ok with this, I will close this issue maybe?

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d/issues/270#issuecomment-151099853 .

azakhary commented 9 years ago

Btw, not only the loading time, but we had errors reported from some android devices when json reading ended up doing out of memory on device. (I assume it takes more space during reading procedure then after it's done) (This is on some weak android devices, which are in vast quantities in some countries)

dcman commented 9 years ago

I don't doubt that. The test code I wrote worked well on my Nexus 5X but hit the garbage collector hard on my N4 and N7. Almost want to pull out my G1 and test it. On Oct 26, 2015 11:32 PM, "Avetis" notifications@github.com wrote:

Btw, not only the loading time, but we had errors reported from some android devices when json reading ended up doing out of memory on device. (I assume it takes more space during reading procedure then after it's done) (This is on some weak android devices, which are in vast quantities in some countries)

— Reply to this email directly or view it on GitHub https://github.com/UnderwaterApps/overlap2d/issues/270#issuecomment-151392264 .

nicolasdiehl commented 8 years ago

Couldn't we have the best of two worlds by pretty-printing the project data and compressing on export?