Make tilemap respond to Anchor component.

[shanecelis]

Hi, thank you for all your work on this crate! It's been incredibly useful.

I had a specific need for controlling how the tilemap was positioned. I specifically wanted it to be positioned from the top-left, but I went ahead and implemented all the positions available on bevy::sprite::Anchor. So now one can choose the anchor much the same way one chooses with a sprite.

I added a example called "anchor" that demonstrates this feature. The tilemap is placed at the origin and only its anchor changes. Here is a movie of it.

This is not a breaking change. Tilemaps that have no anchor component will be positioned the same as before.

I did not update the other examples that rely on get_tilemap_center_transform() but they can all be altered to use Anchor::Center. One nice property of this anchor is that the Transform does not contain any "center" information, so if you place the tilemap centered at (0,0,0), the transform will be at (0,0,0) unlike the helper function.

If this PR is accepted, and you'd like the other examples updated, let me know.

Kind regards,

Shane

[ChristopherBiscardi]

ci isn't passing at the moment and this will need some docs to explain what its supposed to be doing. That said I think I like this change.

Question about how you anticipate Anchor being used here. It seems like Anchor::None places the Transform on the center of the first bottom-left tile, and Anchor::BottomLeft places it at the edge of the first bottom-left tile (the other anchors all seem to be at the edge). Do you envision the examples and suggested user usage to change to use Anchor::Center?

I have to check some more things, like how it behaves with multiple chunks, what other math it affects, and whether we should be using Anchor for this (or a custom TilemapAnchor). (bevy's Text2d and Sprite use Anchor, but I don't think Mesh2d does in Bevy).

[shanecelis]

Hi Chris, thanks for looking at these changes!

It seems like Anchor::None places the Transform on the center of the first bottom-left tile, and Anchor::BottomLeft places it at the edge of the first bottom-left tile

Yes, that was done to preserve the original behavior where no Anchor is present. Placing the anchor at the edges seemed appropriate partly to match Sprite's behavior, also it fit my usecase.

A TilemapAnchor with Anchor's variants and None or TileCenter(u16,u16) might be desirable then the it could default to None or TileCenter(0,0), which would preserve the original behavior. That would clean up a little awkwardness when dealing with Option<&Anchor> in the code too since it could be part of the bundle.

However, one could also achieve a similar effect with a helper function that that produced an Anchor::Custom(v): get_tilemap_anchor(..) -> Anchor.

Do you envision the examples and suggested user usage to change to use Anchor::Center?

Yes, but only for the examples that are currently centering their tilemaps, which are the majority. I'd also suggest deprecating get_tilemap_center_transform() and placing a note to refer the user to Anchor::Center.

Oh! One big consideration I haven't looked at is how this works with non-rectangular tilemaps.

[shanecelis]

Well, I took my own suggestions from above and am now using a TilemapAnchor. It was definitely more involved supporting all the layouts but I think the code actually came out better for it. The heart of it in src/anchor.rs is much more legible now.

I decided to make use of the AABB code to generalize the behavior for all map types. The AABB seemed slightly bigger than it needed to be. I halved the size of the border. It's possible that's incorrect in its other uses, but I believe it's just more tightly bound now. Before it had a half-tile-size of buffer around it for my purposes. If that's incorrect in its other uses, please tell me.

I also went through all the examples and I ran them to compare with previous behavior and new, and I believe they are all working even more complicated ones like "mouse_to_tile". I did add an "Enter" key press to "hex_neighbors" to exercise the different anchors like the "anchor" example. Here's a movie of it.

I think this is in good shape now.

[ChristopherBiscardi]

I'd also suggest deprecating get_tilemap_center_transform() and placing a note to refer the user to Anchor::Center

yeah that seems like the right path forward. Would be nice to drop it.

[shanecelis]

I think the typical user code that used to center the tilemap is an improvement. It's more flexible, doesn't require any knowledge about the map size, type, or tile size.

