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

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

Open
nan0m opened this issue Oct 3, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@nan0m
Copy link

nan0m commented Oct 3, 2024

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
Copy link
Owner

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
Copy link
Author

nan0m commented Oct 3, 2024

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

@cloudofoz
Copy link
Owner

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 cloudofoz added the enhancement New feature or request label Oct 4, 2024
@cloudofoz cloudofoz changed the title Selecting a deformable node shows all nodes in scene Selecting a deformable node shows all nodes in scene (Refactor Deformer Array Type) Oct 11, 2024
@cloudofoz
Copy link
Owner

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!

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

No branches or pull requests

2 participants