Physics2DServer.set_active(false) doesn't work

[Scripptor]

Godot version:
v3.2.3.stable.mono.official
v3.2.2.stable

OS/device including version:
Windows 10

Issue description:
Physics2DServer.set_active(false) doesn't disable the physics server.
I was expecting 2D physics to be disabled.
I don't know if Physics2DServer.set_active(true) works, because I can't disable physics in the first place..

Steps to reproduce:
in gd script call Physics2DServer.set_active(false)
in C# call Physics2DServer.SetActive(false)

Minimal reproduction project:
Physics2DSetActive.zip

[AndyBarcia]

The kinematic body doesn't stop because you continue to call move_and_slide in the code. But if you try it with a plain rigidbody2D it does stop.
I don't know if this is the expected behavior though.

[Xrayez]

Yes, this only seems to affect rigid bodies. As a user, I'd expect all physics interactions to be disabled, though.

Perhaps the method should be renamed to set_stepping_active() or something (#16863), or document the actual behavior better. Look at the implementation:

godot/servers/physics_2d/physics_server_2d_sw.cpp

Lines 1249 to 1267 in 81fc37b

	void PhysicsServer2DSW::step(real_t p_step) {
	if (!active) {
	return;
	}
	_update_shapes();
	last_step = p_step;
	PhysicsDirectBodyState2DSW::singleton->step = p_step;
	island_count = 0;
	active_objects = 0;
	collision_pairs = 0;
	for (Set<const Space2DSW *>::Element *E = active_spaces.front(); E; E = E->next()) {
	stepper->step((Space2DSW *)E->get(), p_step, iterations);
	island_count += E->get()->get_island_count();
	active_objects += E->get()->get_active_objects();
	collision_pairs += E->get()->get_collision_pairs();
	}
	};

There's also a related proposal to allow manual physics stepping, see godotengine/godot-proposals#236, so perhaps the method could be eventually adapted to work something along the lines of set_manual_stepping().

[Scripptor]

It sure would be nice for it to stop all physics related things. Perhaps they can relabel the current behaviour to set_stepping_active(), but also eventually reimplement set_active() to disable all physics objects.. Essentially if it isn't active, no calls to _physics_process(delta) should be made

[Xrayez]

Yes, I'm not sure about this myself, technically this is a bug (to maintainers: if someone thinks the same, please assign the "bug" label).

But currently I think that's just badly documented feature.

Yet for this to be functional in your case, I see no Physics2DServer.is_active(), so you could switch off the physics processing in your kinematic bodies manually. So that's something which can be exposed, at least.

The potential fix is checking the active state directly in move_and_collide/slide() methods for kinematic bodies, so no movement shall be done.

In any case, need feedback from the @godotengine/physics team to determine whether it's a bug or not (or more users stumbling upon the same limitation as you do). 🙂

I'd personally treat this as a bug, but I'm afraid that the consensus may be different.

[pouleyKetchoupp]

I wouldn't consider this exactly a bug, but it would make sense to make things more consistent. Based on the previous suggestions this could be done easily:

• Physics2DServer.set_active(false): Disable stepping, and move functions do nothing but raise a warning when called.
• Physics2DServer.is_active(): Can be used to disable script code in _physics_process().
• Physics2DServer.set_stepping_active(false): Old behavior which just disables stepping, but move functions still work.

[Xrayez]

• Physics2DServer.is_active(): Can be used to disable script code in _physics_process().

I think this approach makes sense since _physics_process() can be used for game logic as well (or even visual effects that are tied to fixed updates), so we cannot simply disable _physics_process() callbacks from being called.

I also think that Physics2DServer.set_active() is mainly used with the pause system internally. If you pause the scene tree, _physics_process() will also be disabled (as well as physics stepping), so this is why it works as expected when paused. So that makes me wonder whether Physics2DServer.set_active() was actually internal and wasn't meant to be exposed (in it's current state).

But again, from the user standpoint (and if I were to stumble upon the same issue myself), this is unexpected, all physics interactions should be disabled if Physics2DServer.active == false, in my opinion.

That's why I suggested that move_and_collide/slide could handle this transparently, and it wouldn't impact game logic code since we don't have to disable _physics_process() either.

[pouleyKetchoupp]

That's why I suggested that move_and_collide/slide could handle this transparently, and it wouldn't impact game logic code since we don't have to disable _physics_process() either.

I would like to trigger a warning instead of just disabling the motion silently (even if just warning once). The reason is users can catch cases where an object is trying to use physics logic while the server is not active (with possibly some more stuff that shouldn't be run in physics process in this case). And it can also help with compatibility breakage if Physics2DServer.set_active(false) is modified from its current behavior.

[Xrayez]

If it's just for deprecation which the intention that the message will be removed in the future, sure. But I'm usually not in favor of redundant run-time warnings (even if they're printed once), because if you do use Physics2DServer.set_active(false), then users should probably know what they're doing already, otherwise this may create too much noise, so I'd just properly document those cases instead. These kind of warnings are fine for editor stuff.

See other cases such as #24222, or even #43783.

Run-time warnings can be added, but since there's no easy way to disable them, those warnings get in the way of more important warnings such as GDScript warnings.

Note that I'm talking purely about Godot 4.0 changes, not 3.2 (current behavior should remain the same as in 3.2 I think, Physics2DServer.is_active() can be exposed in 3.2, but that's all).

[pouleyKetchoupp]

The warning message is not intended to add noise, but to tell users what to fix. The point would be that it's not allowed to call the move functions while the server is inactive, and users would have to use Physics2DServer.is_active() to avoid doing these calls.

So the warning would be something like:
Calling move_and_slide while the physics server is inactive is not allowed. You need to check whether the physics server is active before calling any move function, or use set_stepping_active(false) to disable stepping while keeping move functions active.

[Xrayez]

That's the point I'm trying to convey: why should a user call Physics2DServer.is_active() prior to making those calls if they won't be executed during _physics_process() anyway? It would be basically similar to _integrate_forces() processing for rigid bodies, where both game and physics logic can be currently processed.

To comment on your previous statements:

And it can also help with compatibility breakage if Physics2DServer.set_active(false) is modified from its current behavior.

I understand, but I can't find a concrete use case where this would become problematic.

The reason is users can catch cases where an object is trying to use physics logic while the server is not active (with possibly some more stuff that shouldn't be run in physics process in this case).

I'd go for complete compatibility breakage for 4.0 if those changes do improve the experience, I recall many other people preferred the same development approach for 4.0.

That's all my feedback for this issue, I don't want to potentially speculate on the problem which might not be a big deal at the end of the day. 🙂

[Scripptor]

I don't think checking if the server is active in _physics_process(delta) makes sense.. A user would assume that the Physics Server is active if the physics call is being made.

So rename the function from set_active(state) to set_stepping(state), and also change is_active() to is_stepping()
I'd like set_active(state) to remain though, and for it to completely disable physics processing, and for is_active() to let the user know if the PhysicsServer is active.

[berarma]

I don't think checking if the server is active in _physics_process(delta) makes sense.. A user would assume that the Physics Server is active if the physics call is being made.

So rename the function from set_active(state) to set_stepping(state), and also change is_active() to is_stepping() I'd like set_active(state) to remain though, and for it to completely disable physics processing, and for is_active() to let the user know if the PhysicsServer is active.

What should we do if we want to implement our own physics?

This callback is used for more than PhysicsServer related stuff. There might be other physics code running.

I think it makes sense having a physics process callback whether the built-in PhysicsServer is active or not.