- transform: get_tilemap_center_transform(&map_size, &grid_size, &map_type, 0.0),
+ anchor: TilemapAnchor::Center,

However, the code that needs to ascertain the position of tiles is not an unmitigated improvement. Currently it looks like this:

let tile_center = tile_pos.center_in_world(grid_size, map_type).extend(1.0);
- let transform = *map_transform * Transform::from_translation(tile_center);
+ let anchor_offset = anchor.from_map(map_size, grid_size, tile_size, map_type);
+ let transform = *map_transform * anchor_offset * Transform::from_translation(tile_center);

The anchor_offset is my addition. Perhaps it could be improved by adding TilemapAnchor as an argument to TilePos::center_in_world().

EDIT: The second block of code was supposed to be a diff. Sorry for that. It must've been confusing. Removed 'populate_transform' comment for submission in a different PR.

[shanecelis]

To address the problem I created and cited above with ascertaining the position of tiles, I no longer require the user to manipulate anchor and its associated transform directly, I added the arguments map_size and anchor to TilePos::center_in_world(), trying to follow the ordering of arguments in similar calls.

-let tile_center = tile_pos.center_in_world(grid_size, map_type);
+let tile_center = tile_pos.center_in_world(map_size, grid_size, map_type, anchor);
let transform = *map_transform * Transform::from_translation(tile_center.extend(1.0));

Hopefully this improves the ergonomics and makes center_in_world() live up to its name better.

Unused Optimization

Anchor now produces a Vec2 instead of a Transform behind pub(crate) fn as_offset(...). In "prepare.rs" I multiply by a Transform::from_translation(anchor_offset) but I considered merely adding the offset to the translation field directly, which would be faster and effective for many usecases; however, it would prohibit the user from rotating or scaling their map's transform. I left a note in the code mentioning this tradeoff.

[shanecelis]

Let me summarize this PR's changes in the hopes it will ease reviewing:

• It does make breaking changes, namely the center_in_world() function has two more parameters map_size and anchor, so I'd suggest bumping bevy_ecs_tilemap to version 0.16.0 if accepted.
• It updates all the examples that used get_tilemap_center_transform() to use TilemapAnchor::Center.
• It deprecates get_tilemap_center_transform() with a message that recommends using TilemapAnchor::Center.
• It adds a new example called "anchor" that changes a map's anchor when Space is pressed.
• It modified "hex_neighbors" example to change its anchor when Enter is pressed.

I have gone through all the examples and I believe it's working in all cases. If anyone sees any discrepancies or bugs, please post them here.

[adrien-bon]

Added some comments but it's mostly nitpcking.
Very nice feature overhall ! :)

[shanecelis]

Many thanks for your feedback @adrien-bon. I will endeavor to incorporate your feedback soon.

[shanecelis]

After playing around with an example that altered the grid size, it seemed best to propagate tile size through the API. It aligns with intuitions and looks correct on inspection. I've done that and I modified the hex_neighbors example to react to the TAB key to change the grid scale between 1, 0.5, and 1.5. Here's a video of it in action:

I also modified the example to show the three parameters in play and their key bindings more clearly. Although I worry I've overloaded the hex_neighbors example well past its original reason for being. Perhaps I should try to merge the "anchor" and "hex_neighbors" into just one "anchor" example and restore the original "hex_neighbors".

I believe these changes address all of @adrien-bon's feedback. Thank you again @adrien-bon for the review.

[shanecelis]

$ commit -m "final-final-complete-patch-last-for-sure.doc"

Replace "anchor.rs" example

I replaced the "anchor.rs" example with the modified "hex_neighbors.rs" example. I added the square and isometric map types so it covers all types. I removed the neighbors specific code to simplify the example to focus purely on the anchor aspect. Here is the new "anchor" example in action:

Restore "hex_neighbors.rs" example

I restored hex_neighbors to how it was prior to any work I did on this branch and then made to minimal changes to bring it up to date with the branch like using TilemapAnchor::Center.

Last two commits only touch example code.