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

Implement X-draw-order switch in TileMapLayer #92787

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

groud
Copy link
Member

@groud groud commented Jun 5, 2024

So, I marked it as a bug since setting on Y-sort does end invert the X-drawing order, and I think it should not. So, this makes the default behavior consistent with when Y-sort is disabled.

However, for users who would like the keep the old behavior (which is a valid use case too), I added an x_draw_order_reversed switch.

I think it's a pretty safe change, and it would be very nice to have this bugfix/enhancement in 4.3.

Closes #71257
Closes godotengine/godot-proposals#9882

@groud groud added this to the 4.3 milestone Jun 5, 2024
@groud groud requested a review from a team as a code owner June 5, 2024 10:12
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.

Was just looking into this too. 😄

Missing docs.

Also while at it:

  • there's missing guard in TileMapLayer::set_use_kinematic_bodies,
  • in TileMapLayer::set_rendering_quadrant_size dirty flag should be set after the error and assignment.

scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.h Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.h Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
scene/2d/tile_map_layer.cpp Outdated Show resolved Hide resolved
@groud groud force-pushed the tilemap_x_sort branch from 14d5076 to cb9dc59 Compare June 5, 2024 10:48
@groud groud requested a review from a team as a code owner June 5, 2024 10:48
@groud groud force-pushed the tilemap_x_sort branch from cb9dc59 to 2be9bd9 Compare June 5, 2024 15:23
@groud
Copy link
Member Author

groud commented Jun 5, 2024

I've just pushed a new version, I believe everything 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.

The code LGTM, haven't tested though. 🙃

+Still you could address:

Also while at it:

  • there's missing guard in TileMapLayer::set_use_kinematic_bodies,
  • in TileMapLayer::set_rendering_quadrant_size dirty flag should be set after the error and assignment.

doc/classes/TileMapLayer.xml Outdated Show resolved Hide resolved
doc/classes/TileMap.xml Outdated Show resolved Hide resolved
doc/classes/TileMapLayer.xml Outdated Show resolved Hide resolved
@groud groud force-pushed the tilemap_x_sort branch from 2be9bd9 to f2c8846 Compare June 7, 2024 07:41
@groud
Copy link
Member Author

groud commented Jun 7, 2024

Done!

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.

Rechecked the changes, noticed only another little typo in the docs. Besides looks mergeable to me. 👍

Also briefly tested this, AFAICT it works as intended.

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

groud commented Jun 10, 2024

Thanks, should be good now!

@akien-mga akien-mga merged commit 95b84f1 into godotengine:master Jun 10, 2024
16 checks passed
@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
3 participants