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

libtiled-java - Tiled editor parity #2207

Merged
merged 9 commits into from
Oct 30, 2020

Conversation

viitahenri
Copy link
Contributor

I've been making modifications to the libtiled-java in order to use it for automated processing/modifying of Tiled maps.

It's very much based on my current needs, so the features added to reading&writing the map files are features I've noticed end up with differences (or missing data altogether) in the map files.

I'm opening this PR mostly in case someone else has a need to touch the map files programmatically while maintaining the same file structure (the goal is to have no changes if you just do read/write with the Java lib).

Hoping to get this clean enough to be mergeable in the future.

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.

Those are some great updates! I've left a few comments, though I haven't touched Java since so long I may be wrong about some of them. In general it seems pretty much ready for merging. Thank you so much for working on this!

I noticed you've also made some updates to the DTD. Is that file still relevant now that we have an XSD that describes the format in much more detail?

@@ -345,6 +356,9 @@ private MapObject readMapObject(Node t) throws Exception {

MapObject obj = new MapObject(x, y, width, height, rotation);
obj.setShape(obj.getBounds());
if (id != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

I reckon the id defaults to 0 anyway, in which case this condition is not adding any value and should just be removed.

@@ -501,6 +575,11 @@ private ObjectGroup unmarshalObjectGroup(Node t) throws Exception {
final int offsetY = getAttribute(t, "y", 0);
og.setOffset(offsetX, offsetY);

final int locked = getAttribute(t, "locked", 0);
if (locked != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Please also remove this condition and just call og.setLocked(locked).

}

final int locked = getAttribute(t, "locked", 0);
if (locked != 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Just call g.setLocked(locked) if the result is the same.

@bjorn
Copy link
Member

bjorn commented Dec 18, 2019

@viitahenri Will you still get around to finishing this pull request? Maybe it would be an option to make the minimal changes needed so that this can be merged, even though there may not yet be 100% parity? (I'm not sure what's missing in that regard)

@csueiras csueiras mentioned this pull request Oct 23, 2020
@bjorn bjorn merged commit c8f11fd into mapeditor:master Oct 30, 2020
@bjorn
Copy link
Member

bjorn commented Oct 30, 2020

@viitahenri Alright, I've just merged this as-is since @csueiras could use these changes. I did some small corrections on top in commit d3c3987, but kept the != 0 checks I commented on since I wasn't sure whether they could be removed.

If either of you wants to help further update or fix this library, such help would be really appreciated!

Thank you for these updates!

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.

2 participants