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

Move tile transforms handling cache to TileData #84660

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

groud
Copy link
Member

@groud groud commented Nov 9, 2023

This PR does multiple things:

Bugsquad edit: Fixes #83825

@groud groud requested review from a team as code owners November 9, 2023 11:53
@groud groud requested a review from KoBeWi November 9, 2023 11:54
@YuriSizov YuriSizov added this to the 4.3 milestone Nov 9, 2023
@groud groud force-pushed the better_tileset_polygons branch 3 times, most recently from a6dbdc2 to ca65c8a Compare November 9, 2023 13:14
@groud groud requested review from a team as code owners November 9, 2023 13:14
@dsnopek
Copy link
Contributor

dsnopek commented Nov 9, 2023

I only looked at the GDExtension compatibility changes, but those look good to me!

Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Looks correct otherwise

misc/extension_api_validation/4.1-stable.expected Outdated Show resolved Hide resolved
misc/extension_api_validation/4.1-stable.expected Outdated Show resolved Hide resolved
scene/resources/tile_set.cpp Outdated Show resolved Hide resolved
scene/resources/tile_set.cpp Outdated Show resolved Hide resolved
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 added some minor optimization suggestions (they apply to physics and navigation too ofc), but otherwise looks fine.

scene/resources/tile_set.compat.inc Outdated Show resolved Hide resolved
scene/resources/tile_set.compat.inc Outdated Show resolved Hide resolved
modules/navigation/nav_mesh_generator_2d.cpp Outdated Show resolved Hide resolved
modules/navigation/nav_mesh_generator_2d.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map.cpp Outdated Show resolved Hide resolved
scene/resources/tile_set.cpp Outdated Show resolved Hide resolved
scene/resources/tile_set.cpp Outdated Show resolved Hide resolved
scene/resources/tile_set.cpp Outdated Show resolved Hide resolved
Copy link
Member

@AThousandShips AThousandShips left a comment

Choose a reason for hiding this comment

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

Missed in docs

doc/classes/TileData.xml Outdated Show resolved Hide resolved
doc/classes/TileData.xml Outdated Show resolved Hide resolved
@groud
Copy link
Member Author

groud commented Nov 13, 2023

Thanks for the feedback @kleonc and @AThousandShips, should be fixed now :)

Copy link
Member

@kleonc kleonc left a comment

Choose a reason for hiding this comment

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

Haven't tested, just looked at the code changes, LGTM!

@YuriSizov
Copy link
Contributor

@groud Could you address the comment above and do the rebase? Otherwise this seems to be good to go based on reviews.

@YuriSizov
Copy link
Contributor

Still has conflicts 🙃

@groud
Copy link
Member Author

groud commented Dec 13, 2023

@groud Could you address the comment above and do the rebase? Otherwise this seems to be good to go based on reviews.

Done.

Still has conflicts 🙃

Yeah, sorry for that, I forgot to rebase.

@YuriSizov YuriSizov merged commit 4cf6325 into godotengine:master Dec 14, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

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.

Painting a rotated or flipped tile in a Tilemap loses collision
8 participants