Interface::getVersionIntegers doesn't return the full version

[ix-dcourtois]

Hi,

I noticed that in the latest MaterialX version, a new version of the standard surface was introduced. It's labeled as version "1.0.1" and the previous one is "1.0.0". Unfortunately the Interface::getVersionIntegers helper only returns the first 2 numbers, which makes it a bit awkward to use :) Not a very big issue, but it would be nice if that method would return the 3 numbers, like the getVersionIntegers found in the MaterialXCore/Utils.h header.

Regards, Damien.

[kwokcb]

Hi Damien,

This is most likely due to assumed "usage". This utility is used to get the MaterialX document version of which the patch version in a major.minor.patch string only returns the major and minor versions. The version on a definition (nodedef) can be anything so this utility "fails" to parse the last digit. (Both nodedef and document are derived from interface).

@jstone-lucasfilm , what do you think about this? Should a document have a specific derived class version which limits parsing to major.minor and the default one on interface does as many as it can find. The caveat is of course that version is a string and I believe it's only a suggestion as to the format to use.

Another way to handle this is that I've written utilities to accept a "template" and basicaly based on the template the utility attempts to extract out the digits. e.g. if a version can use this template: `my_version.#.#" would parse out 2 digits.

Of course don't want to over-complicate things but want to keep the versioning and upgrade logic "safe".

Thoughts?

[ix-dcourtois]

Hi,

The helper doesn't need to cover all usages, far from me to suggest that! (I'm generally more a "the simpler the better" person) No need to make an overly complicated version parser to handle all cases, it's just a matter of making the utility at least match the usage in the official libraries: since the version string is provided anyway, anyone can still write his own "version parser". Which I did. But it just felt wrong to have to do that for the "official" nodes :p So basically just adding the patch number would be enough I think.

[jstone-lucasfilm]

@ix-dcourtois @bernardkwok Good points, and perhaps our getVersionIntegers method should return three integers for all Interface elements, with the final integer set to zero when not present in the string. This would be consistent with the global getVersionIntegers method, which returns the major, minor, and build versions of the MaterialX codebase itself.

[kwokcb]

This sounds like a good idea to not-prefilter the patch version number in case it's desired.

I want to double check this is true:

1. A Document write will not store patch version so the utility would return the previous release e.g. 1.38.0 always as 1.39 is not released. Once released all files will be 1.39 when extracted returns 1.39.0. A user can still override this and write out with a patch version but it is still ignored for upgrade logic.

2. The current distribution / library version is part of the build so includes patch version so 1.39.3 is what's hard-coded. This is what's returned from the global query and is used to generate the namespacing for library interfaces as well as for Document upgrade.

3. Any nodedef can store version in any format but to have this utility parse the numbers out of the string, it's assumed it's: <major>.<minor>.<patch>. Thus the 2 versions of std surface will parse properly.

• For 3. For generalized nodedef version string to numeric support, If there is interest I can put up a "template" parsing proposal issue, so you have something like getVersionIntegers(const string& template = "<major>.<minor>.<patch>);