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

PackedInt32Array handling in Editor is broken, SoftBody almost unusable. #97155

Open
LastPhoen1x opened this issue Sep 18, 2024 · 6 comments
Open

Comments

@LastPhoen1x
Copy link

Tested versions

Reproducible: 4.3 mono stable, 4.4.dev2 mono
Working: 4.1 mono

System information

Windows 7 - Godot 4.3 stable mono

Issue description

Editing PackedInt32Array from the editor (SoftBody3D pinned points in this example) is fundamentally broken, changing any value other than the last removes the last entry. Clicking on the pinned points does nothing. This makes SoftBody barely usable.

Am I the only one having those issues or this have gone unnoticed somehow?

Steps to reproduce

  1. Make a SoftBody3D with 3 pinned points.
  2. Change the first or second point's index.
    OR
  3. Click on any point in the 3D view to add it to the array - does not work.

Minimal reproduction project (MRP)

softbodyisbroken.zip

@imp0s5ible
Copy link

I can reproduce this in v4.3.stable.official on Win11 as well. It doesn't look like the issue exists for PackedInt32Arrays exported from scripts, so this might be specific to SoftBody3D somehow.

@BurryBurst
Copy link

Did some testing on the SoftBody3D. With 2 elements in the array it swaps the first and last entry. Maybe something to do with how the array gets copied when changes occur

@LastPhoen1x
Copy link
Author

So it is not just me or Win 7.

The same behavior of last entry dissapearing happens to AnimationPlayer array of AnimationLibraries, that made me believe this is more global in scope.

@imp0s5ible
Copy link

I did a little digging and SoftBody3D::_set_property_pinned_points_indices(const Array &p_indices) (I assume this is called when the pinned points are changed in the editor) does quite a bit of removing and filtering, so this is definitely a good place to start debugging.

@BurryBurst
Copy link

So I just build the current master 4.4 and clicking works. Editing the array is still a bit weird the array swapping doesnt happen but sometimes entries still disappear

@BurryBurst
Copy link

Okay so I found part of why this happens:
in SoftBody3D::_set_property_pinned_points_indices(const Array &p_indices)
image
Line 206: It tries to find the old point_index (8) but p_indices contains the already updated indices (I incremented it from 8 to 9)
As it doesn't find it it calls pin_point with "pin" set to false, this deletes the pinned_point from the array so the pinned_points.size is now 8 instead of 9.
Next in Line 212 it now calls pin_point with pin_insert_at set to 8 (4th parameter here i) which will immediaty fail as the p_insert_at is >= to pinned_points.size (Which is 8 as the element was removed before).
image
(Ignore the if statement did that for debugging)
So the element gets removed and never gets added back.
I don't know why it checks if the index was already in there though just to remove and add it back anyways?

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

No branches or pull requests

3 participants