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

Add a 'tracker' string to .tscn files (for preventing data loss in tool scenes) #2365

Closed
Error7Studios opened this issue Feb 27, 2021 · 10 comments

Comments

@Error7Studios
Copy link

Error7Studios commented Feb 27, 2021

Describe the project you are working on

Godot RPG Framework

Describe the problem or limitation you are having in your project

The framework I'm making uses tool scripts everywhere, and tool scripts reset their data on reload.
See #1012, #1659

This only happens under specific circumstances, as described in this comment.

My current workaround for this is to have a save_data() function which writes all of a node's property values to a file, and a load_data() function which updates all the properties of the node using that file data.
This saves/updates all variables in get_script_property_list.

(See this example video where I show tool node property saving/loading in action.)

This works, but there are a few problems with it.

  1. You must call save_data() on all of the tool nodes before they reload, or any changes will be lost.
  2. You must call load_data() for each of these nodes at the top of their _ready() func, so the in-game values match the in-editor values.
  3. You must call load_data() using an export var whenever the scene is re-opened in the editor to restore the saved state.
  4. You can only save one node's data (or their children's data as an array) per text file, because:
    4a. The node's instance id changes on every reload, so it can't be used as a dictionary key.
    4b. Using the node's name or path as a dictionary key won't work, because they can change between save and reload.
    (Also, multiple nodes can have the same name.)
  5. A .tscn file doesn't assign any unique identifiers to child nodes that could be used to track them between save and reload if their name or path is changed.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I propose that whenever a node is created or instanced into a scene, the .tscn file for that scene's root node assign each of these children a unique identifier, as mentioned in point 5 above.

This identifier should be a long string of randomly generated characters.
It should be long enough that it would be virtually impossible to generate the same one twice.

The string can then be added into the .tscn file, under each child node's section like this:
[tracker= "y4r#t7jXJb5chf^Q#Lj2Pi$BN6QeoE3opZQMEBXV!8Etr&HmdAR77DMNKs$EQn23Xp1&Uh@8Hp8SC3j6^UCXt3^dUt&1E3pE3h"]

By doing so, it would be possible to have a single save file containing a dictionary of all nodes properties in the scene, using the tracker strings as keys.

If the save file could be updated every time the current scene is saved, that would be ideal, because the user wouldn't have to remember to call save_data() manually.
Also, when a tool script runs its _ready() func, it should automatically call load_data().

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

(See above)

If this enhancement will not be used often, can it be worked around with a few lines of script?

No. Also it would require remembering to put function calls in every tool script, and remembering to manually call save_data() every time a change is made.

Is there a reason why this should be core and not an add-on in the asset library?

This would have to be built in, because it would require adding a new tracker section in every .tscn file.
It would also require automatically calling save_data() whenever a scene is saved, and load_data() in the _ready() func for tool scripts.

Minimal Example Project

Tool Save Load Example.zip

Steps:

  1. Open the Main scene, and click on the ToolScene child node.
  2. Change the ToolScene export var Texture Index to either Goblin or Human.
  3. Run the scene. The Label and TextureRect properties will not match the editor.
  4. Now go to the _ready() func in ToolScene.gd, and uncomment the line, G.load_property_dict_file(self, SAVE_FILE_PATH)
  5. Now when you run the scene, the editor properties and in-game properties will match.
@YuriSizov
Copy link
Contributor

The linked issues are related to the fact that what is currently running gets ungracefully disconnected from the scripts. The problem is not so much data loss, after all you kind of want it to happen, because scripts and scenes may have changed dramatically and may not map to the previous instance. The problem is mostly about how it happens ungracefully.

So I am not sure how this proposal would address that situation.

@Error7Studios
Copy link
Author

The problem is not so much data loss, after all you kind of want it to happen

The way I use tool scripts is to be able to set up a scene with characters, a GUI layout, etc, and then I want the running scene to be exactly the same as how I set it in the editor. This is what most users would expect to happen.

But by default, all the changes you made just get reverted when you run the scene.
See this example video where I show tool node property saving/loading in action.

In the video, I save the data by toggling an export bool, and I load the data with a function call in _ready().
I'm just proposing that we automate this. (Or allow the user to decide if they want it to be automated).

As the video shows, this works perfectly for tool scripts, but I can't comment on plugins, since I don't use any.

@YuriSizov
Copy link
Contributor

But by default, all the changes you made just get reverted when you run the scene.

Hmm, I usually have to work with the opposite, because tool scripts actually change the scene and so on _ready I cannot assume that the scene is as pristine as I may hope it is. Are texture in your video exported properties so that they can actually be serialized and saved?

@Error7Studios
Copy link
Author

Are texture in your video exported properties so that they can actually be serialized and saved?

No, the KinematicBody2D is an instanced scene with a TextureRect child.
I'm just setting the texture property on that child TextureRect.
The KinematicBody2D has a tool script.

@Error7Studios
Copy link
Author

Please see this minimal example project: Tool Save Load Example.zip

  1. Open the Main scene, and click on the ToolScene child node.
  2. Change the ToolScene export var Texture Index to either Goblin or Human.
  3. Run the scene. The Label and TextureRect properties will not match the editor.
  4. Now go to the _ready() func in ToolScene.gd, and uncomment the line, G.load_property_dict_file(self, SAVE_FILE_PATH)
  5. Now when you run the scene, the editor properties and in-game properties will match.

@fire
Copy link
Member

fire commented Jan 3, 2022

Is this resolved by godotengine/godot#50676 ?

@KoBeWi
Copy link
Member

KoBeWi commented Jun 5, 2022

@Error7Studios Since 4.0 alpha 9, tool scripts should no longer reset their variables. Can you check if your problem is maybe resolved?

@Error7Studios
Copy link
Author

Error7Studios commented Jun 6, 2022

@fire @KoBeWi
In 4.0 alpha 9:

--- solved ---
export properties are now retained when launching the scene and closing/re-opening the editor.
(Awesome! Thank you!)

--- Edit ---
Reported the unrelated backwards text issue.
is_inside_tree() actually does behave the same in 3.4 and 4.0a9.
The issue was just with some manual calls to the setters in my code.

Example project:
4.0 alpha9 Tool BTN Test.zip

@Zireael07
Copy link

@Error7Studios seems like unrelated issues, make a bug report instead

@Error7Studios
Copy link
Author

Issue is resolved in 4.0 Alpha 9. Thanks!

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

No branches or pull requests

5 participants