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 a tiles iterator to FiniteTileLayer/InfiniteTileLayer #167

Open
aleokdev opened this issue Feb 20, 2022 · 10 comments
Open

Add a tiles iterator to FiniteTileLayer/InfiniteTileLayer #167

aleokdev opened this issue Feb 20, 2022 · 10 comments
Labels

Comments

@aleokdev
Copy link
Contributor

Add some kind of iterator that has Item = (LayerTile, (i32, i32)) or similar. Very useful for rendering code.

@bjorn
Copy link
Member

bjorn commented Feb 21, 2022

But in which order should it iterate the tiles? In Tiled I have:

    using RenderTileCallback = std::function<void (QPoint, const QPointF &)>;

    /**
     * Calls the given \a renderTile callback for each tile in the given
     * \a exposed rectangle.
     *
     * The callback takes two arguments:
     *
     * \list
     * \li \c tilePos - The tile position of the cell being rendered.
     * \li \c screenPos - The screen position of the cell being rendered.
     * \endlist
     */
    virtual void drawTileLayer(const RenderTileCallback &renderTile,
                               const QRectF &exposed) const = 0;

But the order in which it calls renderTile depends on the map orientation. Of course this function is doing a bit more than iterating tiles, it also says where each tile goes on the screen.

Note that it doesn't get a tile reference on purpose. There was no tile layer reference going in either. This is to make the function more generally useful, since you could be drawing some other overlay or stacks of tiles based on this.

@aleokdev
Copy link
Contributor Author

IMO the order shouldn't be specified, and we should leave it as implementation-dependent. This is because we can't really specify the order of iteration of infinite tile layers without adding overhead, for instance.

Note that it doesn't get a tile reference on purpose. There was no tile layer reference going in either. This is to make the function more generally useful, since you could be drawing some other overlay or stacks of tiles based on this.

I don't really see how it makes it more useful. If you have a tiles iterator the normal thing would be to iterate over the tiles in the layer. Otherwise you could just use two nested fors.

@bjorn
Copy link
Member

bjorn commented Feb 21, 2022

IMO the order shouldn't be specified, and we should leave it as implementation-dependent. This is because we can't really specify the order of iteration of infinite tile layers without adding overhead, for instance.

It is about the order of the visible tiles. The function takes the exposed rectangle (the area where tiles should get drawn), so the size of the layer is irrelevant and could be infinite. The tile rendering order is important in case they overlap.

I don't really see how it makes it more useful. If you have a tiles iterator the normal thing would be to iterate over the tiles in the layer. Otherwise you could just use two nested fors.

So, it makes it more useful in the cases I mentioned. If you're rendering a special overlay or rendering tiles as stacks, a reference to a single tile is not always useful. And it can anyway be trivially obtained using the tile coordinates.

Anyway, I think I may be over-complicating things. Personally I'd hope we can provide some support as well for rendering maps with different orientations, rendering only a part of a tile layer and rendering some overlay, but your proposed iterator is useful in its own right.

Edit: And yeah, I just realized we even have this kind of iterator in Tiled itself as well: mapeditor/tiled#1640

@aleokdev
Copy link
Contributor Author

Anyway, I think I may be over-complicating things. Personally I'd hope we can provide some support as well for rendering maps with different orientations, rendering only a part of a tile layer and rendering some overlay, but your proposed iterator is useful in its own right.

Whoa. Okay, that is a bit too much for this library. This is supposed to be a library for parsing and reading tiled files; The actual rendering is done by the user. IMO they are the ones that should care about draw order and whatnot.

@bjorn
Copy link
Member

bjorn commented Feb 21, 2022

Right, maybe it's going too far, but I wasn't talking about the actual rendering, but rather about an abstraction on top of the various map orientations supported by Tiled, which have non-trivial logic about how tiles get placed. It could anyway be more suited for a different crate.

@ShikuWorld
Copy link

Hey I wrote my own parser for tiled maps because none of the existing solutions were really to my liking and then saw this is now the official one and wanted to switch. I have already implemented infinite maps for my game and was saddened to see that its not really? supported yet. I don't get how to iterate over an infinite map from the examples at least. Is there anything speaking against just exposing the internally stored chunks to iterate over? I actually kinda just rolled with these and my game now loads terrain in the chunks tiled has defined. That way I'd also avoid iterating over huge empty spaces. Was even thinking about making a fork of this repository but maybe I can help out a little?

@aleokdev
Copy link
Contributor Author

The thing about the chunks is that they're meant to be an implementation detail and as such you cannot change e.g. their size. I guess we could make them public, don't see anything wrong with it, as we do need some kind of iterator over infinite maps anyways.

@aleokdev
Copy link
Contributor Author

@bjorn Opinions on this?

@adambiltcliffe
Copy link

Chiming in here because I just discovered this crate and was also disappointed at the limited support for infinite maps. At minimum I was hoping for some way to retrieve a lower and upper bound for the x and y coordinates for the tiles in an infinite layer but exposing chunks would allow for more uses too. I understand that chunks are an implementation detail but they do exist in the TMX file itself so not exposing that information through the crate just encourages people to roll their own TMX parser rather than using this one which doesn't seem like it helps anyone.

@aleokdev
Copy link
Contributor Author

Working on a PR now. The tiles iterators can wait until we find out the order they should return tiles in, but an arbitrary-sorted chunks iterator can be easily implemented with the design we currently have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants