Fix SoftBody3D pinned points interaction

[Musicgun47]

There are a few issues related to modifying pinned points of the SoftBody3D node which this PR addresses.

The first was that the handles in the 3D viewport were not clickable, meaning selecting pinned points was not possible. This was caused by changes made in this PR which was aimed at reducing console spam from gizmo updates by checking mouse position had changed before committing changes. This has been reverted specifically for the mouse button up event to re-enable clicking of pinned point handles in the 3D viewport without the need for the mouse position to have changed. I have also added an update_gizmos() call to ensure changes to pinned points are displayed immediately as they were not getting updated until the mouse overlapped another handle. As far as I can tell this change hasn't brought back the console spam fixed in the mentioned PR.

Secondly, modifying pinned points directly via the inspector caused points to be dropped from the list. This was caused by the way values were set in the _set_property_pinned_points_indices() function. If a value was changed further up the list, the original value of the pinned point was removed from the list and the new value was added. However, if the modified value was not at the start of the list, this caused a copy to be created and a cascade of remove/add calls as the pointer *w still points to the original list. The result was that the last value of the list was never re-added as it would always match. This has been fixed by ensuring that entries are only removed from the list if the specified index is no longer present in the updated list, and ensuring that the updated index is added at the same point it was removed from (as _add_pinned_point() always added a new point to the end of the list resulting in unwanted reordering of the list). There is still an issue where if an index is incremented from a single value to a repeated value, the duplicate is removed, however I think that this is acceptable given duplicate vertices shouldn't be present in the pinned point array. With the previous fix, this functionality is not as necessary (given most users would set pinned points in the viewport), however it is important that it functions as intended if any improvements are to be made to the UI which may leverage calls to this function.

Fixes #93847

[Chaosus]

Thanks for your contribution. Please squash the commits as required by our pipeline (see docs.godotengine.org/en/latest/contributing/workflow/pr_workflow.html).

[Musicgun47]

Yeah, no worries. I wasn't sure if I would need to or not.

[AThousandShips]

Please fix the documentation by running --doctool on the compiled code, you will also need to fix compatibility, see here

[Musicgun47]

Okay, I think I've fixed everything, but let me know if I haven't. Reading the documentation and actually implementing it are two very different things, so happy for any pointers.

[AThousandShips]

Looks good code wise otherwise, but not my area so can't speak for functionality

[AThousandShips]

Approving on code fundamentals, still need a team review for the functionality as this isn't my area

[Musicgun47]

No worries, thanks for the help.

[akien-mga]

This should validate that the p_insert_at range is valid, i.e. >= -1 and < pinned_points.size(). Otherwise this will lead to hard-to-understand errors surfaced in CowData if users pass invalid values.

[Musicgun47]

I did think that would be the case, but I was unsure how CowData handled that so I was waiting for confirmation. It should be fixed now. Validation is done in the _add_pinned_point() method at line 753. Values out of bounds are simply added to the end of the list.

[akien-mga]

My idea was more to abort and notify the user that they used the API illegally, with:

ERR_FAIL_COND_MSG(p_insert_at < -1 || p_insert_at >= pinned_points.size(), "Invalid index for pin point insertion position.");

at the top of this method (which is the end-user facing one).

[Musicgun47]

Ah yes, that makes more sense; I'll do that instead. I keep forgetting that's a user facing method.

[akien-mga]

Thanks! And congrats for your first merged Godot contribution 🎉