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.

Sign up for GitHub

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

Jump to bottom

Fix Pathfollow direction and add Z forward option #72842

Merged
merged 1 commit into from
May 24, 2023
Merged

Fix Pathfollow direction and add Z forward option #72842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/classes/Basis.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
<description>
Creates a Basis with a rotation such that the forward axis (-Z) points towards the [param target] position.
The up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the forward axis. The resulting Basis is orthonormalized. The [param target] and [param up] vectors cannot be zero, and cannot be parallel to each other.
If [param use_model_front] is [code]true[/code], the +Z axis (asset front) is treated as forward (implies +X is left) and points toward the [param target] position. By default, the -Z axis (camera forward) is treated as forward (implies +X is right).
</description>
</method>
<method name="orthonormalized" qualifiers="const">
Expand Down
3 changes: 2 additions & 1 deletion doc/classes/Node3D.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,11 @@
<param index="1" name="up" type="Vector3" default="Vector3(0, 1, 0)" />
<param index="2" name="use_model_front" type="bool" default="false" />
<description>
Rotates the node so that the local forward axis (-Z, [constant Vector3.FORWARD]) points toward the [param target] position. If the [param use_model_front] options is specified, then the model is oriented in reverse, towards the model front axis (+Z, [constant Vector3.MODEL_FRONT]), which is more useful for orienting 3D models.
Rotates the node so that the local forward axis (-Z, [constant Vector3.FORWARD]) points toward the [param target] position.
The local up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the local forward axis. The resulting transform is orthogonal, and the scale is preserved. Non-uniform scaling may not work correctly.
The [param target] position cannot be the same as the node's position, the [param up] vector cannot be zero, and the direction from the node's position to the [param target] vector cannot be parallel to the [param up] vector.
Operations take place in global space, which means that the node must be in the scene tree.
If [param use_model_front] is [code]true[/code], the +Z axis (asset front) is treated as forward (implies +X is left) and points toward the [param target] position. By default, the -Z axis (camera forward) is treated as forward (implies +X is right).
</description>
</method>
<method name="look_at_from_position">
Expand Down
3 changes: 0 additions & 3 deletions doc/classes/PathFollow2D.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,6 @@
<member name="h_offset" type="float" setter="set_h_offset" getter="get_h_offset" default="0.0">
The node's offset along the curve.
</member>
<member name="lookahead" type="float" setter="set_lookahead" getter="get_lookahead" default="4.0">
TokageItLab marked this conversation as resolved.
Show resolved Hide resolved
How far to look ahead of the curve to calculate the tangent if the node is rotating. E.g. shorter lookaheads will lead to faster rotations.
</member>
<member name="loop" type="bool" setter="set_loop" getter="has_loop" default="true">
If [code]true[/code], any offset outside the path's length will wrap around, instead of stopping at the ends. Use it for cyclic paths.
</member>
Expand Down
3 changes: 3 additions & 0 deletions doc/classes/PathFollow3D.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
<member name="tilt_enabled" type="bool" setter="set_tilt_enabled" getter="is_tilt_enabled" default="true">
If [code]true[/code], the tilt property of [Curve3D] takes effect.
</member>
<member name="use_model_front" type="bool" setter="set_use_model_front" getter="is_using_model_front" default="false">
If [code]true[/code], the node moves on the travel path with orienting the +Z axis as forward. See also [constant Vector3.FORWARD] and [constant Vector3.MODEL_FRONT].
</member>
<member name="v_offset" type="float" setter="set_v_offset" getter="get_v_offset" default="0.0">
The node's offset perpendicular to the curve.
</member>
Expand Down
1 change: 1 addition & 0 deletions doc/classes/Transform3D.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
<description>
Returns a copy of the transform rotated such that the forward axis (-Z) points towards the [param target] position.
The up axis (+Y) points as close to the [param up] vector as possible while staying perpendicular to the forward axis. The resulting transform is orthonormalized. The existing rotation, scale, and skew information from the original transform is discarded. The [param target] and [param up] vectors cannot be zero, cannot be parallel to each other, and are defined in global/parent space.
If [param use_model_front] is [code]true[/code], the +Z axis (asset front) is treated as forward (implies +X is left) and points toward the [param target] position. By default, the -Z axis (camera forward) is treated as forward (implies +X is right).
</description>
</method>
<method name="orthonormalized" qualifiers="const">
Expand Down
4 changes: 2 additions & 2 deletions editor/plugins/path_3d_editor_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,10 @@ void Path3DGizmo::redraw() {

// Fish Bone.
v3p.push_back(p1);
v3p.push_back(p1 + (side - forward + up * 0.3) * 0.06);
v3p.push_back(p1 + (side + forward + up * 0.3) * 0.06);

v3p.push_back(p1);
v3p.push_back(p1 + (-side - forward + up * 0.3) * 0.06);
v3p.push_back(p1 + (-side + forward + up * 0.3) * 0.06);
}

