we should have semantics in instanceTechnique along with values

[fabrobinet]

No description provided.

[tparisi]

Presumably with the same syntax, i.e. an array? :-p

[fabrobinet]

I was waiting for you to get back there.. ;)

Now that we don't need to specify either semantic or value as this is done upfront with 2 distincts properties semantics and values we can indeed reconsider having an object instead of an array.

So I extend a bit the topic of this bug for a moment....

We have to make a trade-off: have to choose between the solutions that "looks good" i.e objects versus the most pratical one i.e the array.
It means, is it a big deal to have to do an Object.keys() ? I think, I could live with it... but let's take the decision all together.

So @RemiArnaud @pjcozzi

Would you guys be OK to switch from:
1.

        "instanceTechnique": {
            "technique": "technique1",
            "values": [
                {
                    "parameter": "shininess",
                    "value": 38.4
                }
            ]
        },
        "name": "blinn3"
    }

to
2.

    "instanceTechnique": {
            "technique": "technique1",
            "values": {
                    "shininesss" : 38.4
           },
        "name": "blinn3"
    }

1. looks obviously better but requires to get the keys. If that's make a big difference in file size that could be worth it.

What do you think ? @tparisi I guess what's your side here ;)

[tparisi]

@fabrobinet I think you meant that 2. looks obviously better. Of course I prefer #2. We're doing gets on the keys all over the place anyway, so IMO doing one more foreach won't kill an implementation. An array is a bit more efficient but that efficiency is probably going to be eclipsed in any practical implementation by code that error-checks the input and other processing, anyway. Long story short I prefer #2.

[fabrobinet]

yeah, I meant 2.

[fabrobinet]

I know @tparisi is OK, @RemiArnaud @pjcozzi OK too ? If yes I will makes changes in the converter.

[fabrobinet]

Ok, will implement this one soon.

[pjcozzi]

Sorry for the delay here, but I am strongly for 2 as well.

In fact, I had this exact feedback in my notes from when I redid my implementation a little while back.

[fabrobinet]

Ok thanks for confirming Patrick. I'll make it sometime this week.

[fabrobinet]

6c39102 ready in dev-1 branch

[fabrobinet]

pushed on master

[pjcozzi]

@fabrobinet schema is updated, let's close.