Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XML2GFF: Add initial xml2gff tool #26

Closed
wants to merge 5 commits into from

Conversation

Nostritius
Copy link
Contributor

This adds an initial xml2gff tool. I modified the gff3 writer, taken from xoreos, slightly, so it won't require glm

@DrMcCoy
Copy link
Member

DrMcCoy commented Sep 30, 2018

You're going C++11 mode now, with that range-based for? :P

Our Travis CI config is not yet adapted for that. But yes, I did say that I'm opening xoreos-tools up for C++11, so I guess it's time I put my money where my mouth is.

I'll see if I find time to change that during the week. :)

@Nostritius
Copy link
Contributor Author

Yeah, i was not sure, if i should correct this, since travis failed.

* Creates GFFs out of XML files.
*/

#include <src/common/error.h>
Copy link
Member

Choose a reason for hiding this comment

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

#include "" for our own files, please

#include <src/common/error.h>
#include "src/xml/xmlparser.h"

#include "gffcreator.h"
Copy link
Member

Choose a reason for hiding this comment

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

Full paths, please

reinterpret_cast<char*>(&typeId)[3 - i] = *type.getPosition(i);
else
reinterpret_cast<char*>(&typeId)[3 - i] = ' ';
}
Copy link
Member

Choose a reason for hiding this comment

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

That's...pretty icky. Can you use MKTAG() instead?

You don't even need the loop, you can just pad the string with 4 spaces beforehand.

Like

const Common::UString type = xmlRoot.getProperty("type") + "    ";
const uint32 typeID = MKTAG(*type.getPosition(0), ...

(Theoretically, you could even use Common::UString::at() from a6f1c2b)

void readStructContents(const XMLNode::Children &strctNodes, Aurora::GFF3WriterStructPtr strctPtr);
void readListContents(const XMLNode::Children &listNodes, Aurora::GFF3WriterListPtr listPtr);

Common::ScopedPtr<Aurora::GFF3Writer> _gff3;
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, can you push the state into the method parameters, and make create() static?

uint32 language;
Common::parseString(child->getProperty("language"), language);

// TODO: Add gender if available.
Copy link
Member

Choose a reason for hiding this comment

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

Can you fulfill this TODO first? I'd rather not have a tool available that accidentally removes information when doing a round-trip

src/version/libversion.la \
$(LDADD) \
$(EMPTY)

Copy link
Member

Choose a reason for hiding this comment

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

Can you add the binary (both Unix and Windows) to the .gitignore file too, please?

(Also, a manpage would be great as well)

src/xml2gff.cpp Outdated
Parser parser(argv[0], "XML to BioWare GFF converter",
"If no input file is given, the input is read from stdin.\n\n"
"Since different games use different SSF file version, specifying the\n"
"game for which to create the SSF file is necessary.",
Copy link
Member

Choose a reason for hiding this comment

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

That's obviously wrong for a GFF tool :P

src/xml2gff.cpp Outdated
}

void createGFF(const Common::UString &inFile, const Common::UString &outFile) {
Common::WriteFile ssf(outFile);
Copy link
Member

Choose a reason for hiding this comment

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

Also not the right variable name here :P

@Nostritius
Copy link
Contributor Author

The issues should now be fixed and I added a man page

@DrMcCoy
Copy link
Member

DrMcCoy commented Oct 14, 2018

Merged as 367563e...41aaf31, thanks! :)

@DrMcCoy DrMcCoy closed this Oct 14, 2018
@Nostritius Nostritius deleted the tools_xml2gff branch November 28, 2018 17:58
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.

None yet

2 participants