mapeditor / tiled

Flexible level editor
https://www.mapeditor.org/
Other
11.22k stars 1.76k forks source link

Support rotation of Wang tiles #2833

Closed cpetig closed 3 years ago

cpetig commented 4 years ago

E.g. in this very nice tile set https://forums.rptools.net/viewtopic.php?t=21677 a lot of different terrain types are defined with smooth transitions. Also they work nicely as a Wang tile - but ... you have to (image) rotate some tiles to cover missing edges (the tile set assumes that you rotate the tiles to get complete coverage). But if you rotate the images you create additional image files (waste space) and loose the connection to the original tile (as tiled supports rotated tiles this also wastes space in the map file (increases the number of symbols up to four times)).

It would be perfect if either

I even volunteer to implement this if one of the three proposals is considered feasible and helpful, but I can't promise about the completion date.

PS: I had the feeling that this issue should already have come up multiple times, but I was unable to find any reference. I see it being proposed at https://discourse.mapeditor.org/t/wang-tiles-and-filling-improvements/2339/6 but never seems to have gotten implemented or documented.

bjorn commented 4 years ago

@cpetig Indeed this should be supported, and in fact the Wang system already can handle flipped and rotated tiles, but there is currently no way to enable this in the UI. Hence I also don't think it was really tested, but it should work if you change the tileset file in a text editor (see the hflip, vflip and dflip attributes at <wangtile>).

However, including entries for all tiles you would like to have rotated or flipped is going to get tedious, and it wasn't implemented in the UI because so far I'm not sure how to best represent that either. I think it's an interesting idea to handle these transformations as an option at the Wang set level (not sure about having it at the Wang Brush, because I can't think of a use-case where you'd want to toggle it on/off within the same set).

As such it would require only a small change to WangSet::findMatchingWangTiles, apart from the UI and loading/saving of the property. The rotated / flipped variants of the WangId can be created with available functions WangId::rotate, WangId::flipHorizontally, etc. and the relevant flags will then need to be set on the returned WangTile.

It'd be great if you could look into implementing this! Feel free to ask any questions.

cpetig commented 4 years ago

Dear Bjørn, with this good overview I will for sure give a try implementing it. Thank you very much!

cpetig commented 4 years ago

Hi Bjørn, at first I started to implement this as a wang set property, but then I took a much closer look into the code and function and realized that the terrain brush would benefit from rotation support as well. I currrently envision two new properties for tilesets: CanRotate (rotate/flip tiles as needed to cover missing terrain/wang entries) and AlternateRotation (randomly vary flipping and rotation to generate a much more versatile terrain).

cpetig commented 4 years ago

You can visit my progress at https://github.com/cpetig/tiled/tree/feature_2833 , right now the GUI and tileset attribute load+save is done. The implementation of the functionality inside the painters will be the next step, perhaps the terrain painter is more easy to complete 😉).

bjorn commented 4 years ago

@cpetig Please be aware that I've been doing a lot of work on the Wang sets and brush recently and that it now replaces terrains entirely. This work is done at PR #2888 and I aim to merge that to master on Tuesday or Wednesday.

I think this option should be on the Wang set for now and not on the tileset. You're of course welcome to work on it, though to avoid conflicts I'd advise to work on top of the wip/wang-improvements branch or to wait until I've merged that to master. Especially the changes to the painting logic, which should be done in wangfiller.cpp, which has changed a lot.

cpetig commented 4 years ago

@bjorn Seeing your recent milestone tag I suspected some plans on your side, but I hoped to be quick enough implementing it 😉. After today's deep look into terrainbrush.cpp I clearly see the advantage of unifying that with Wang tiles (which are a true superset). Thanks for pointing out the new branch I will take a closer look at it tomorrow.

cpetig commented 4 years ago

There is a new branch in my forked repository combining #2833 and #2888 : https://github.com/cpetig/tiled/tree/feature_2833_2888 - so far my initial attempt at supporting rotated terrain brush didn't work (although I feel I have seen all relevant parts of the code).

cpetig commented 4 years ago

@bjorn I absolutely adore the new blob tileset editor!

The only minor nitpick I found was that a Wang set with two edge colors and two corner colors (identical colors) gets converted into a mixed Wang set with four colors and you have to manually convert it back into a two color set again. While I understand that edge and corner colors are separate entities, identical RGB colors might give a case for handling this in the import function.

