cloudofoz / godot-deformablemesh

Addon to deform 3D meshes using customizable deformers at run-time.
MIT License
250 stars 9 forks source link

Selecting a deformable node shows all nodes in scene (Refactor Deformer Array Type) #4

Open nan0m opened 2 weeks ago

nan0m commented 2 weeks ago

Not precisely an issue but maybe a QoL improvement. Currently the export variable for deformers is typed as a Array[NodePath] this showing all nodes in the scene tree. If this were Array[Deformer] or similar, it would only show the relevant nodes in the selection popup.

cloudofoz commented 2 weeks ago

This makes me think that you might be using an older version of my plugin.

The exported variable is already typed as Array[Deformer] (var dm_deformers: Array[Deformer]). You can check the relevant line here.

Let me know if this was the case.

nan0m commented 2 weeks ago

Hi. It's the @export var a few lines up. It's about this one here

cloudofoz commented 2 weeks ago

Thank you for highlighting the correct line.

Now that you mention it, I recall there was a specific reason I chose to use NodePath during my tests, though I can't quite remember why at the moment.

That said, this add-on was originally created for an earlier version of Godot, so re-testing it sounds like a good idea!

cloudofoz commented 1 week ago

In the first iteration of my add-on, I was "forced" to use NodePath for a specific reason during testing, though I can’t recall the exact details.

Thanks to @nan0m's suggestion and feedback, I’ve had the chance to re-evaluate this decision. After reviewing the current state of the add-on, it now makes sense to modify the @onready @export var deformers: Array[NodePath] to @onready @export var deformers: Array[Deformer].

I’ve refactored the related functions, and from my tests, everything seems to work fine with the new type.

That said, there is a known issue that the deformers array will be cleared the first time after updating, so you'll need to manually re-set the deformers for existing projects.

I’m looking for feedback on whether this is a significant issue for users or if further adjustments are needed.

I've created a branch with this modification.

It would be great if anyone could test the changes and confirm that nothing breaks. Your feedback would be much appreciated!