add_lines(v3p, path_material);
Expand Down
32 changes: 10 additions & 22 deletions scene/2d/path_2d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,11 @@ void PathFollow2D::_notification(int p_what) {
}
}

void PathFollow2D::set_cubic_interpolation(bool p_enable) {
cubic = p_enable;
void PathFollow2D::set_cubic_interpolation_enabled(bool p_enabled) {
cubic = p_enabled;
}

bool PathFollow2D::get_cubic_interpolation() const {
bool PathFollow2D::is_cubic_interpolation_enabled() const {
return cubic;
}

Expand Down Expand Up @@ -312,26 +312,22 @@ void PathFollow2D::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_progress_ratio", "ratio"), &PathFollow2D::set_progress_ratio);
ClassDB::bind_method(D_METHOD("get_progress_ratio"), &PathFollow2D::get_progress_ratio);

ClassDB::bind_method(D_METHOD("set_rotates", "enable"), &PathFollow2D::set_rotates);
ClassDB::bind_method(D_METHOD("is_rotating"), &PathFollow2D::is_rotating);
ClassDB::bind_method(D_METHOD("set_rotates", "enabled"), &PathFollow2D::set_rotation_enabled);
ClassDB::bind_method(D_METHOD("is_rotating"), &PathFollow2D::is_rotation_enabled);

ClassDB::bind_method(D_METHOD("set_cubic_interpolation", "enable"), &PathFollow2D::set_cubic_interpolation);
ClassDB::bind_method(D_METHOD("get_cubic_interpolation"), &PathFollow2D::get_cubic_interpolation);
ClassDB::bind_method(D_METHOD("set_cubic_interpolation", "enabled"), &PathFollow2D::set_cubic_interpolation_enabled);
ClassDB::bind_method(D_METHOD("get_cubic_interpolation"), &PathFollow2D::is_cubic_interpolation_enabled);

ClassDB::bind_method(D_METHOD("set_loop", "loop"), &PathFollow2D::set_loop);
ClassDB::bind_method(D_METHOD("has_loop"), &PathFollow2D::has_loop);

ClassDB::bind_method(D_METHOD("set_lookahead", "lookahead"), &PathFollow2D::set_lookahead);
ClassDB::bind_method(D_METHOD("get_lookahead"), &PathFollow2D::get_lookahead);

ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "progress", PROPERTY_HINT_RANGE, "0,10000,0.01,or_less,or_greater,suffix:px"), "set_progress", "get_progress");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "progress_ratio", PROPERTY_HINT_RANGE, "0,1,0.0001,or_less,or_greater", PROPERTY_USAGE_EDITOR), "set_progress_ratio", "get_progress_ratio");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "h_offset"), "set_h_offset", "get_h_offset");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "v_offset"), "set_v_offset", "get_v_offset");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "rotates"), "set_rotates", "is_rotating");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "cubic_interp"), "set_cubic_interpolation", "get_cubic_interpolation");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "loop"), "set_loop", "has_loop");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "lookahead", PROPERTY_HINT_RANGE, "0.001,1024.0,0.001"), "set_lookahead", "get_lookahead");
}

void PathFollow2D::set_progress(real_t p_progress) {
Expand Down Expand Up @@ -395,20 +391,12 @@ real_t PathFollow2D::get_progress_ratio() const {
}
}

void PathFollow2D::set_lookahead(real_t p_lookahead) {
lookahead = p_lookahead;
}

real_t PathFollow2D::get_lookahead() const {
return lookahead;
}

