Support for groups

[agmcleod]

Hi there,

I started on a game project, wanting to use groups to sorta simulate a scene graph type representation. Given rs-tiled didn't have that, I thought I'd implement and submit a PR. Here's a summary of the changes:

• Breaking change - renamed Layer struct to TileLayer. Reason for this, is I needed a trait type that each layer type implemented, and I thought Layer was the more generic name. If you wish to avoid the breaking change, I can name it to something else. Definitely open to suggestions here
• Added the downcast crate, so the Group struct can store a Vec<Layer> and downcast into the specific types
• Some tests and code just got reformatted, as my editor uses fmt on save.
• Only thing I don't like is the bit of duplicate code in parse_tag! in the Group::new function. Which is fairly re-purposed from Map::new. If you have suggestions on how to refactor this, I am also open to suggestions. Though it's a little tricky, given that in Group::new the layers are pushed as a Box type to a single Vec instead of across many.

[mattyhall]

Hey, sorry this took so long. As I hope you can appreciate life often gets in the way of open source. Particularly when I'm so forgetful! To answer your points:

• I think I'd prefer not renaming the Layer struct. It seems simpler to me to keep the names the same as the tags. Of course naming things is the hardest problem so I don't know what the name of the trait should be!
• I've never come across the downcast crate before. I think if I had been writing it I would have used an enum. Not sure which is better - I might have to have a think about that.

I think the Layer trait (as it's called now) could have at least an id function..? In general empty traits like that feel wrong to me. There's no real reason why I think they're bad (I know they're used elsewhere in rust) but if we can have some functionality there then that's all to the good.

Once again, apologies for being so slow (and, with my last merge, allowing this to bit rot).

[paaaav]

Now that the latest version of Phaser 3 supports groups too I'd love to see support of groups in rs-tiled as well.

[agmcleod]

ah i should really come back to this and address the changes. I recall seeing the comments, but had a real busy work summer :)

[agmcleod]

@mattyhall Very much overdue, but i made some updates and fixed conflicst. I added a function to the trait to return the layer index. I also renamed it to AnyLayer, but I'm still not super happy with this name.

[paaaav]

@mattyhall Hey there :) Is there any way to get it in? What needs to be done?

[aleokdev]

Sorry for the delay. Since this PR was opened, this crate is has moved to the official mapeditor organization. As such, we are looking for new members to contribute to this crate and review new PRs.

As for the PR itself, I do agree with @mattyhall in terms of the dynamic casts. Since layer types are known at compiletime, dynamic references are not needed here, and enums can work better in this case (Leads to better performance and less runtime errors overall). Apart from that, the crate got reorganized in #104 and as such, some changes are needed. If you can fix these issues I'll be happy to review the PR in detail. Thank you for your time! :)

[bjorn]

Closing this because it's not the approach we want to take with the implementation. Alternative implementation is now being worked on at #131.

[agmcleod]

@bjorn sounds good! Sorry i wasn't able to update as well :) @alexdevteam