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

[3.x] Make autotiles fall back to the most similar bitmask #71533

Merged
merged 2 commits into from
Jun 13, 2023

Conversation

wareya
Copy link
Contributor

@wareya wareya commented Jan 17, 2023

Implements godotengine/godot-proposals#3851 for 3.x. If the desired bitmask doesn't have any hits in the tilemap, it searches the tilemap for tiles with the most similar bitmask and uses that instead.

Not needed for 4.x because the terrains system already behaves similarly.

Test project in this comment: #71533 (comment)

I need feedback:

  1. What is the standard way to define helper functions in the Godot codebase, like my _score_bitmask_difference and _count_bitmask_bits? Should they belong to a class or are they okay standing in open scope in a cpp file?
  2. Should this be behind a toggle? It's a compatibility break to anyone generating maps from code with broken/incomplete autotiles.
  3. If I put it behind a toggle, should it be on TileMap or somewhere else?
  4. What pieces of documentation do I need to update?

@Calinou Calinou added this to the 3.6 milestone Jan 17, 2023
@wareya wareya changed the title [WIP] (3.x) Make autotiles fall back to the most similar bitmask (3.x) Make autotiles fall back to the most similar bitmask Mar 16, 2023
@wareya wareya marked this pull request as draft March 16, 2023 07:25
@wareya wareya marked this pull request as ready for review April 20, 2023 21:04
@wareya wareya requested a review from a team as a code owner April 20, 2023 21:04
@wareya
Copy link
Contributor Author

wareya commented Apr 20, 2023

Updated to latest 3.x, added an option (on by default) to TileSet, and updated the docs. The autotile tutorial is slightly out of date, but it's in another repo and I'm not sure what the process is for updating tutorials.

I'm still not sure whether it's OK to leave helper functions like _score_bitmask_difference and _count_bitmask_bits floating around in global scope, and I'm not sure I hooked everything up in the editor properly (the tileset editor plugin has an extra layer of abstraction around tileset options), so it would be nice if someone went over this.

@akien-mga akien-mga changed the title (3.x) Make autotiles fall back to the most similar bitmask [3.x] Make autotiles fall back to the most similar bitmask Jun 7, 2023
@akien-mga akien-mga requested review from KoBeWi and groud June 7, 2023 12:39
@KoBeWi
Copy link
Member

KoBeWi commented Jun 9, 2023

What is the standard way to define helper functions in the Godot codebase, like my _score_bitmask_difference and _count_bitmask_bits? Should they belong to a class or are they okay standing in open scope in a cpp file?

They are normally part of a class.

@wareya
Copy link
Contributor Author

wareya commented Jun 10, 2023

Thanks. Rebased and made the helper functions static class functions.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 10, 2023

Do you maybe have some example TileSet (texture + resource) to make testing this easier?

scene/resources/tile_set.h Outdated Show resolved Hide resolved
@wareya
Copy link
Contributor Author

wareya commented Jun 10, 2023

Test project:
TileTest.zip

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I tested the feature and it does visibly improve autotiling. I'm not familiar with the implemented algorithm, but the code is clean and works™, so the implementation is alright I guess.

As for the concerns about compatibility breakage mentioned in the proposal, this only affects newly drawn tiles in the editor, no? Then I don't think a different default behavior is a problem.
(although maybe it does affect procedurally generated TileMaps; not sure if it's fine 🤔)

@akien-mga akien-mga merged commit 4b5e94d into godotengine:3.x Jun 13, 2023
@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
Development

Successfully merging this pull request may close these issues.

5 participants