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

Curve Editor gets re-constructed after every operation. #74865

Closed
MewPurPur opened this issue Mar 13, 2023 · 2 comments · Fixed by #74927
Closed

Curve Editor gets re-constructed after every operation. #74865

MewPurPur opened this issue Mar 13, 2023 · 2 comments · Fixed by #74927

Comments

@MewPurPur
Copy link
Contributor

MewPurPur commented Mar 13, 2023

Godot version

4.0

System information

Ubuntu 22.04.1 LTS

Issue description

The CurveEditor constructor is called every time you do any operation. This is for example why handles get deselected after you drag them.

Upon investigation, it appears that this is because Curve's methods add_point() and remove_point() call notify_property_list_changed(). This was added as a fix in #61628, which fixed a regression most likely from #58023.

This bug isn't a big deal at the moment, but I'm currently making a PR to rework the curve editor and this bug is blocking it completely.

Steps to reproduce

Create a Curve.
Add a point.
Move the point.
Watch it get deselected.

Minimal reproduction project

N/A takes 10sec to set up, moreover the best way to see this is by playing with the source code.

@MewPurPur
Copy link
Contributor Author

MewPurPur commented Mar 13, 2023

I am not really sure why we're doing those notify_property_list_changed() calls in Curve. Other similar resources like Gradient don't seem to have them.

Edit: Maybe because the points are dumped in an array as a raw data, while gradient points have separate sections for offsets and colors?

@Rindbee
Copy link
Contributor

Rindbee commented Mar 15, 2023

Because the structure of the property tree (see Curve2D::_get_property_list()) has changed, it needs to use notify_property_list_changed() to finally call EditorInspector::update_tree(). This is true for all arrays added using the ADD_ARRAY* macros.

But for the case where the size of the array has not changed, there is no need to call notify_property_list_changed(). The size of the array can be checked to avoid calling notify_property_list_changed().

Here it is caused by an unnecessary notify_property_list_changed() call in Curve::set_data() when just changing the element value.

notify_property_list_changed();

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

Successfully merging a pull request may close this issue.

4 participants