Changed the way the rotation of a curve at a point is evaluated to match PathFollow2D

[botero-dev]

Fixes #78361

[AThousandShips]

This breaks PathFollow2D though, you need to update it as well, and the unit tests

[botero-dev]

merging this would also benefit this other PR I'm working on #77819

[botero-dev]

This breaks PathFollow2D though, you need to update it as well, and the unit tests

You're right, I'll do that now

[botero-dev]

I fixed the PathFollow2D code, but I have to leave now, I'll come back later to update tests and docs.

[capnm]

Well, you need to tell your kids to paint rockets horizontally. In any case, better than in mirrored shape :)
LGTM

[Calinou]

I tested this PR locally (rebased on top of master f2ce0b6) with the MRP. While it fixes the issue mentioned in #78361, It breaks rendering of "fish bones" in the editor:

This PR

master

This is likely resolved by rotating the fish bone rendering, but it does highlight the compatibility-breaking nature of this PR. As a result, I removed the cherry-pick label.

[capnm]

IIRC if I tested the initial commit the fish direction path was was ok.

[capnm]

This is likely resolved by rotating the fish bone rendering

Where is this actually painted? curve_editor_plugin is a non-functioning piece of code.

[AThousandShips]

It's in Path2D directly, so scene/2d/path_2d.cpp:

godot/scene/2d/path_2d.cpp

Lines 141 to 159 in e74bf83

	// Draw fish bones
	{
	PackedVector2Array v2p;
	v2p.resize(3);
	Vector2 *w = v2p.ptrw();
	for (int i = 0; i < sample_count; i++) {
	const Vector2 p = r[i].get_origin();
	const Vector2 side = r[i].columns[0];
	const Vector2 forward = r[i].columns[1];
	// Fish Bone.
	w[0] = p + (side - forward) * 5;
	w[1] = p;
	w[2] = p + (-side - forward) * 5;
	draw_polyline(v2p, get_tree()->get_debug_paths_color(), line_width * 0.5, false);
	}
	}

Fix should be to swap:

const Vector2 side = r[i].columns[0];
const Vector2 forward = r[i].columns[1];

For:

const Vector2 side = r[i].columns[1];
const Vector2 forward = r[i].columns[0];

But haven't tested

[capnm]

@0xafbf needs squash and rebase.

[kleonc]

LGTM! 👍

Changed the way the rotation of a curve at a point is evaluated to match PathFollow2D

To precise, this PR rather makes Curve2D::sample_baked_with_rotation¹ return a Transform2D compliant with the Godot's 2D coordinate system (previously it was returning a transform with the basis axes swapped). This implicates it makes it compliant with all other things compliant with the Godot's 2D coordinate system, including e.g. PathFollow2D.

All usages of Curve2D::sample_baked_with_rotation within the codebase were changed appropriately (including more tests added).

The previous behavior was documented in #78362, so it can be considered a compatibility breaking change. I do consider this as a bugfix though (a buggy documented behavior is still a bug).
Sure there are probably some users using Curve2D::sample_baked_with_rotation and thus using some workarounds for the previous behavior. Their code would indeed break after merging this PR and thus they would need to get rid of the mentioned workarounds (shouldn't be a big deal).

So in case of merging this PR the change in Curve2D::sample_baked_with_rotation's behavior needs to be clearly stated out in the changelog (or some other relevant place).

---

@0xafbf Rebasing would be probably a good idea.

Footnotes

1. The changed Curve2D::_sample_posture is used internally only by Curve2D::sample_baked_with_rotation. ↩

[akien-mga]

Since this changes behavior and might surprise users, I'd prefer waiting for 4.3 so we have enough dev snapshots to get this tested and users to report any unexpected breakage in their projects.

[botero-dev]

Rebased :) also took the colon from the docs example as requested.

Yeah, too bad this was approved a bit late for this release, but its great that its approved nonetheless 💯

Thanks for your efforts

[akien-mga]

Thanks!