Skip to content

Comments

Add Getter accessor for <GDTF DataVersion>#118

Closed
jordan-walters wants to merge 3 commits intomvrdevelopment:masterfrom
jordan-walters:chamsys/jjw/GDTF_FileVersion
Closed

Add Getter accessor for <GDTF DataVersion>#118
jordan-walters wants to merge 3 commits intomvrdevelopment:masterfrom
jordan-walters:chamsys/jjw/GDTF_FileVersion

Conversation

@jordan-walters
Copy link

Needed when importing MVR to ensure all contained GDTFs are using the same (and most up-to-date) GDTF Version

//------------------------------------------------
// Parameters for the GdtfDataFile
TXString gdtfFileVersion = XML_GDTF_CurrentVersion;
TXString gdtfLatestVersion = XML_GDTF_CurrentVersion;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean with gdtfLatestVersion? GDTF file have only one version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, a GDTF file could be an old one that was made using GDTF version 1.1, so GetGDTFFile would return 1.1
I would also like a function to return the latest GDTF Version that is available in GDTF-Share, i.e. 1.2.

//----------------------------------------------------------------------------------------------------------------------------------------------------------------
// Getter
const TXString& GetGdtfFileVersion() const { return gdtfFileVersion; }
const TXString& GetCurrentGdtfVersion() const { return gdtfLatestVersion; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same question here: Why you need 2 APIs GetGdtfFileVersion and GetCurrentGdtfVersion? GDTF file can have only one version.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment.

private:
//------------------------------------------------
// Parameters for the GdtfDataFile
TXString gdtfFileVersion = XML_GDTF_CurrentVersion;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it looks like all member variable had a prefix "f" in the name. So please adapt you changes.

@adragnevVW
Copy link
Collaborator

Hi @AndriiVoitenko,
I have made a fix for this few months ago. You can check it and if you think is okay we can submit it instead of that fix.#114

@jordan-walters
Copy link
Author

Hi @AndriiVoitenko, I have made a fix for this few months ago. You can check it and if you think is okay we can submit it instead of that fix.#114

Yes, that is good to use previous fix you made.
However could I still have TWO functions, one to return the current latest version of GDTF, and one to return the GDTF version of the current file being parsed.

If it makes it easier maybe the current latest GDTF Version could be const or #defined.

@adragnevVW
Copy link
Collaborator

Hi @jordan-walters ,
I think we can open a new issue with your suggestion and we can start a discussion about it. Also we can submit #114 as a current fix and then discuss further changes. @AndriiVoitenko Do you think is good idea?

@jordan-walters
Copy link
Author

Hi @jordan-walters , I think we can open a new issue with your suggestion and we can start a discussion about it. Also we can submit #114 as a current fix and then discuss further changes. @AndriiVoitenko Do you think is good idea?

Hello Andrii. Yes that makes sense.
Your merge will return the version used to create the GDTF file (could be 1.0, 1.1, 1.2).

A new issue to provide function to return the latest/highest version of GDTF file (currently 1.2)

@adragnevVW
Copy link
Collaborator

Thank you @jordan-walters! I will close this pull request. Please create a new issue, which we can start discuss. Thank you so much!

@adragnevVW adragnevVW closed this Nov 3, 2025
@jordan-walters
Copy link
Author

New issue created for latest/current GDTF Version. #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants