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

Add support for <group> layer tags #53

Merged
merged 6 commits into from
May 13, 2019
Merged

Conversation

Grant1219
Copy link
Contributor

This is a start for supporting the <group> tag added in version 1.0 of the TMX format. Let me know if my approach here doesn't make sense, or if I'm missing something major.

I ran the parse test and it seems to still be working, but I would like to update it to include nested layers within layer groups to make sure everything is functioning.

A group layer is used to organize the layers of the map in a hierarchy.
This tag was added in version 1.0 of the TMX map format.

More information can be found here:
https://doc.mapeditor.org/en/stable/reference/tmx-map-format/#group
@fallahn
Copy link
Owner

fallahn commented May 12, 2019

It looks sane to me - from what I understand a layer group is not dissimilar to an object group, so a similar approach to parsing feels correct. The parse test doesn't specifically test for layer groups, so it will need updating. If you're prepared to do it that would be fantastic :D I've noticed the CI for msvc has failed, but I'll fix that when I update the Visual Studio solution. Thanks for your efforts!

@Grant1219
Copy link
Contributor Author

@fallahn Oh is the MSVC build failing for a reason unrelated to my code? I made a couple fixups, but it didn't seem to resolve the build errors. Unfortunately I don't have access to MSVC to dig into it.

Anyway, I'll probably get around to updating the tests tomorrow with layer groups. Thanks for being so responsive. :)

@fallahn
Copy link
Owner

fallahn commented May 12, 2019

No problem! Visual Studio is my main environment so I can hop into it as soon as I have a few moments.

Grant1219 and others added 2 commits May 12, 2019 02:47
I simply enclosed the tile layers within a group layer and made sure the
test code iterated over all the sublayers within the group.
add explicit move constructor/assignment declarations to layer group (required by msvc)
@Grant1219
Copy link
Contributor Author

Thanks for fixing the Windows build! I added a group layer to the test map and made sure to check that fetching properties and sublayers works. I realize not every edge case is tested, but increasing the test coverage for all the map elements might be a task for another PR.

Let me know if you want any further changes!

@fallahn
Copy link
Owner

fallahn commented May 13, 2019

Brilliant! All looks good, thanks for the contribution!

@fallahn fallahn merged commit 74f1f5c into fallahn:master May 13, 2019
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