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

Fix incorrect Z direction for AABB's position #99352

Merged

Conversation

Mickeon
Copy link
Contributor

@Mickeon Mickeon commented Nov 17, 2024

Fixes godotengine/godot-docs#9598

Comes from #87114 (oops)

@Mickeon Mickeon added bug documentation cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Nov 17, 2024
@Mickeon Mickeon added this to the 4.4 milestone Nov 17, 2024
@Mickeon Mickeon requested a review from a team as a code owner November 17, 2024 10:54
@Mickeon Mickeon force-pushed the documentation-aabb-coords-are-complicated branch from 9d2fdca to cd80068 Compare November 17, 2024 18:45
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.

LGTM, as far as I can tell the changes to fix the incorrect directions are correct (with my correction) and I think the word choice is good

Copy link
Member

@aaronfranke aaronfranke left a comment

Choose a reason for hiding this comment

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

Please be mindful of the precise terminology. As of Godot 4.1 we have separate concepts of forward/back direction and front/rear sides (Vector3 MODEL_* constants). We need to pick one of these wordings and use it consistently:

  • Forward -Z and back +Z.
  • Front +Z and rear -Z.

However, we should consider just mentioning directly that position is in the negative corner, -X -Y -Z, and end is in the positive corner, +X +Y +Z.

@Mickeon Mickeon force-pushed the documentation-aabb-coords-are-complicated branch from cd80068 to 6e01c03 Compare November 22, 2024 16:41
@Mickeon Mickeon requested a review from aaronfranke November 22, 2024 18:56
@Mickeon Mickeon force-pushed the documentation-aabb-coords-are-complicated branch from 6e01c03 to d143f25 Compare November 22, 2024 18:56
@Mickeon Mickeon force-pushed the documentation-aabb-coords-are-complicated branch from d143f25 to e0b1300 Compare November 22, 2024 19:25
@Repiteo Repiteo merged commit 47f3e95 into godotengine:master Nov 22, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 22, 2024

Thanks!

@Mickeon Mickeon deleted the documentation-aabb-coords-are-complicated branch November 22, 2024 21:06
@akien-mga akien-mga removed the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release documentation topic:core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AABB Confusion
6 participants