Fix PathFollow tests, Add PathFollow3D forward test

[raulsntos]

• Fixes the tests for PathFollow2D and PathFollow3D so they compile and add them to test_main.cpp because they weren't being used before.
• Adds a test for the the forward direction of PathFollow3D to prevent regressions like Path follow 3D returning different vectors in new beta build snapshot. #51342 in the future.

---

NOTE: I've marked this PR ready for review since the CI passes but I'd like to know what to do with the remove point test and if the custom is_equal_approx is acceptable, then I'll squash the commits.

[Paulb23]

For the SceneTree and RenderingSever, take a look at #50375.

[raulsntos]

The tests are failing because I get Vector3(99.93983, 0.050718, 0) instead of Vector3(100, 0, 0) probably because of float precision or the curve bake quality.

I tried to reduce the bake_interval and even though it gets closer to Vector3(100, 0, 0) it's still not enough. If I reduce it too much the tests crash probably because it runs out of memory.

[akien-mga]

You can use approx_equal if it's close enough, or compare manually to a big enough epsilon that takes precision into account.

[raulsntos]

I'm using Vector3::is_equal_approx which, unlike Math::is_equal_approx, does not have an overload with a tolerance parameter and therefore uses CMP_EPSILON.

The difference between Vector3(99.93983, 0.050718, 0) and Vector3(100, 0, 0) is not huge but it's not tiny either, so I'm not sure if modifying the tests' precision to pass would be acceptable. Maybe the difference is caused by an underlying issue and we'd be masking it, no?

[akien-mga]

The difference between Vector3(99.93983, 0.050718, 0) and Vector3(100, 0, 0) is not huge but it's not tiny either, so I'm not sure if modifying the tests' precision to pass would be acceptable. Maybe the difference is caused by an underlying issue and we'd be masking it, no?

Yes, that should be investigated.

[raulsntos]

Yes, that should be investigated.

After investigating I reached the conclusion that the _bake method is at fault here.

The points that are added to the curve are not baked so the cached points go from Vector2(100, 99.9516) to Vector2(95.00677, 100) and when trying to retrieve the point it interpolates between the two which never returns the original point Vector(100, 100).

I've modified the _bake methods for both Curve2D and Curve3D and now retrieving those corner points is more accurate, before the tests failed with an error of ±1 and now they still fail but in fewer asserts and with an error of ±0.0001 which I think is much more acceptable (but it's still not enough for the is_equal_approx method to return true.

[raulsntos]

This test won't pass because PathFollow currently does not listen to changes in the curve and AFAIK the PathFollow nodes never had this behavior.

Also, this test uses the unit offset but the PathFollow actually stores the offset. This test assumes that removing the middle point will update the transform to be at the interpolated point between the start and end points because that'd be the new 0.5. However, since PathFollow stored the offset it won't be equivalent to 0.5 anymore.

Same thing in the 3D version.

---

EDIT: I've commented these tests for now

[reduz]

Whats the status of this?

[raulsntos]

It's done but not sure what to do about the last 2 commits:

• Is the custom is_equal_approx method acceptable? Otherwise, with the default tolerance, the tests don't pass.
• What is the expected behavior of removing a point? I commented the test because that's not how it currently works. I can remove the test, change the test or change the behavior.

When those questions are resolved I have to squash the commits and rebase.

[YuriSizov]

This probably needs at least some changes after the recent PathFollow changes regarding forward direction.

[raulsntos]

• While rebasing I found a bug in PathFollow3D, I opened Clamp PathFollow3D progress when not looping #78280 to fix it. I will rebase this PR after the other one is merged.
• It seems the tests regarding forward direction fail unless I increase the tolerance even more, but at this point I feel like this might be a bug in PathFollow3D or Curve3D.

[Geometror]

Could you do a quick rebase?

[AThousandShips]

Suggested change

	// TEST_CASE("[SceneTree][PathFollow2D] Removal of a point in curve") {
	//TEST_CASE("[SceneTree][PathFollow2D] Removal of a point in curve") {

No space before commented out code, for all

I'd suggest using a block comment though and explaining why the test is inactive

[raulsntos]

I've commented these tests because of #51372 (comment). I don't think the behavior these tests intended to test was ever implemented in PathFollow{2D,3D} so we may want to remove these tests instead.

[Geometror]

With the following adjustments it passes. We need to do the same immediate update as for the other test cases, and after the curve length has changed, the absolute progress value is outdated (and doesn't represent the ratio of 0.5 anymore).

	path_follow_3d->set_progress_ratio(0.5);
	path_follow_3d->update_transform(true);
	CHECK(is_equal_approx(Vector3(100, 0, 0), path_follow_3d->get_transform().get_origin()));

	curve->remove_point(1);

	path_follow_3d->set_progress_ratio(0.5);
	path_follow_3d->update_transform(true);
	CHECK_MESSAGE(
			is_equal_approx(Vector3(50, 50, 0), path_follow_3d->get_transform().get_origin()),
			"Path follow's position should be updated after removing a point from the curve");

[raulsntos]

Rebased.

Also added path_follow_3d->update_transform(true) because #80233 made the transform update deferred.

It seems #80233 also broke the forward vector test and I'm not sure if the new behavior is correct. I also get a bunch of ERROR: The target vector and up vector can't be parallel to each other.

[Geometror]

Looks good to me!
We could refactor the checks to use doctest::Approx, but I'm working on a better way to integrate Godot's types with doctest, so I guess it's fine to merge this now.

[akien-mga]

Seems good to me too. Let's squash and merge, and fine tune things in follow up PRs.

[akien-mga]

Thanks!