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 new terrain painting mode in TileMap to force diagonal connection #72030

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

groud
Copy link
Member

@groud groud commented Jan 25, 2023

Allows connecting diagonally when required. This was kind of need for having the behavior asked in #71829.

Peek.25-01-2023.12-05.mp4

@OddlifeDev
Copy link

Amazing this looks great, thanks so much for the work and your attention to this issue, much apreciated.

@YuriSizov YuriSizov modified the milestones: 4.0, 4.1 Feb 10, 2023
doc/classes/TileMap.xml Outdated Show resolved Hide resolved
@YuriSizov
Copy link
Contributor

Since this PR breaks API compatibility, which we should avoid in minor releases, this makes it less than ideal solution. Still, if we prefer to break existing projects to some degree to achieve this functionality, then we need to make sure the risk is minimized for possible future changes of the sort. So I propose to unity these boolean flags into a single method parameter exposed as a bitfield enum.

So instead of adding a new flag, we replace existing flag with a bitfield parameter and expose two options for it, ignore_empty_terrains and connect_corners. This allows us to add more configuration options in future without creating compatibility breakage, or minimizing its effect. It also doesn't extend method's signature further (remember, that ideally methods should have no more than 3 parameters to be human-readable).

@groud groud force-pushed the new_terrain_painting_mode branch from 656a76d to 8fa0994 Compare March 6, 2023 10:40
@groud groud force-pushed the new_terrain_painting_mode branch from 8fa0994 to bdb0c50 Compare March 6, 2023 10:41
@groud
Copy link
Member Author

groud commented Mar 6, 2023

I've updated the PR to use another function instead. I think it's simpler to use than a bitfield enum.

@YuriSizov YuriSizov modified the milestones: 4.1, 4.2 Jun 14, 2023
@dandeliondino
Copy link

dandeliondino commented Jun 19, 2023

I think this is a good feature to add, thank you for doing it. I am curious why it's a draw mode and not a terrain mode, though (or a sub-mode of corners and sides). It seems it could be more easily added as a terrain mode without breaking compatibility? It also doesn't have to be mutually exclusive with path drawing mode, right?

My understanding is the diagonal corner matching is just the difference, in corners and sides mode, between whether you match the single diagonal corner bit or all adjacent corner bits. Which was also the difference between 3x3 and 3x3 minimal modes in 3.x. This means that diagonal mode should require a full 256-tile set.

But perhaps I am missing something because I just tried testing it and can't get it working. As expected, the corners-mode won't match correctly with a 47-tile corners and sides tileset, but it also isn't working with a full 256-tile corners and sides tileset.

47 tiles:

corners_pr_47

256 tiles:

corners_pr_256

Here is an example using the same tileset and bitmasks of how it should be matching the 256-tile corners:

corners_pr_256_mockup

And here is the again issue, but with a bitmask tile template from TilePipe2, so it's easier to see what's happening:

corners_pr_256_bits

It seems the last tile painted is usually matched correctly at diagonal corners, but the tile diagonal to it isn't always updating as expected.
Edit: I just watched that last gif more closely and the painted tile also seems to be choosing corners incorrectly sometimes.

@JustMog
Copy link

JustMog commented Sep 24, 2023

i don't understand why this should be a separate drawing mode. surely whether diagonals should be taken into account is implicit in the terrain matching mode, i.e. if i didn't expect diagonals to connect, i would be using "match sides"?

@AThousandShips AThousandShips modified the milestones: 4.2, 4.3 Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants