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

Make gizmo plugin handle SpriteBase3D instead of Sprite3D #82901

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

kleonc
Copy link
Member

@kleonc kleonc commented Oct 6, 2023

Fixes #82873. There was no gizmo plugin handling AnimatedSprite3D... 🙃

Replaces Sprite3DGizmoPlugin with SpriteBase3DGizmoPlugin (rename + changed handled type from Sprite3D to SpriteBase3D).

Removes redundant AnimatedSprite3D::centered member which was always false. It was used only in AnimatedSprite3D::get_item_rect() where it was incorrectly checked instead of SpriteBase3D::centered (which is private and hence needs to be checked with SpriteBase3D::is_centered()). This buggy check wasn't affecting anything before this PR as AnimatedSprite3D::get_item_rect() is called only within SpriteBase3D::generate_triangle_mesh() which was never called for an AnimatedSprite3D as there was no gizmo plugin handling it. With a gizmo plugin handling AnimatedSprite3D added it was making it selectable by the non-centered area regardless of the centered property value.

@kleonc kleonc added this to the 4.2 milestone Oct 6, 2023
@kleonc kleonc requested review from a team as code owners October 6, 2023 08:43
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

I think the rename wasn't absolutely necessary, but shouldn't hurt either. It's technically more accurate.

@kleonc kleonc force-pushed the sprite-base-3d-gizmo-plugin branch from 1a5ee00 to cdae834 Compare October 6, 2023 08:53
@kleonc
Copy link
Member Author

kleonc commented Oct 6, 2023

I think the rename wasn't absolutely necessary, but shouldn't hurt either. It's technically more accurate.

Initially I've changed only the handled type but seeing other gizmo plugins for base classes like CollisionObject3DGizmoPlugin or CollisionShape3DGizmoPlugin the rename made more sense / seemed more consistent.

@kleonc kleonc force-pushed the sprite-base-3d-gizmo-plugin branch from cdae834 to db6a895 Compare October 6, 2023 09:04
@akien-mga akien-mga merged commit fba341c into godotengine:master Oct 6, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

@kleonc kleonc deleted the sprite-base-3d-gizmo-plugin branch October 6, 2023 11:06
@lorenzokerbrat
Copy link

Thanks @kleonc ! That was fast.

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.

Cannot select an AnimatedSprite3D node by clicking on it in the scene view
3 participants