Modifying Pinned Points of SoftBody3D is broken

[Musicgun47]

Tested versions

• Godot 4.3.beta2

System information

Godot v4.3.beta2 - Windows 10.0.22631 - Vulkan (Forward+) - integrated Intel(R) Iris(R) Xe Graphics (Intel Corporation; 31.0.101.5186) - 11th Gen Intel(R) Core(TM) i7-1165G7 @ 2.80GHz (8 Threads)

Issue description

I'm not sure if this issue is just related to the SoftBody3D pinned points array or PackedInt32Arrays in general as I haven't been able to find other uses of it quickly.

When modifying values and position of points in the Pinned Points array of a SoftBody3D, behaviour is unstable. Current bahaviour observed is as follows:

• Increasing value using the value arrows occasionally swaps the position in the array and is inconsistent in whether the values are actually changed or not (this can occur even if you miss click the arrows and just click on the item)
• Clicking an entry not at the end of the array will delete the last entry in the array.
• Trying to increment the last entry in the array deletes it instead
• Moving an entry to another position in the array just deletes the entry also.

I have also observed that occasionally entering values directly, by typing them in, will delete the last value too.

In addition to the issues mentioned above, setting pinned points via the 3D viewport doesn't work. Clicking on the handles does nothing and deselecting and reselecting the node doesn't change this meaning it's not just a visual glitch. However, clicking and dragging on a vertex will select it, but only the vertex where the drag began is selected and it must begin on the vertex.

Steps to reproduce

MRP contains a single scene with a SoftBody3D node. The mesh is a simple plane primitive. Issue can be observed by attempting to modify the pinned points of this soft body.

Minimal reproduction project (MRP)

test.zip

[RickyYCheng]

Pretty sure this works in 4.2.2 stable and not work in 4.3-beta2. Sometimes it is hard to click.

[Musicgun47]

I'm finally starting to get a bit more familiar with the code base, but still not entirely confident yet. Having said that, I may have isolated the issue:

godot/editor/plugins/node_3d_editor_plugin.cpp

Lines 1930 to 1933 in 82cedc8

	if (_edit.gizmo.is_valid()) {
	if (_edit.original_mouse_pos != _edit.mouse_pos) {
	_edit.gizmo->commit_handle(_edit.gizmo_handle, _edit.gizmo_handle_secondary, _edit.gizmo_initial_value, false);
	}

This is from the mouse button section when the button is released. It seems that changes to gizmo handles are only committed if the mouse has moved. This makes sense for things such as the transform gizmos but not so much for other types.

Is there any reason this check needs to be there? I'm not confident I know all the dependencies well enough to know if this would affect something else.

[Musicgun47]

Can confirm the above code is the cause of clicking handles not registering in the 3D viewport, and was introduced in this PR. Obviously, I'd like to keep these modifications if possible, so I'll do some further investigation to see if there's a way to keep these optimisations while fixing the issue (obviously there's more than just this one addition, but this seems to be the only one affecting SoftBody handles). I'm also going to do some investigation into the other issues mentioned above before opening a PR for this fix.

[Musicgun47]

So I've fixed the 3D viewport issue by just removing the check and it seems fine with no recurrence of the issue addressed in the mentioned PR. I also added an update_gizmos() call as handles weren't updating until the mouse moved off of the handle.

With regards to the issue of modifying pinned points via the inspector, I've narrowed down the issue to this bit of code in the _set_property_pinned_points_indices() function:

godot/scene/3d/soft_body_3d.cpp

Lines 197 to 211 in ff8a278

	pinned_points.resize(p_indices_size);
	PinnedPoint *w = pinned_points.ptrw();
	int point_index;
	for (int i = 0; i < p_indices_size; ++i) {
	point_index = p_indices.get(i);
	if (w[i].point_index != point_index) {
	if (-1 != w[i].point_index) {
	pin_point(w[i].point_index, false);
	}
	w[i].point_index = point_index;
	pin_point(w[i].point_index, true);
	}
	}
	return true;

Somewhere between line 199 and 210 the pinned_points vector gets resized. My assumption is that it's something in _pin_point(), either with how they're set or removed.

I ran a test with some data and this is the results:

Starting Data
pinned_points = [6, 4, 3, 2, 1] <- pinned point indices prior to update
p_indices = [5, 4, 3, 2, 1] <- expected indices after update

Output
w = [5, 4, 3, 2, 1] <- pointer used in update with correct indices
pinned_points = [5, 2, 4, 3] <- the actual pinned point indices after update