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

Fix current field not updating after pasting value #78603

Closed

Conversation

Rindbee
Copy link
Contributor

@Rindbee Rindbee commented Jun 23, 2023

Reverts #76711 and supersedes #76711.
Fixes #75124, fixes #78578.

This commit only updates the field that is currently affected.

@Rindbee Rindbee changed the title Fix paste value not update Fix current field not updating after pasting value Jun 23, 2023
akien-mga
akien-mga previously approved these changes Jun 23, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safer for sure.

@akien-mga
Copy link
Member

CC @ajreckof

@akien-mga
Copy link
Member

Reverts #76711 and supersedes #76711.

Did you mean to link #78591 as superseded?

@Rindbee
Copy link
Contributor Author

Rindbee commented Jun 23, 2023

Did you mean to link #78591 as superseded?

I'm not sure, #78590 doesn't seem to be a regression of #76711.

@ajreckof
Copy link
Member

ajreckof commented Jun 23, 2023

This will not supersede #78591 As this does not fix #78590 but #78591 will in fact supersede this.

I'm not sure about the way to update the property here as it will not fix the inconsistency with other places where calling emit changed with p_changing = false will trigger an property update

You could on the other hand call update property from the emit_changed callback but only the property needed. This would need to store esitor properties but shouldn't be too hard.

Edit : I just realised this PR does not fix the first issue (neither does my suggestion) as it won't work when type has changed because each editor property can only handle one type and therefore it can't show the new value since it has an invalid type. This might even create errors or crash when updating the property will require the correct type

@akien-mga akien-mga dismissed their stale review June 23, 2023 10:21

Doesn't seem as safe as I initially thought, as it actually will update in more cases when pasting.

@Rindbee
Copy link
Contributor Author

Rindbee commented Jun 23, 2023

For some cases in Array, updating the parent's EditorProperty seems to be a must. It may be necessary to distinguish between typing and pasting.

@ajreckof
Copy link
Member

I think the proper solution to avoid too much update is to rework update_property to be a bit more intelligent and only destroy/recreate that do need because they were either added removed or type changed. But I feel like this is something that can wait for 4.2

@Rindbee
Copy link
Contributor Author

Rindbee commented Jun 23, 2023

It is impossible to solve the case where a value of a different type is copied to an element of Array to change the element type.

@Rindbee Rindbee closed this Jun 23, 2023
@Rindbee Rindbee deleted the fix-paste-value-not-update branch August 17, 2023 14:09
@AThousandShips AThousandShips removed this from the 4.1 milestone Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants