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

Add the ability to look-at in model-space. #76082

Merged

Conversation

reduz
Copy link
Member

@reduz reduz commented Apr 15, 2023

This is a much simpler attempt to solve the same problem as #76060, but without breaking any compatibility, based on the feedback gathered yesterday.

  • Adds a description of what model space is in the Vector3 enums (MODEL_* constants). This has the proper axes laid out for imported 3D assets.
  • Adds the option to look_at using model_space, which uses Vector3.MODEL_FRONT as forward vector.

The attempt of this PR is to still break the assumption that there is a single direction of forward (which is not the case in Godot) and make it easier to understand where 3D models are facing, as well as orienting them via look_at.

@reduz reduz requested review from a team as code owners April 15, 2023 08:06
@reduz reduz force-pushed the ability-to-look-at-in-model-space branch from 9e67ae3 to 59f98d6 Compare April 15, 2023 08:12
@TokageItLab
Copy link
Member

TokageItLab commented Apr 15, 2023

If it is an alternative to #76060, then it need to add an option to Basis::looking_at() and Transform3D::looking_at() as well.

@adamscott
Copy link
Member

Looks like a more reasonable PR. Introducing breaking changes at that level would not only break projects, but addons too.

We could maybe introduce too CAMERA_FORWARD as an alias to FORWARD. Could make things more verbose.

@adamscott
Copy link
Member

  • which uses Vector3.MODEL_FRONT as forward vector.

@reduz Why not Vector3.MODEL_FORWARD?

@reduz
Copy link
Member Author

reduz commented Apr 18, 2023

@adamscott The rationale for the model constants is that this is what you see in the 3D application (both Godot and any other). In other words, for a 3D model, forward is not a convention but artists just do it this way because its more practical due to how modelling apps work. What is a convention is that this is the front view of the model, hence the constant names.

Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

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

Since @reduz seems to be busy now, I just changed to Basis::looking_at() and Transform3D::looking_at() to follow #76060.

@TokageItLab TokageItLab requested a review from akien-mga May 23, 2023 08:38
@TokageItLab TokageItLab force-pushed the ability-to-look-at-in-model-space branch 2 times, most recently from d308592 to 72e0dc4 Compare May 24, 2023 01:04
This is a much simpler attempt to solve the same problem as godotengine#76060, but without breaking any compatibility.

* Adds a description of what model space is in the Vector3 enums (MODEL_* constants). This has the proper axes laid out for imported 3D assets.
* Adds the option to `look_at` using model_space, which uses Vector3.MODEL_FRONT as forward vector.

The attempt of this PR is to still break the assumption that there is a single direction of forward (which is not the case in Godot)
and make it easier to understand where 3D models are facing, as well as orienting them via look_at.
@TokageItLab TokageItLab force-pushed the ability-to-look-at-in-model-space branch from 72e0dc4 to 5fdc123 Compare May 24, 2023 01:10
@akien-mga akien-mga modified the milestones: 4.x, 4.1 May 24, 2023
@akien-mga akien-mga merged commit 6f34a23 into godotengine:master May 24, 2023
@akien-mga
Copy link
Member

Thanks!

geowarin added a commit to geowarin/godot that referenced this pull request Jul 6, 2023
To LookAt methods.
Also adds Vector3 Model constants.

These were not added after godotengine#76082 was merged.
suinswofi pushed a commit to suinswofi/godot that referenced this pull request Jul 20, 2023
To LookAt methods.
Also adds Vector3 Model constants.

These were not added after godotengine#76082 was merged.
IntangibleMatter pushed a commit to IntangibleMatter/godot that referenced this pull request Aug 13, 2023
To LookAt methods.
Also adds Vector3 Model constants.

These were not added after godotengine#76082 was merged.
YuriSizov pushed a commit to YuriSizov/godot that referenced this pull request Aug 31, 2023
To LookAt methods.
Also adds Vector3 Model constants.

These were not added after godotengine#76082 was merged.

(cherry picked from commit 6c6e5c4)
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.

4 participants