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

Automatically generate Java classes from XSD #1637

Merged
merged 8 commits into from
Jul 31, 2017

Conversation

mikepthomas
Copy link
Contributor

Created XML Schema file using the tiled map format documentation to automatically generate Java classes and javadoc documentation and bring in attributes that have been introduced into newer versions of Tiled

Copy link
Member

@bjorn bjorn left a comment

Choose a reason for hiding this comment

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

What an amazing piece of work!

As far as I understand, much of the code is actually no longer there, because it will get automatically generated as part of the build?

I just noticed one problem with the layers. I hope it is still possible to have all layers in a single array, since the order is important.

renderer.paintTileLayer(g2d, layer);
}
// Draw each object group
for (ObjectGroup group : map.getObjectGroups()) {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, this is not how layers work, since layers of different types are supposed to get interleaved. Would it still be possible to set it up that way?

@bjorn
Copy link
Member

bjorn commented Jul 4, 2017

Ah, maybe we should not commit util/java/libtiled-java/src/main/resources/map.htm? It would be nice to host this as part of the documentation website, though.

@mikepthomas
Copy link
Contributor Author

As far as I understand, much of the code is actually no longer there, because it will get automatically generated as part of the build?

Yes that is correct, I have updated the appveyor build script to build the java projects using maven so that the code can be generated as the qbs task cannot do this, however the build is still failing even though the maven task is saying it is successful... not sure why at the moment

I just noticed one problem with the layers. I hope it is still possible to have all layers in a single array, since the order is important.

This makes perfect sense now you mention it, I have now fixed it so that TileLayer, ObjectGroup and ImageLayer all live in the same list as requested

Ah, maybe we should not commit util/java/libtiled-java/src/main/resources/map.htm? It would be nice to host this as part of the documentation website, though.

I have now removed the generated documentation, im not quite sure where you would want it put to be part of the documentation website though... let me know if you need anything more

@bjorn
Copy link
Member

bjorn commented Jul 10, 2017

Well, that fixes the AppVeyor build. The fact that now the Travis CI build is broken is not relevant to this PR. Do you think this change should be ready for merging now, @mikepthomas?

@mikepthomas
Copy link
Contributor Author

I believe so, If you are happy with the content it can be merged. Once merged, I can start the process to deploy to maven central. Let me know if you need anything more :)

@bjorn
Copy link
Member

bjorn commented Jul 11, 2017

Alright, I've just tried to get this to work locally. While compiling libtiled-java, I noticed many warnings about deprecated x, y, width and height in LayerData. Is that a currently expected state or did you forget something?

When I tried to get tmxviewer-java to work, mvn clean install gave me:

[ERROR] Failed to execute goal on project tmxviewer: Could not resolve dependencies for project org.mapeditor:tmxviewer:jar:1.0-SNAPSHOT: Failed to collect dependencies at org.mapeditor:libtiled:jar:1.0-SNAPSHOT: Failed to read artifact descriptor for org.mapeditor:libtiled:jar:1.0-SNAPSHOT: Could not find artifact org.mapeditor:tiled-parent:pom:1.0-SNAPSHOT -> [Help 1]

But I did do the mvn clean install in the libtiled-java directory before as mentioned in the README. What can I do?

@mikepthomas
Copy link
Contributor Author

Alright, I've just tried to get this to work locally. While compiling libtiled-java, I noticed many warnings about deprecated x, y, width and height in LayerData. Is that a currently expected state or did you forget something?

This is expected because these values have been marked as deprecated in the tmx map format docs:
"Attributes or elements that are deprecated or unsupported by the current version of Tiled are formatted in italics."

The warnings are being displayed as the showDeprecation flag is set in the maven compiller plugin, this can be turned off if you want to surpress the warnings during compile time.

But I did do the mvn clean install in the libtiled-java directory before as mentioned in the README. What can I do?

The command "mvn clean install" should be done in the util/java directory so that it builds both projects at the same time. The error you are receiving is due to the parent pom not being in your local maven repository and I have yet to deploy a 1.0-SNAPSHOT build to the oss snapshots repository. When a snapshot build has been deployed, then building from each project directory should work fine.

@bjorn
Copy link
Member

bjorn commented Jul 11, 2017

This is expected because these values have been marked as deprecated in the tmx map format docs:
"Attributes or elements that are deprecated or unsupported by the current version of Tiled are formatted in italics."

Hmm, well technically I wrote "deprecated or unsupported", and for these attributes it actually means more like "unsupported". Tiled can read and write maps using those attributes and will generally work fine, but it can't move or resize tile layers individually. I realized now that width and height are even required, since leaving them out would mean an empty (0x0) tile layer (the properties don't default to the map size, like I may have planned at some point). So I need to fix up those docs...

The command "mvn clean install" should be done in the util/java directory so that it builds both projects at the same time. The error you are receiving is due to the parent pom not being in your local maven repository and I have yet to deploy a 1.0-SNAPSHOT build to the oss snapshots repository. When a snapshot build has been deployed, then building from each project directory should work fine.

Ok, that makes sense. The README file in the tmxviewer-java should probably mention the part about running the command in the parent directory.

@mikepthomas
Copy link
Contributor Author

Hmm, well technically I wrote "deprecated or unsupported", and for these attributes it actually means more like "unsupported". Tiled can read and write maps using those attributes and will generally work fine, but it can't move or resize tile layers individually. I realized now that width and height are even required, since leaving them out would mean an empty (0x0) tile layer (the properties don't default to the map size, like I may have planned at some point). So I need to fix up those docs...

Even though they are deprecated in libtiled-java they should be able to be read and written to without issues too, and the warnings are just being generated due to the property being marked with the @deprecated annotation, I can either remove the annotation or ignore the warning, whichever you feel best.

Ok, that makes sense. The README file in the tmxviewer-java should probably mention the part about running the command in the parent directory.

I can add this to the documentation, however it will only be required when the pom does not exist in the snapshots repository. I will need to add it as part to the deployment to maven central and once it is there, building each project individually should work just fine. So I am not sure the documentation will be relevant after deployment. I can add a snapshot build to oss tonight from my forked repository if you like however the last one was created with a clean checkout from your master branch.

@bjorn bjorn merged commit 14a6f0a into mapeditor:master Jul 31, 2017
@bjorn
Copy link
Member

bjorn commented Jul 31, 2017

Sorry for the delay. It looks like a great improvement and I've just given it another test run and it was working fine, so it's merged now!

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