void PathFollow2D::set_rotates(bool p_rotates) {
rotates = p_rotates;
void PathFollow2D::set_rotation_enabled(bool p_enabled) {
rotates = p_enabled;
_update_transform();
}

bool PathFollow2D::is_rotating() const {
bool PathFollow2D::is_rotation_enabled() const {
return rotates;
}

Expand Down
12 changes: 4 additions & 8 deletions scene/2d/path_2d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class PathFollow2D : public Node2D {
Timer *update_timer = nullptr;
real_t h_offset = 0.0;
real_t v_offset = 0.0;
real_t lookahead = 4.0;
bool cubic = true;
bool loop = true;
bool rotates = true;
Expand Down Expand Up @@ -98,17 +97,14 @@ class PathFollow2D : public Node2D {
void set_progress_ratio(real_t p_ratio);
real_t get_progress_ratio() const;

void set_lookahead(real_t p_lookahead);
real_t get_lookahead() const;

void set_loop(bool p_loop);
bool has_loop() const;

void set_rotates(bool p_rotates);
bool is_rotating() const;
void set_rotation_enabled(bool p_enabled);
bool is_rotation_enabled() const;

void set_cubic_interpolation(bool p_enable);
bool get_cubic_interpolation() const;
void set_cubic_interpolation_enabled(bool p_enabled);
bool is_cubic_interpolation_enabled() const;

PackedStringArray get_configuration_warnings() const override;

Expand Down
29 changes: 22 additions & 7 deletions scene/3d/path_3d.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,9 @@ void PathFollow3D::_update_transform(bool p_update_xyz_rot) {
t.origin = pos;
} else {
t = c->sample_baked_with_rotation(progress, cubic, false);
if (use_model_front) {
t.basis *= Basis::from_scale(Vector3(-1.0, 1.0, -1.0));
}
Vector3 forward = t.basis.get_column(2); // Retain tangent for applying tilt
t = PathFollow3D::correct_posture(t, rotation_mode);

Expand Down Expand Up @@ -230,11 +233,11 @@ void PathFollow3D::_notification(int p_what) {
}
}

void PathFollow3D::set_cubic_interpolation(bool p_enable) {
cubic = p_enable;
void PathFollow3D::set_cubic_interpolation_enabled(bool p_enabled) {
cubic = p_enabled;
}

bool PathFollow3D::get_cubic_interpolation() const {
bool PathFollow3D::is_cubic_interpolation_enabled() const {
Copy link
Member

Choose a reason for hiding this comment

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

These changes seem unrelated to adding a new model front mode to look_at. I'm not sure why not split these changes into a separate PR.

Copy link
Member Author

@TokageItLab TokageItLab May 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I talked to @akien-mga earlier and got his agreement that these method names are bad, but he seems to have a terrible aversion to changing method names in 4.x, so I decided to change only the cpp function names same as #74190 (comment). Binding method names have not been changed. If he does not like it, I will split it.

return cubic;
}

Expand Down Expand Up @@ -314,8 +317,11 @@ void PathFollow3D::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_rotation_mode", "rotation_mode"), &PathFollow3D::set_rotation_mode);
ClassDB::bind_method(D_METHOD("get_rotation_mode"), &PathFollow3D::get_rotation_mode);

ClassDB::bind_method(D_METHOD("set_cubic_interpolation", "enable"), &PathFollow3D::set_cubic_interpolation);
ClassDB::bind_method(D_METHOD("get_cubic_interpolation"), &PathFollow3D::get_cubic_interpolation);
ClassDB::bind_method(D_METHOD("set_cubic_interpolation", "enabled"), &PathFollow3D::set_cubic_interpolation_enabled);
ClassDB::bind_method(D_METHOD("get_cubic_interpolation"), &PathFollow3D::is_cubic_interpolation_enabled);

ClassDB::bind_method(D_METHOD("set_use_model_front", "enabled"), &PathFollow3D::set_use_model_front);
ClassDB::bind_method(D_METHOD("is_using_model_front"), &PathFollow3D::is_using_model_front);

ClassDB::bind_method(D_METHOD("set_loop", "loop"), &PathFollow3D::set_loop);
ClassDB::bind_method(D_METHOD("has_loop"), &PathFollow3D::has_loop);
Expand All @@ -330,6 +336,7 @@ void PathFollow3D::_bind_methods() {
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "h_offset", PROPERTY_HINT_NONE, "suffix:m"), "set_h_offset", "get_h_offset");
ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "v_offset", PROPERTY_HINT_NONE, "suffix:m"), "set_v_offset", "get_v_offset");
ADD_PROPERTY(PropertyInfo(Variant::INT, "rotation_mode", PROPERTY_HINT_ENUM, "None,Y,XY,XYZ,Oriented"), "set_rotation_mode", "get_rotation_mode");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "use_model_front"), "set_use_model_front", "is_using_model_front");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "cubic_interp"), "set_cubic_interpolation", "get_cubic_interpolation");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "loop"), "set_loop", "has_loop");
ADD_PROPERTY(PropertyInfo(Variant::BOOL, "tilt_enabled"), "set_tilt_enabled", "is_tilt_enabled");
Expand Down Expand Up @@ -412,6 +419,14 @@ PathFollow3D::RotationMode PathFollow3D::get_rotation_mode() const {
return rotation_mode;
}

