Adding a CollisionShape2D node crashes Godot

[SamDevelopsCode]

Godot version

4.0.3

System information

Linux Mint 21.1 Cinnamon

Issue description

Adding a CollisionShape2D crashes Godot Engine.

Crash replicated across scenes and projects. Completely fresh projects crash as well.

Steps to reproduce

1. Create new scene or open existing.
2. Add new CollisionShape2D node. Crashes on step 2.

Minimal reproduction project

N/A

[ghsoares]

I had this exact same issue now when trying to add custom code to my engine build in 4.0 branch, does it also crash by simply adding the CollisionShape2D as child of any node?

I have a suspect that the Ref<Shape2D> shape is not initialized to an empty reference so it is initialized with random memory, looking like it is a valid reference, but when trying to get any property of the reference, the editor crashes. I'm testing right now to see if it's the culprit.

[SamDevelopsCode]

Good catch! It actually is adding the CollisionShape2D to any node or scene, not just Area2Ds that is causing the crash.

Updated bug report to reflect

[ghsoares]

Ok, I think I've found the issue, in collision_shape_2d_editor_plugin.cpp (source code to edit CollisionShape2D's) there is a variable called shape_type, which is used to check which type of shape the editor is currently editing, the problem is that it needs to be -1 to represent an invalid or null shape reference, but the function that is called when the shape is changed (_shape_changed), isn't setting this variable to -1 when the shape is null.

So the issue isn't with the CollisionShape2D node, but with the editor plugin that edits it. By making the edit function set shape_type to -1 when shape is null makes it work. See the source code:

godot/editor/plugins/collision_shape_2d_editor_plugin.cpp

Lines 377 to 415 in fdf66b3

	void CollisionShape2DEditor::_shape_changed() {
	canvas_item_editor->update_viewport();
	if (current_shape.is_valid()) {
	current_shape->disconnect(SceneStringNames::get_singleton()->changed, callable_mp(canvas_item_editor, &CanvasItemEditor::update_viewport));
	current_shape = Ref<Shape2D>();
	shape_type = -1;
	}
	if (!node) {
	return;
	}
	current_shape = node->get_shape();
	if (current_shape.is_valid()) {
	current_shape->connect(SceneStringNames::get_singleton()->changed, callable_mp(canvas_item_editor, &CanvasItemEditor::update_viewport));
	} else {
	return;
	}
	if (Object::cast_to<CapsuleShape2D>(*current_shape)) {
	shape_type = CAPSULE_SHAPE;
	} else if (Object::cast_to<CircleShape2D>(*current_shape)) {
	shape_type = CIRCLE_SHAPE;
	} else if (Object::cast_to<ConcavePolygonShape2D>(*current_shape)) {
	shape_type = CONCAVE_POLYGON_SHAPE;
	} else if (Object::cast_to<ConvexPolygonShape2D>(*current_shape)) {
	shape_type = CONVEX_POLYGON_SHAPE;
	} else if (Object::cast_to<WorldBoundaryShape2D>(*current_shape)) {
	shape_type = WORLD_BOUNDARY_SHAPE;
	} else if (Object::cast_to<SeparationRayShape2D>(*current_shape)) {
	shape_type = SEPARATION_RAY_SHAPE;
	} else if (Object::cast_to<RectangleShape2D>(*current_shape)) {
	shape_type = RECTANGLE_SHAPE;
	} else if (Object::cast_to<SegmentShape2D>(*current_shape)) {
	shape_type = SEGMENT_SHAPE;
	}
	}

It should have shape_type = -1; on line 395, like this:

	current_shape = node->get_shape();

	if (current_shape.is_valid()) {
		current_shape->connect(SceneStringNames::get_singleton()->changed, callable_mp(canvas_item_editor, &CanvasItemEditor::update_viewport));
	} else {
		shape_type = -1; // Here
		return;
	}

[adamscott]

It should have shape_type = -1; on line 395, like this:

	current_shape = node->get_shape();

	if (current_shape.is_valid()) {
		current_shape->connect(SceneStringNames::get_singleton()->changed, callable_mp(canvas_item_editor, &CanvasItemEditor::update_viewport));
	} else {
		shape_type = -1; // Here
		return;
	}

@KoBeWi Could you confirm?

[KoBeWi]

Duplicate of #76543 (workaround in comments)
The issue is already fixed on master.

It should have shape_type = -1; on line 395, like this:

This is not needed. The crash only happened, because the initial value set in constructor was 0 instead of -1.