cpetig commented 4 years ago

I have a working prototype based on the newest wang improvements branch. The only drawback (?) of the current code is that when you save the tileset after loading it stores all the newly created rotated tiles (which are inaccessible from the GUI and thus no longer editable). But apart from that this simple (rotate as needed on load) approach works nicely. Probably I should add a flag preventing the rotated tiles from getting written into a file.

cpetig commented 4 years ago

Update: I implemented not writing the virtual wangtiles and am quite happy with the result, e.g. see this nice example using the 15 blob tiles from http://cr31.co.uk/stagecast/wang/blob.html screenshot tileset available here: BlobDemo.zip

I will issue a merge/pull request once wang improvements made it into the main branch.

bjorn commented 4 years ago

The only minor nitpick I found was that a Wang set with two edge colors and two corner colors (identical colors) gets converted into a mixed Wang set with four colors and you have to manually convert it back into a two color set again. While I understand that edge and corner colors are separate entities, identical RGB colors might give a case for handling this in the import function.

You're right, though I'm not sure if it's worth implementing the necessary logic for this, given that such mixed sets have never really worked conveniently before so I don't think they really exist "in the wild". Of course it might be quite trivial to handle this based on color and name for example.

I have a working prototype based on the newest wang improvements branch. The only drawback (?) of the current code is that when you save the tileset after loading it stores all the newly created rotated tiles (which are inaccessible from the GUI and thus no longer editable). But apart from that this simple (rotate as needed on load) approach works nicely. Probably I should add a flag preventing the rotated tiles from getting written into a file.

I'm looking forward to seeing the pull request! I probably would have preferred to avoid the addition of these Wang entries to the set and rather handle the variations in the Wang filler, but they will each have their pros and cons so we'll see.

cpetig commented 4 years ago

@bjorn I don't agree to make rotation a wang set property, even though this way it is more fine grained controllable and less effort to implement (no icons, just a bool property in the list) - and at first I did it exactly that way. But to me this truly feels like a tile set property. PS: I tried implementing it in the Wang filler, but the change was much more elegant and localized (only 16 lines added) in the loading routine.

bjorn commented 4 years ago

PS: I tried implementing it in the Wang filler, but the change was much more elegant and localized (only 16 lines added) in the loading routine.

Hmm, but at least it will need to be somehow supported in the editor as the user can change this property, rather than just being applied on load. Anyway, I guess we can look at those details once you open the pull request.

cpetig commented 4 years ago

I was surprised to see that the pull request did not automatically reference this issue (even though it included commits containing the reference). Fixed now.

cpetig commented 4 years ago

Hello Bjørn, any thoughts on the updated Pull request? https://github.com/bjorn/tiled/pull/2912 [The Mac build failure doesn't seem to be my fault]

bjorn commented 4 years ago

@cpetig Yeah, the macOS build has been failing for a while with what seems like a connection issue, not sure what's up with that. I'm sorry for not getting around to look into your pull request and will aim to get around to that in the coming week.

cpetig commented 4 years ago

I just realized that with more complex Wang sets a flip might result in a different WangPattern than any rotation, so I will need to modify it to generate more complete transformations.

cpetig commented 4 years ago

I just pushed an updated patch which implements flipped+rotated tiles as well as rotated tiles (so 8 variations per tile). I envision to regenerate the rotated entries upon saving a tileset (if it has rotation enabled) to no longer need to reload the tileset after modifying it. Also one could implement adding rotated tiles if they have a higher probability than already existing tiles. Apart from this this feature feels complete to me.

cpetig commented 3 years ago

@bjorn While trying to push my implementation of regenerating the virtual tile ids on change, I found your latest commit 🤷. Looks good. (Just in case you are curious my changes went into https://github.com/cpetig/tiled/tree/feature_2833_dead_end , but I am more than happy with your more elegant implementation)

bjorn commented 3 years ago

@cpetig Right, I'm sorry that we ended up working in parallel and wasting a bit of time, but it's interesting to see that approach implemented as well in any case!

cpetig commented 3 years ago

There is more recent discussion in the pull reqest https://github.com/mapeditor/tiled/pull/2912