Return RID instead of Object id in area-body_shape_entered-exited signals.

[madmiraal]

Currently, the Area and RigidBody area_shape_entered, area_shape_exited, body_shape_entered and body_shape_exited signals incorrectly (try to) return the entering or exiting Object's instance id instead of its RID. This PR ensures the signals correctly return the RID.

Fixes #12215

Note: Although, one could argue that this is a compat breaking change, it is highly unlikely that anyone is using the Object's instance id, because, as far as I know, it is generally only used to obtain the Object, which is passed as the second parameter. Should the Object's instance id be needed, it is easily obtained using area.get_instance_id() or body.get_instance_id().

[ghost]

You're talking about the first parameter right? The body / area int id.

I think I was wondering about that last I used these signals. It seemed redundant and thought maybe it was included by accident, since the API generally doesn't work with RIDs unless you're working with the servers, and when needed you can get that information from the instance itself.

[madmiraal]

@avencherus Yes, this is about updating the first parameter. The RID is needed when, as described in #12215, there is no object, and, as you suggest, you're working directly with the server.

[ghost]

@madmiraal Yeah, that makes sense. The more I look at it, the more I think that the RID was probably the original intention.

At least from my point of view in development, I agree with your compatibility statement. If working with instances, I can't imagine myself or many others who would ignore the body reference in favor of doing an extra step like instance_from_id(area_id). In that case it is redundant parameter, and in the case of using RIDs, it's not possible to use it.

But more importantly, it would be fixing a hole for performance use cases like that bullet storm example.

[madmiraal]

The current documentation also says that it is the RID; so that's the expectation and the issue this PR fixes e.g. for RigidBody:

body_shape_entered ( int body_id, Node body, int body_shape, int local_shape )

Emitted when a body enters into contact with this one. Requires contact_monitor to be set to true and contacts_reported to be set high enough to detect all the collisions.

This signal not only receives the body that collided with this one, but also its RID (body_id), the shape index from the colliding body (body_shape), and the shape index from this body (local_shape) the other body collided with.

[madmiraal]

Rebased following merge of #42713 and #43412.

[madmiraal]

Rebased following merge of #44630.

[Xrayez]

Note: Although, one could argue that this is a compat breaking change, it is highly unlikely that anyone is using the Object's instance id, because, as far as I know, it is generally only used to obtain the Object, which is passed as the second parameter. Should the Object's instance id be needed, it is easily obtained using area.get_instance_id() or body.get_instance_id().

Is it possible that the pointer to collision object gets invalidated in the middle of signal emission? In that case, having the instance id would be preferable to use over Object reference. I think that's unlikely to happen and not sure if possible, but worth to ask!

[madmiraal]

Is it possible that the pointer to collision object gets invalidated in the middle of signal emission? In that case, having the instance id would be preferable to use over Object reference. I think that's unlikely to happen and not sure if possible, but worth to ask!

I don't think it's possible, and even if it were the Object's instance id would be invalid too. Besides, the second parameter (the Area or Body) is not being changed by this PR only the first parameter: the ID (from Object's instance id to RID).

[madmiraal]

Rebased following merge of #46364.

[Xrayez]

This PR is going to help in solving the underlying use case in godotengine/godot-proposals#2543.

[pouleyKetchoupp]

There should be no compatibility problem, given the documentation already states ids are supposed to be RIDs from the physics server:

godot/doc/classes/Area2D.xml

Line 120 in cc57bb9

[code]area_id[/code] the [RID] of the other Area2D's [CollisionObject2D] used by the [PhysicsServer2D].

[akien-mga]

Thanks!