void PathFollow3D::set_use_model_front(bool p_use_model_front) {
use_model_front = p_use_model_front;
}

bool PathFollow3D::is_using_model_front() const {
return use_model_front;
}

void PathFollow3D::set_loop(bool p_loop) {
loop = p_loop;
}
Expand All @@ -420,8 +435,8 @@ bool PathFollow3D::has_loop() const {
return loop;
}

void PathFollow3D::set_tilt_enabled(bool p_enable) {
tilt_enabled = p_enable;
void PathFollow3D::set_tilt_enabled(bool p_enabled) {
tilt_enabled = p_enabled;
}

bool PathFollow3D::is_tilt_enabled() const {
Expand Down
11 changes: 8 additions & 3 deletions scene/3d/path_3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ class PathFollow3D : public Node3D {
ROTATION_ORIENTED
};

bool use_model_front = false;

static Transform3D correct_posture(Transform3D p_transform, PathFollow3D::RotationMode p_rotation_mode);

private:
Expand Down Expand Up @@ -108,14 +110,17 @@ class PathFollow3D : public Node3D {
void set_loop(bool p_loop);
bool has_loop() const;

void set_tilt_enabled(bool p_enable);
void set_tilt_enabled(bool p_enabled);
bool is_tilt_enabled() const;

void set_rotation_mode(RotationMode p_rotation_mode);
RotationMode get_rotation_mode() const;

void set_cubic_interpolation(bool p_enable);
bool get_cubic_interpolation() const;
void set_use_model_front(bool p_use_model_front);
bool is_using_model_front() const;

void set_cubic_interpolation_enabled(bool p_enabled);
bool is_cubic_interpolation_enabled() const;

PackedStringArray get_configuration_warnings() const override;

Expand Down
8 changes: 4 additions & 4 deletions scene/resources/curve.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,9 +1648,9 @@ void Curve3D::_bake() const {
Vector3 forward = forward_ptr[0];

if (abs(forward.dot(Vector3(0, 1, 0))) > 1.0 - UNIT_EPSILON) {
frame_prev = Basis::looking_at(-forward, Vector3(1, 0, 0));
frame_prev = Basis::looking_at(forward, Vector3(1, 0, 0));
} else {
frame_prev = Basis::looking_at(-forward, Vector3(0, 1, 0));
frame_prev = Basis::looking_at(forward, Vector3(0, 1, 0));
}

up_write[0] = frame_prev.get_column(1);
Expand Down Expand Up @@ -1809,8 +1809,8 @@ Basis Curve3D::_sample_posture(Interval p_interval, bool p_apply_tilt) const {
}

// Build frames at both ends of the interval, then interpolate.
const Basis frame_begin = Basis::looking_at(-forward_begin, up_begin);
const Basis frame_end = Basis::looking_at(-forward_end, up_end);
const Basis frame_begin = Basis::looking_at(forward_begin, up_begin);
const Basis frame_end = Basis::looking_at(forward_end, up_end);
const Basis frame = frame_begin.slerp(frame_end, frac).orthonormalized();

if (!p_apply_tilt) {
Expand Down
6 changes: 3 additions & 3 deletions tests/scene/test_curve_3d.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,9 @@ TEST_CASE("[Curve3D] Sampling") {
}

SUBCASE("sample_baked_with_rotation") {
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 0, 0))) == Transform3D(Basis(Vector3(0, 0, 1), Vector3(1, 0, 0), Vector3(0, 1, 0)), Vector3(0, 0, 0)));
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 25, 0))) == Transform3D(Basis(Vector3(0, 0, 1), Vector3(1, 0, 0), Vector3(0, 1, 0)), Vector3(0, 25, 0)));
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 50, 0))) == Transform3D(Basis(Vector3(0, 0, 1), Vector3(1, 0, 0), Vector3(0, 1, 0)), Vector3(0, 50, 0)));
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 0, 0))) == Transform3D(Basis(Vector3(0, 0, -1), Vector3(1, 0, 0), Vector3(0, -1, 0)), Vector3(0, 0, 0)));
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 25, 0))) == Transform3D(Basis(Vector3(0, 0, -1), Vector3(1, 0, 0), Vector3(0, -1, 0)), Vector3(0, 25, 0)));
CHECK(curve->sample_baked_with_rotation(curve->get_closest_offset(Vector3(0, 50, 0))) == Transform3D(Basis(Vector3(0, 0, -1), Vector3(1, 0, 0), Vector3(0, -1, 0)), Vector3(0, 50, 0)));
}

SUBCASE("sample_baked_tilt") {
Expand Down