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

Keep property values when extending script #43081

Merged
merged 1 commit into from
Jul 29, 2022

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Oct 25, 2020

Closes #37480
EDIT: Closes godotengine/godot-proposals#4415

This can also trigger when replacing a script for one node. Making it work only with script extending would make the code more complicated, because the editor doesn't really differentiate between extending and attaching. I don't think this is a big issue.

I added 2 methods in EditorNode:
store_object_properties() and apply_object_properties(). The first one gets properties from the script instance and stores it in EditorNode, the second one applies it to the new instance, to the properties that have matching name and type. The intended usage is that you call these methods as part of a UndoRedo action that changes script and the exported values are carried to the new script.

The values will be preserved when:

  • extending script
  • making script unique
  • attaching a new script
  • if you know any other action that can potentially clear exported variables, please comment

@KoBeWi KoBeWi added enhancement topic:editor cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Oct 25, 2020
@KoBeWi KoBeWi added this to the 4.0 milestone Oct 25, 2020
@KoBeWi KoBeWi force-pushed the property_keeper branch from a7ef76b to 3250ddb Compare June 26, 2022 16:54
@KoBeWi KoBeWi requested a review from a team as a code owner June 26, 2022 16:54
@KoBeWi KoBeWi force-pushed the property_keeper branch from 3250ddb to d761a27 Compare June 26, 2022 18:59
@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 26, 2022

I rebased and improved the PR.

Aside from extending script, the properties will now be kept when making the script unique or attaching a new one (regardless if it's the same script or a completely different one; properties with the same name will keep values).

@akien-mga akien-mga requested a review from a team June 28, 2022 12:22
@akien-mga
Copy link
Member

@reduz suggests that @vnen should review this. Ideally this should/could be fixed in the script itself so that this workaround is not necessary in the editor.

@KoBeWi
Copy link
Member Author

KoBeWi commented Jun 28, 2022

Not sure how can this be fixed in script if the script is replaced by a different one. This is editor issue, because the script is removed and a new one is added and only editor can keep track of the former variables.

editor/editor_node.cpp Outdated Show resolved Hide resolved
editor/editor_node.cpp Outdated Show resolved Hide resolved
@@ -5808,6 +5831,9 @@ void EditorNode::_bind_methods() {
ClassDB::bind_method("_close_messages", &EditorNode::_close_messages);
ClassDB::bind_method("_show_messages", &EditorNode::_show_messages);

ClassDB::bind_method("store_object_properties", &EditorNode::store_object_properties);
Copy link
Member

Choose a reason for hiding this comment

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

IMO This function should only be called if you have a script, so the intention is better. Otherwise you have to go to the function and see that it does nothing if it has a script.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nah, the function is supposed to store available script properties. If there is no script, it just has no properties to store.

It makes the code much simpler. Otherwise I'd have to wrap each call and make it less readable.

@KoBeWi KoBeWi force-pushed the property_keeper branch from d761a27 to 4c75fee Compare July 29, 2022 14:54
@KoBeWi KoBeWi force-pushed the property_keeper branch from 4c75fee to 44cf3c2 Compare July 29, 2022 14:57
@akien-mga akien-mga merged commit 74d92bf into godotengine:master Jul 29, 2022
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the property_keeper branch July 29, 2022 17:30
@timothyqiu
Copy link
Member

timothyqiu commented Dec 17, 2022

Cherry-picked for 3.6.

@timothyqiu timothyqiu removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Dec 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants