search

mapeditor / rs-tiled

Reads files from the Tiled editor into Rust
https://crates.io/crates/tiled
MIT License
268 stars 103 forks source link

Common layer interface #197

Closed aleokdev closed 2 years ago

aleokdev commented 2 years ago

Closes #179.

aleokdev commented 2 years ago

The changes look good to me, though I don't quite understand why it closes #179, since the common interface appears to be limited to getting the width/height, which is then non-existent for infinite layers.

Well, that's really all they have to offer. There's already a get_tile method iirc so what was missing were the w/h properties.

In that sense, actually I think a bounds property returning a rectangle might be more useful. It would be rect(0, 0, width, height) for finite layers, but could return the bounding rectangle of the content for infinite layers, providing a straight-forward way of determining which area to process for tiles while rendering.

Sure, but this would be an even better job for the future tiles iterator. I'll add a bounds function though, it does seem useful.

179 also mentions the unified storage for finite and infinite tile layers, but I guess that's an idea better split off into its own issue.

Yeah I don't think that's happening at any point, since it'd require another refactor which would completely change the interface again. It's ok as is, I guess.

aleokdev commented 2 years ago

I'm merging now and introducing the bounds function in another PR, since right now neither of the finite/infinite tile layers have the function so there's nothing to unify (it's just a new convenience function). I think it deserves its own PR