emilk / wfc

A C++ port of Wave Function Collapse Tiling
324 stars 34 forks source link

Support JSON config files #2

Open VelocityRa opened 7 years ago

VelocityRa commented 7 years ago

Make this project able to accept configuration in XML just like the original, so that porting of config files isn't necessary. As for the library, TinyXML 2 might be a good candidate.

VelocityRa commented 7 years ago

So, I'm thinking of attempting this, would you @emilk be ok with a tinyxml2 dependency? It's on Github so it can be added easily as a submodule. Unless there's any other way you'd rather do this?

emilk commented 7 years ago

tinyxml2 sounds good, and a submodule sounds good too, so go ahead!

...unless you'd like to switch to JSON in both C++ and C#? It would be trivial in C++ (.cfg is basically nice-looking JSON anyway). I personally prefer JSON to XML, by a lot. In particular, I really don't like how you have to encode numbers as strings in XML. Still, if you prefer XML, that's fine.

VelocityRa commented 7 years ago

I dislike XML too, but that's what the C# version uses, so if someone tried that version first and then tried this version (maybe C# proved to be too slow), they'd have to convert their config file.

So are you suggesting we implement JSON compatibility for both versions instead? The goal for me is for both versions to have a common config system, not have it be XML per se. I'm not that familiar with C#, but I could probably handle JSON on the other repo, would also be easier for you here since you're obviously familiar with all this (configuru included).

So do you want to agree on a JSON scheme/format and implement it seperately on both versions?

emilk commented 7 years ago

I think that would be great - I started porting to JSON now, but I did not have time to finish right now. This is my suggested format, loosely:

{
    "rule_126": { "type": "overlapping", "image": "samples/rule_126.bmp", "n": 3, "periodic_out": false, "symmetry": 2, "periodic_in": false, "width": 64, "height": 64 },
    "circuit_turnless": { "type": "tiled", "dir": "samples/circuit", "subset": "turnless", "width": 34, "height": 34, "screenshot": 3 }
}

The keys on the left are to have unique names for the output images.

VelocityRa commented 7 years ago

Interesting, I was actually thinking of using YAML for this, since:

Here's a comparison:

JSON

{
  "rule_126": {
    "type": "overlapping",
    "image": "samples/rule_126.bmp",
    "n": 3,
    "periodic_out": false,
    "symmetry": 2,
    "periodic_in": false,
    "width": 64,
    "height": 64
  },
  "circuit_turnless": {
    "type": "tiled",
    "dir": "samples/circuit",
    "subset": "turnless",
    "width": 34,
    "height": 34,
    "screenshot": 3
  }
}

YAML

---
rule_126:
  type: overlapping
  image: samples/rule_126.bmp
  n: 3
  periodic_out: false
  symmetry: 2
  periodic_in: false
  width: 64
  height: 64
circuit_turnless:
  type: tiled
  dir: samples/circuit
  subset: turnless
  width: 34
  height: 34
  screenshot: 3

Edit: I guess JSON doesn't look that bad after properly formatting it, but I still like YAML better for its simplicity. Haven't actually looked into any YAML libraries for either version, though. What do you think?

emilk commented 7 years ago

YAML looks nice, but the C++ libraries are all big monsters. I'd rather keep this repository light, with simple dependencies (one or two files each). I suggest either JSON:

{
    "rule_126": {
        "type":         "overlapping",
        "image":        "samples/rule_126.bmp",
        "n":            3,
        "periodic_out": false,
        "symmetry":     2,
        "periodic_in":  false,
        "width":        64,
        "height":       64
    },
    "circuit_turnless": {
        "type":       "tiled",
        "dir":        "samples/circuit",
        "subset":     "turnless",
        "width":      34,
        "height":     34,
        "screenshot": 3
    }
}

or my CFG format (which is, I believe, a valid subset of YAML):

rule_126: {
    type:         "overlapping"
    image:        "samples/rule_126.bmp"
    n:            3
    periodic_out: false
    symmetry:     2
    periodic_in:  false
    width:        64
    height:       64
}
circuit_turnless: {
    type:       "tiled"
    dir:        "samples/circuit"
    subset:     "turnless"
    width:      34
    height:     34
    screenshot: 3
}

My vote is on JSON, as finding a JSON library in a hypothetical third language will be much simpler than YAML.

emilk commented 7 years ago

Here is a JSON branch: https://github.com/emilk/wfc/tree/json

VelocityRa commented 7 years ago

:+1: Cool!

VelocityRa commented 7 years ago

In retrospect, I'm not sure we want to force the user to specify an output name for every file generated, this could get pretty annoying. Maybe have an optional output parameter? The default output could be formatted depending on N and the other settings, so that you don't even need to change the output name for most use cases.

{
  "overlapping": [
    {
      "name":     "Chess",
      "N":        2,
      "periodic": true,
      "output":   "chess_2_P"
    }
  ]
}

Also I ported samples.xml to the above format and it's 280 lines long (edit: 236 after some packing). It would get even bigger if we needed to add 1 line for the type for each occurence. But my main point is above.

emilk commented 7 years ago

Okay, I've updated the json branch. How is this: https://github.com/emilk/wfc/blob/json/samples.json ?

(please ignore the less-than-perfect alignment)

VelocityRa commented 7 years ago

Options look good!

Just a note: formatting it like that looks good, but it's even less editable than the XML counterpart, I thought that was the point? Make it more readable/editable? I mean someone editing this would spend way too much time on aligning things properly, whereas he wouldn't need to with 1 option per line. Maybe you can leave that there and I can upload my version and link both to showcase both formattings.

VelocityRa commented 7 years ago

mxgmm has objections regarding the JSON and isn't willing to merge it (read the Issue referenced above). I'm not willing to go through the effort of porting to JSON if noone is going to actually make use of it. If you agree with me, I can either wait for you to finish the JSON version and port to XML too, or we could drop JSON and go with XML as originally intended. Such a tedious process for something so simple...

emilk commented 7 years ago

You don't have to align anything - it is still valid JSON, aligned or not. I don't care much either way, but here we go: https://github.com/emilk/wfc/blob/json/samples.json

VelocityRa commented 7 years ago

Yeah but if you don't, it looks like XML, if not worse. So I suppose you're still going to do JSON then?

emilk commented 7 years ago

JSON 👍