Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.4] move_and_slide() and move_and_slide_with_snap() getting nullptr errors when colliding body is freed #54666

Closed
SeaFishelle opened this issue Nov 6, 2021 · 8 comments · Fixed by #54820

Comments

@SeaFishelle
Copy link

Godot version

3.4

System information

Windows 10, GLES3, Intel i5 4460, NVidia RTX 3060ti

Issue description

When an object that a KinematicBody node is colliding with is freed, the following errors are logged:

E 0:00:16.752   get: Condition "!id_map.has(p_rid.get_data())" is true. Returned: nullptr
E 0:00:16.756   body_get_direct_state: Condition "!body" is true. Returned: nullptr

This is a frustrating error to encounter when simply trying to change levels, as you must free the level scene before adding a new one, which causes this error. It's not a fatal error, thankfully, but logging it is a tad annoying when there will be a new body in the very next frame.

Steps to reproduce

  1. Call either move_and_slide() or move_and_slide_with_snap() in the _physics_process() step on a KinematicBody object using typical parameters.
  2. Place character on a floor or a wall.
  3. Call queue_free() on the floor or wall.

Minimal reproduction project

floor_free_example.zip

@akien-mga
Copy link
Member

CC @godotengine/physics

@ICatRegister
Copy link

This behavior should be investigated for KinematicBody2D as well. I have a similar situation, but only in the release build, it never appears in the editor or debug build. The current workaround is to use notification(NOTIFICATION_ENTER_TREE) in KinematicBody after releasing the "floor" object, as this reinitializes the variable on_floor_body = RID()

@Calinou
Copy link
Member

Calinou commented Nov 6, 2021

Judging by the error message, this may be fixed by #54650.

@ICatRegister
Copy link

Judging by the error message, this may be fixed by #54650.

It does

@RandomShaper
Copy link
Member

RandomShaper commented Nov 7, 2021

What I get with #43650 in this project is a crash.

That PR can't reasonably fix this. It may at much silence the first error line, but the second one is natural as long as the physics engine considers it's an error to call body_get_direct_state() about an invalid RID. A possibility may be that such functions don't compain (don't consider that is an error anymore) and just return nullptr in such cases, provided the physics engine itself is ready to get null pointers in those cases, which seems to be the case, at least in some code covered by this case:

PhysicsDirectBodyState *bs = PhysicsServer::get_singleton()->body_get_direct_state(on_floor_body);
if (bs) { // <-------------------- If we decide it's OK that the body has been released at some arbitrary point, this check does the biggest benefit of preventing a crash
	Transform gt = get_global_transform();
	Vector3 local_position = gt.origin - bs->get_transform().origin;
	current_floor_velocity = bs->get_velocity_at_local_position(local_position);
}

@Zylann
Copy link
Contributor

Zylann commented Nov 7, 2021

I reproduced the issue as well. It happens in Godot 3.4 stable and 3.4 rc3, but not in Godot 3.3.2.
My game has this happening because the player can delete blocks they are standing on.
I had a reproduction project too until I find this issue existed: FreeColliderStandingOn.zip

@akien-mga akien-mga added this to the 3.5 milestone Nov 7, 2021
@pouleyKetchoupp
Copy link
Contributor

As a first step and to avoid changing the general behavior for RIDs, I'm thinking of adding a method to the physics server to check if a given RID is valid (similar to what we do on free(RID). This way KinematicBody can just check each frame and reset the RID if it becomes invalid to avoid errors.

@pouleyKetchoupp
Copy link
Contributor

After checking the different cases and the code, I'm actually going with @RandomShaper's suggestion to make body_get_direct_state() silently return nullptr for freed bodies. It was already the case in 2D and it makes sense for the client code to decide whether it's an error.

I'm also going to apply the same thing in 2D and 3D for bodies removed from the scene, since it's currently causing similar errors (and a crash in Godot Physics 3D).

Later we can decide if there's a way to solve this kind of situation in a more generic way, but for now it's easy and seems to work well for this case in physics.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants