JS Bindings: Wrap all functions that return value pointers

[frericp]

This PR wraps all remaining functions that return a ValuePtr (ValueElement::getValue was already wrapped before). Calling these functions will now return the value directly, removing the need to call getData first. The original behavior is exposed through functions prefixed with _, e.g. _getValue.

While this aligns the JS bindings with the Python ones, I'm not particularly happy with the approach. The need to call getData doesn't justify the introduction of potentially error-prone wrapping code, for the following reasons:

• Changes of the wrapper code (post.js) are invisible to the build system. Updating them requires touching any .h or .cpp file.
• Future contributors might not know about this convention, and forget about adding wrappers for new bindings, leading to inconsistent APIs.
• The API is already inconsistent. In Python, functions like getValue are wrapped, but others, mostly from the shader generation API, are not. In general, functions returning a ValuePtr are wrapped, but functions like TypedValue::copy are not, because it doesn't make sense from an API point of view (calling copy on a Value should return a Value, not just the contained value). Wrapping static Value factory functions like Value::createValue does also feel counter-intuitive.
• There's no clear path on how to deal with functions that take ValuePtrs as a parameter (we currently don't have bindings for them).

--> Voting to not wrap the functions in the first place @sdunkel @muenstc @kohakukun

Update #1184

[frericp]

Still working on some more involved cases, so this isn't final yet. Ready for review

[kohakukun]

I really don't have a strong opinion about this. On the one hand, for the first two points, I dont think there is an issue. I think this a fairly stable API. On the technical part, I agree with you but, as a javascript user, I would find it very off-putting having to call the getData() from a function like getFloatValue :/

[frericp]

@bernardkwok @ashwinbhat We have discussed this topic internally, and came to the conclusion that wrapping the ValuePtr returning functions might not be beneficial enough to justify the drawbacks. However, we would like to take an approach that is consistent between Python and JS (and ideally C++, in general). Since the Python bindings are currently (partially) wrapping such functions, what is your opinion on how we should handle them?

If we decide to diverge from the C++ API here, how could we document this in a more obvious way than putting it in a readme? We have a paragraph on it here at the moment: https://github.com/autodesk-forks/MaterialX/blob/adsk_contrib/dev/source/JsMaterialX/README.md#value-getters

[bernardkwok]

@bernardkwok @ashwinbhat We have discussed this topic internally, and came to the conclusion that wrapping the ValuePtr returning functions might not be beneficial enough to justify the drawbacks. However, we would like to take an approach that is consistent between Python and JS (and ideally C++, in general). Since the Python bindings are currently (partially) wrapping such functions, what is your opinion on how we should handle them?

If we decide to diverge from the C++ API here, how could we document this in a more obvious way than putting it in a readme? We have a paragraph on it here at the moment: https://github.com/autodesk-forks/MaterialX/blob/adsk_contrib/dev/source/JsMaterialX/README.md#value-getters

It would still be good to get an opinion from the LucasFilm side if this hasn't been asked yet. As far as I see it seems that the Python wrappers wrap all types as "objects" which have methods on then, even for simple types like float, int etc.

It could be the deciding factor is what operations are possible at the "object level".

For something like an array, this is a reference to a array object, but could also be an array of references to the atomic type, or just unreferenced atomic values. Any form could work for querying and conversion to string which is common in C++, but not sure if this is really required for Python and Javascript.

This leaves editing operations and conversion from strings which would not be possible if it's not referenced.

I think one other factor to take into consideration is locale support which @ashwinbhat did some work on for Value. It could mean that at least for compound types we want an object to be able to translate to/from string representations.

My 2 cents at this point.

[frericp]

I'm closing this PR because the value wrappers turned out to be a legacy feature of the Python bindings that we'll probably want to deprecate.