Ensure inherited default parameter is not redefined

[madmiraal]

As identified by lgtm, #48908 changed the default values for the inherited functions body_test_motion() and body_test_ray_separation()'s p_margin values in physics_server_2d_sw.h to be different from the parent default values in physics_server_2d.h:

godot/servers/physics_server_2d.h

Line 503 in f178e7a

virtual bool body_test_motion(RID p_body, const Transform2D &p_from, const Vector2 &p_motion, bool p_infinite_inertia, real_t p_margin = 0.001, MotionResult *r_result = nullptr, bool p_exclude_raycast_shapes = true) = 0;

godot/servers/physics_server_2d.h

Line 517 in f178e7a

virtual int body_test_ray_separation(RID p_body, const Transform2D &p_transform, bool p_infinite_inertia, Vector2 &r_recover_motion, SeparationResult *r_results, int p_result_max, real_t p_margin = 0.001) = 0;

"Inherited default parameters should not be redefined because this obscures the behavior of the code. Default values are statically bound and when they are redefined in dynamically bound function calls this reduces readability of the code, increasing the risk of introducing defects."[1]

This PR reverts that change so the inherited default values are once again the same as the parent's default values.

Note: The confusion comes from the fact that this function is called by _body_test_motion() (with an underscore) that passes a defined (non-default) value into body_test_motion():

godot/servers/physics_server_2d.cpp

Lines 514 to 520 in f178e7a

	bool PhysicsServer2D::_body_test_motion(RID p_body, const Transform2D &p_from, const Vector2 &p_motion, bool p_infinite_inertia, real_t p_margin, const Ref<PhysicsTestMotionResult2D> &p_result) {
	MotionResult *r = nullptr;
	if (p_result.is_valid()) {
	r = p_result->get_result_ptr();
	}
	return body_test_motion(p_body, p_from, p_motion, p_infinite_inertia, p_margin, r);
	}

_body_test_motion() p_margin's default value is 0.08:

godot/servers/physics_server_2d.h

Line 233 in f178e7a

virtual bool _body_test_motion(RID p_body, const Transform2D &p_from, const Vector2 &p_motion, bool p_infinite_inertia, real_t p_margin = 0.08, const Ref<PhysicsTestMotionResult2D> &p_result = Ref<PhysicsTestMotionResult2D>());

And it's _body_test_motion() that is exposed as body_test_motion():

godot/servers/physics_server_2d.cpp

Line 646 in f178e7a

ClassDB::bind_method(D_METHOD("body_test_motion", "body", "from", "motion", "infinite_inertia", "margin", "result"), &PhysicsServer2D::_body_test_motion, DEFVAL(0.08), DEFVAL(Variant()));

I've have previously suggested fixing this, so the values are all 0.001 by default as is the case in 3D physics: #45361 (and it's 3.x version: #45362). Perhaps we should reopen those PRs.

[pouleyKetchoupp]

Thanks for catching the discrepancy. It would make more sense though to change PhysicsServer2D to use 0.08 as well, since it's the default value that is used for 2D.

Concerning #45361:
We've already discussed #45361 in a PR meeting, and the conclusion was that 0.001 is way too small to be the default value for 2D (it's 1/1000th of a pixel). Again as discussed, a welcome change would be to make test_motion_min_contact_depth adapt to the margin instead of being a fixed value, so users can lower the margin if needed without breaking the motion algorithm.

[madmiraal]

This is off-topic, but for the record:

0.001 is way too small to be the default value for 2D

I've not seen any reasoning or data to support this assertion. Remember, a value of 0.08 is big enough to cause the visible jitter seen in #45259. 0.001 is the value currently used in 3D, and it's the value 2D was originally designed with. It's also only a default value, so a user can increase it if required.

a welcome change would be to make test_motion_min_contact_depth adapt to the margin instead of being a fixed value, so users can lower the margin if needed without breaking the motion algorithm.

#45361 also addresses the problem with reducing the margin breaking the motion algorithm seen in #36432. Note: test_motion_min_contact_depth causes #16414 and should be removed entirely; as done as part of #43616.

Ultimately, the real problem is Godot physics' use of the safe margin. As described here, currently, to prevent collisions caused by numerical imprecision, Godot physics uses the safe margin to push the body away from other shapes before attempting the motion. The problem arises when this push is not cancelled by the subsequent motion. Note that even a value of 0.001 in 3D is sufficient to cause noticeable jitter as shown here (Figure 3).

Personally, I think, instead of pushing the body away using a safe margin, Godot should have a tolerance during the motion; and a correction after the motion to address the penetration caused by numerical imprecision, similar to the way done in Bullet physics.

[pouleyKetchoupp]

@madmiraal The process behind body motion won't be entirely changed based on your personal preferences. I can be convinced otherwise, but it would be based on fixing issues that can't be fixed with the existing concept. This hasn't happened yet.

If test_motion_min_contact_depth can be removed instead of being changed to be adaptive, and it doesn't break anything, it would be fine for me.

I will review #43616 at some point, but it will take some time to check and test. It's difficult to do so because that PR mixes changes in low-level physics algorithms, high-level motion process and a lot of code refactoring that makes it hard to identify what parts of the code are effective changes.

Now on the current topic:
A value of 0.001 in 2D seems meaningless in most cases, because it's 1/1000 pixel, and it would be effective only for extremely low resolution games. Even in the pixelated game example from #45259, 0.01 was enough to make a difference in the first place, and the regression ended up being fixed with #46148 with the default value of 0.08 anyway.

At this point, the physics API just needs to be harmonized with the same default margin for 2D (0.08) in the physics server and the nodes, which is all I'm asking.

[pouleyKetchoupp]

Superseded by #53280 (now the default margin is defined in PhysicsTestMotionParameters).