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 look_at_from_position() usage in Your first 3D game tutorial #10666

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Feb 10, 2025

The mob's orientation was previously shifted according to the player's height, which could lead to collision and movement issues that were difficult to diagnose.

@Calinou Calinou added bug area:getting started Issues and PRs related to the Getting Started section of the documentation area:manual Issues and PRs related to the Manual/Tutorials section of the documentation cherrypick:4.3 labels Feb 10, 2025
The mob's orientation was previously shifted according to the player's
height, which could lead to collision and movement issues that were
difficult to diagnose.
@Calinou Calinou force-pushed the getting-started-fix-look-at-from-position branch from 76af79f to 14823a2 Compare February 10, 2025 12:10
@tetrapod00 tetrapod00 changed the title Fix look_at_from_position() usage in Your first 2D game tutorial Fix look_at_from_position() usage in Your first 3D game tutorial Feb 10, 2025
Copy link
Contributor

@tetrapod00 tetrapod00 left a comment

Choose a reason for hiding this comment

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

Needs the C# fix but the motivation makes sense and the change looks right.

//
// Ignore the player's height, so that the mob's orientation is not slightly
// shifted if the mob spawns while the player is jumping.
Vector3 target = new Vector3(player_position.x, start_position.y, player_position.z);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Vector3 target = new Vector3(player_position.x, start_position.y, player_position.z);
Vector3 target = new Vector3(playerPosition.X, startPosition.Y, playerPosition.Z);

Only tested to compile the single script, didn't test the whole tutorial.

//
// Ignore the player's height, so that the mob's orientation is not slightly
// shifted if the mob spawns while the player is jumping.
Vector3 target = new Vector3(player_position.x, start_position.y, player_position.z);
Copy link
Contributor

@tetrapod00 tetrapod00 Feb 10, 2025

Choose a reason for hiding this comment

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

Suggested change
Vector3 target = new Vector3(player_position.x, start_position.y, player_position.z);
Vector3 target = new Vector3(playerPosition.X, startPosition.Y, playerPosition.Z);

@skyace65 skyace65 added waiting on PR merge PR's that can't be merged until an engine PR is merged first Linked Demo PR Doc PRs that are tied to a demo repository PR and removed waiting on PR merge PR's that can't be merged until an engine PR is merged first labels Feb 10, 2025
@skyace65
Copy link
Contributor

This would supersede #8718

@aaronfranke
Copy link
Member

PR #8718 is simpler and would accomplish the same thing, but I'm not sure if we want to encourage the practice of modifying function parameters.

@skyace65
Copy link
Contributor

Found another one this would supersede #7123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:getting started Issues and PRs related to the Getting Started section of the documentation area:manual Issues and PRs related to the Manual/Tutorials section of the documentation bug cherrypick:4.3 Linked Demo PR Doc PRs that are tied to a demo repository PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants