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 per-tile flipping and transposing #80144

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Aug 2, 2023

Closes godotengine/godot-proposals#3967

godot.windows.editor.dev.x86_64_ibsEFV55G2.mp4
godot.windows.editor.dev.x86_64_mOQwX7qU05.mp4

@KoBeWi KoBeWi added this to the 4.2 milestone Aug 2, 2023
@KoBeWi KoBeWi force-pushed the freedom_tiles branch 2 times, most recently from 52a3384 to bd365b6 Compare August 3, 2023 18:36
@KoBeWi KoBeWi marked this pull request as ready for review August 3, 2023 18:36
@KoBeWi KoBeWi requested a review from a team as a code owner August 3, 2023 18:36
@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 3, 2023

Finished:

godot.windows.editor.dev.x86_64_ibsEFV55G2.mp4

Although I still need to do some cleanup.

Also, since transform is stored as part of alternate id now, the maximum number of alternate tiles per tile is now more limited (from 32768 to 4096). We should consider enforcing this limitation or documenting it.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 9, 2023

Pattern rotating:

godot.windows.editor.dev.x86_64_mOQwX7qU05.mp4

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 17, 2023

Rebased and squashed (there was no reason to keep the history after the recent rework).
I also fixed the shortcuts now that #80317 was merged (it needed a small tweak though).

@groud
Copy link
Member

groud commented Aug 22, 2023

Sooo, looks mostly good. I wonder about the following situations though:

  • what to do with half-offset layouts (isometric, half-offset square and hexagonal).
  • what to do when the tiles are not square.

Maybe we might want to implement a way to disable the rotation buttons when height != width, and only allow flips ? That would avoid some confusion I believe. Though, for special cases, I guess we could have a way to "force rotations" anyway. Like maybe a checkbox or something.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 22, 2023

Maybe we might want to implement a way to disable the rotation buttons when height != width, and only allow flips ?

Big tiles seem to work fine:

Zni0AoUdC6.mp4

Not sure what confusion are you referring to.

As for non-square tilesets, I don't have one to test how it works. Half-offset square doesn't seem to have problems with transforms. Hexagon tiles will probably look wrong when rotated, but I doubt anyone expects them to retain shape after rotating. Isometric tiles will probably be fine.

@groud
Copy link
Member

groud commented Aug 22, 2023

Big tiles seem to work fine:
Not sure what confusion are you referring to.

Try with a pattern ;)

As for non-square tilesets, I don't have one to test how it works. Half-offset square doesn't seem to have problems with transforms. Hexagon tiles will probably look wrong when rotated, but I doubt anyone expects them to retain shape after rotating. Isometric tiles will probably be fine.

Same, try with a pattern there.

Maybe the solution is to disable only for patterns and not single tiles there ? That would probably make sense I think.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 22, 2023

Still looks correct:

godot.windows.editor.dev.x86_64_e4rmWQaPx4.mp4

Half-offset squares rotate a bit weird, but they correctly fit into the grid:

godot.windows.editor.dev.x86_64_98CYwP8KFO.mp4

It doesn't break anything and produces a result expected from the given grid shape.

@groud
Copy link
Member

groud commented Aug 22, 2023

Half-offset squares rotate a bit weird, but they correctly fit into the grid:

Well, yeah, that's what I mean. If you have a more meaningful pattern (like you usually do) with a house or something, the rotation does not make any sense. I think the behavior in the second video should likely be not possible.

@KoBeWi
Copy link
Member Author

KoBeWi commented Aug 26, 2023

Done.

godot.windows.editor.dev.x86_64_p33ANo7nVz.mp4

@KoBeWi
Copy link
Member Author

KoBeWi commented Sep 8, 2023

Rebased after #81070
Not sure when to clear polygon cache now. Might need separate cache for every polygon type :/

EDIT:
It's in _internal_update(), before everything else. Although I think it might invalidate some RIDs.

Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good overall. The only thing I am not sure about is whether or not the polygon cache should be in the TileMap itself or managed by the TileSet/TileSetAtlasSource itself. Just in case you might have several TileMaps sharing the same TileSet, it might be a bit more efficient.

editor/plugins/tiles/tile_map_editor.cpp Show resolved Hide resolved
@akien-mga akien-mga merged commit 4758608 into godotengine:master Sep 12, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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