Rotating of hexagonal tile stamps #1476

Closed
bjorn opened this Issue Mar 7, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@bjorn
Owner

bjorn commented Mar 7, 2017

Thanks to @nicolaichuk we now have 60-degree rotation steps when rotating hexagonal tiles (#1447).

However, when rotating a tile stamp with multiple tiles, their relative positions are still changed by 90 degrees rather than staying together rotated by 60 degrees. In addition, and this was already the case, the staggering is not taken into account.

This document should really help to get this implemented: http://www.redblobgames.com/grids/hexagons/#rotation

@bjorn bjorn added the bug label Mar 7, 2017

@bjorn bjorn added this to the Tiled 1.0 milestone Mar 7, 2017

@nicolaichuk

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

Bdtrotte Mar 14, 2017

Contributor

Im taking a look at this... Does anyone have a good hex tileset so I could get a better picture of how all this works?
Thanks,
Ben

Contributor

Bdtrotte commented Mar 14, 2017

Im taking a look at this... Does anyone have a good hex tileset so I could get a better picture of how all this works?
Thanks,
Ben

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 14, 2017

Owner

@Bdtrotte This feature could be implemented while testing with the hexagonal-mini.tmx example shipping with Tiled. However, you'll need to disable the actual rotation of the tile graphics since that doesn't work for that tileset. In fact, since this will be the case for many hex tilesets, this would be a useful option to have for the user as well. I'm just not entirely sure yet at which level such an option should exist.

Thanks for looking into this!

Owner

bjorn commented Mar 14, 2017

@Bdtrotte This feature could be implemented while testing with the hexagonal-mini.tmx example shipping with Tiled. However, you'll need to disable the actual rotation of the tile graphics since that doesn't work for that tileset. In fact, since this will be the case for many hex tilesets, this would be a useful option to have for the user as well. I'm just not entirely sure yet at which level such an option should exist.

Thanks for looking into this!

@Bdtrotte

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

Bdtrotte Mar 16, 2017

Contributor

However, you'll need to disable the actual rotation of the tile graphics since that doesn't work for that tileset. In fact, since this will be the case for many hex tilesets, this would be a useful option to have for the user as well.

I was putting some thought into this. It seems the only time one would want the individual hex tile graphics to rotate by 60 (where it would still line up with the gird) would be if the tiles are perfect hexagons. Maybe disable graphic rotation at 60 degree intervals automatically if they aren't perfect (within a margin of error). Then at 90 degree marks (180 and 360) it could flip them? But then this could be done by flipping vertically and horizontally anyways... Either way a manual override could be implemented.

Had some problems with qt which I am resolving now, and should at least have proper 60 stamp rotation by the end of today!

Ben

Contributor

Bdtrotte commented Mar 16, 2017

However, you'll need to disable the actual rotation of the tile graphics since that doesn't work for that tileset. In fact, since this will be the case for many hex tilesets, this would be a useful option to have for the user as well.

I was putting some thought into this. It seems the only time one would want the individual hex tile graphics to rotate by 60 (where it would still line up with the gird) would be if the tiles are perfect hexagons. Maybe disable graphic rotation at 60 degree intervals automatically if they aren't perfect (within a margin of error). Then at 90 degree marks (180 and 360) it could flip them? But then this could be done by flipping vertically and horizontally anyways... Either way a manual override could be implemented.

Had some problems with qt which I am resolving now, and should at least have proper 60 stamp rotation by the end of today!

Ben

@Bdtrotte

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

Bdtrotte Mar 17, 2017

Contributor

Started approaching this, and It's ended up being a bigger task than expected. This weekend I should have it ready for initial review.

Do you think it would be useful to make a new file which contains useful tools for dealing with hexagonal stuff. Such as converting between different coordinate systems (staggered to cube), perform rotations, maybe a class definition to package these tools a bit nicer?

With this in mind is there anything I should know or be sure to do as I make a new file for this project, noting I'm new to qt, and open source contributing.

Thanks,
Ben

Contributor

Bdtrotte commented Mar 17, 2017

Started approaching this, and It's ended up being a bigger task than expected. This weekend I should have it ready for initial review.

Do you think it would be useful to make a new file which contains useful tools for dealing with hexagonal stuff. Such as converting between different coordinate systems (staggered to cube), perform rotations, maybe a class definition to package these tools a bit nicer?

With this in mind is there anything I should know or be sure to do as I make a new file for this project, noting I'm new to qt, and open source contributing.

Thanks,
Ben

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 17, 2017

Owner

I was putting some thought into this. It seems the only time one would want the individual hex tile graphics to rotate by 60 (where it would still line up with the gird) would be if the tiles are perfect hexagons. Maybe disable graphic rotation at 60 degree intervals automatically if they aren't perfect (within a margin of error). Then at 90 degree marks (180 and 360) it could flip them? But then this could be done by flipping vertically and horizontally anyways... Either way a manual override could be implemented.

Right, I don't want to entirely automate it, mainly because that will keep the feature hidden. It would probably be better to introduce some map setting like "Tile Flags" with options like "None", "Flipping", "Flipping + 90-Degree Rotation" and "Flipping + 60-Degree Rotation".

Owner

bjorn commented Mar 17, 2017

I was putting some thought into this. It seems the only time one would want the individual hex tile graphics to rotate by 60 (where it would still line up with the gird) would be if the tiles are perfect hexagons. Maybe disable graphic rotation at 60 degree intervals automatically if they aren't perfect (within a margin of error). Then at 90 degree marks (180 and 360) it could flip them? But then this could be done by flipping vertically and horizontally anyways... Either way a manual override could be implemented.

Right, I don't want to entirely automate it, mainly because that will keep the feature hidden. It would probably be better to introduce some map setting like "Tile Flags" with options like "None", "Flipping", "Flipping + 90-Degree Rotation" and "Flipping + 60-Degree Rotation".

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 17, 2017

Owner

Do you think it would be useful to make a new file which contains useful tools for dealing with hexagonal stuff. Such as converting between different coordinate systems (staggered to cube), perform rotations, maybe a class definition to package these tools a bit nicer?

In general, I usually start with including any helper functions that seem useful directly into the .cpp file I'm using them in as static functions. When the same function turns out to be also useful somewhere else, then I start thinking about a good place to put them.

A class definition is only needed if you have one or more helper functions that depend on a certain state, so that it would be convenient to instantiate such a class first and then use its functions. I did that for example with the SnapHelper class.

With this in mind is there anything I should know or be sure to do as I make a new file for this project, noting I'm new to qt, and open source contributing.

Just put in place the license header that you see on all files, replacing the file name and putting your name and current year for the initial copyright. Also for now, new files will need to be added to both the Qbs project files (.qbs) as well as the qmake project files (.pro).

Owner

bjorn commented Mar 17, 2017

Do you think it would be useful to make a new file which contains useful tools for dealing with hexagonal stuff. Such as converting between different coordinate systems (staggered to cube), perform rotations, maybe a class definition to package these tools a bit nicer?

In general, I usually start with including any helper functions that seem useful directly into the .cpp file I'm using them in as static functions. When the same function turns out to be also useful somewhere else, then I start thinking about a good place to put them.

A class definition is only needed if you have one or more helper functions that depend on a certain state, so that it would be convenient to instantiate such a class first and then use its functions. I did that for example with the SnapHelper class.

With this in mind is there anything I should know or be sure to do as I make a new file for this project, noting I'm new to qt, and open source contributing.

Just put in place the license header that you see on all files, replacing the file name and putting your name and current year for the initial copyright. Also for now, new files will need to be added to both the Qbs project files (.qbs) as well as the qmake project files (.pro).

@Bdtrotte

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

Bdtrotte Mar 19, 2017

Contributor

I have hex rotation very close to working consistently. I was wondering if you could point me to where map settings are added. Would this then be on the properties section of the map?

Thanks!
Ben

Contributor

Bdtrotte commented Mar 19, 2017

I have hex rotation very close to working consistently. I was wondering if you could point me to where map settings are added. Would this then be on the properties section of the map?

Thanks!
Ben

@Bdtrotte

This comment has been minimized.

Show comment
Hide comment
@Bdtrotte

Bdtrotte Mar 20, 2017

Contributor

I have it working, sort of... https://github.com/Bdtrotte/tiled/tree/hexRotate

Not well enough to make a pull request, but enough to share for advice or thoughts. Most of the challenge seems to come from how the grid is staggered. One can rotate the layer, but can't be sure if the coordinates of the stamp layer match with that of the active layer, which doesn't matter with orthogonal rotation, but very much does with this. Also tricky is that the size of the resulting layer is usually bigger than the base, which requires you trim it down to only non-empty tiles after each rotation (in the current version it keeps some empty tiles in an attempt to preserve the center).

If you play around with it, it does the rotations, but will mess up the stamp occasionally. And if you keep rotating it will compound, and get worse.

I think a solution could be to wait longer before converting out of cube coordinates, as they preserve their relative position better when moving around. Perhaps this could be expanded further in the hex system.

When I have more time, I will try to focus on isolating the issues more, and perhaps building a hex class which can handle storing cube (or maybe axial) coords, and converting between that and offset coords. Also this could lead to a more elegant and versatile solution for staggering all together, as in #1496.

Feedback and advice very welcome!
Ben

Contributor

Bdtrotte commented Mar 20, 2017

I have it working, sort of... https://github.com/Bdtrotte/tiled/tree/hexRotate

Not well enough to make a pull request, but enough to share for advice or thoughts. Most of the challenge seems to come from how the grid is staggered. One can rotate the layer, but can't be sure if the coordinates of the stamp layer match with that of the active layer, which doesn't matter with orthogonal rotation, but very much does with this. Also tricky is that the size of the resulting layer is usually bigger than the base, which requires you trim it down to only non-empty tiles after each rotation (in the current version it keeps some empty tiles in an attempt to preserve the center).

If you play around with it, it does the rotations, but will mess up the stamp occasionally. And if you keep rotating it will compound, and get worse.

I think a solution could be to wait longer before converting out of cube coordinates, as they preserve their relative position better when moving around. Perhaps this could be expanded further in the hex system.

When I have more time, I will try to focus on isolating the issues more, and perhaps building a hex class which can handle storing cube (or maybe axial) coords, and converting between that and offset coords. Also this could lead to a more elegant and versatile solution for staggering all together, as in #1496.

Feedback and advice very welcome!
Ben

@bjorn

This comment has been minimized.

Show comment
Hide comment
@bjorn

bjorn Mar 20, 2017

Owner

@Bdtrotte There is no state in which a patch is not ready for pull request, but is ready enough to share for advice and thoughts. You start the discussion by opening a pull request regardless.

The only reason it would make sense to wait with the PR is because of its dependency on #1496, and updating both PRs based on feedback may be a bit of a pain. If possible, try to perform the changes on separate branches. Personally I don't see the patches touch the same code much or at all.

Owner

bjorn commented Mar 20, 2017

@Bdtrotte There is no state in which a patch is not ready for pull request, but is ready enough to share for advice and thoughts. You start the discussion by opening a pull request regardless.

The only reason it would make sense to wait with the PR is because of its dependency on #1496, and updating both PRs based on feedback may be a bit of a pain. If possible, try to perform the changes on separate branches. Personally I don't see the patches touch the same code much or at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment