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

.tres file with a Dictionary/Array with a Resource is only saved the first time (or when the Dictionary/Array is expanded again) #55885

Closed
Maran23 opened this issue Dec 12, 2021 · 5 comments · Fixed by #62318

Comments

@Maran23
Copy link
Contributor

Maran23 commented Dec 12, 2021

Godot version

3.x, 4.0

System information

Windows 10

Issue description

First of all: This only happens when using a Resource inside a Dictionary or Array.
image

Editing and saving a .tres file will work only the first time. When editing and saving again, nothing will happen (can be checked by opening the .tres file in a text editor).

I debugged a bit and found out that it works due to the Dictionary/Array expand, which will set the Dictionary/Array to edited.
That's why it works the first time.
As soon as I at least once collapse/expand the Dictionary/Array again the save will work again for the first time.

Steps to reproduce

  1. Open the attached project
  2. Open the save.tres - Verify that there is 'ChangeMe' in the Dictionary
  3. Open the save.tres in a text editor (e.g. Notepad++) - There should be 'ChangeMe' as well
  4. Change the text (in Godot) to something else
  5. Save
  6. Everything should work and the change is saved Verify in the text editor)
  7. Change the text (in Godot) again
  8. Save
  9. You will see your change in the editor but in the text editor you won't see the change (I used Notepad++)

Minimal reproduction project

GodotSaveBug.zip

@Calinou Calinou added this to the 3.5 milestone Dec 12, 2021
@Maran23 Maran23 changed the title Before saved .tres file will be overwritten every game restart if it was opened at least once in the editor .tres file with a Dictionary with a Resource is only saved the first time (or when the TreeItem is collapsed/expanded) Jan 7, 2022
@Maran23
Copy link
Contributor Author

Maran23 commented Jan 7, 2022

I now had a look in the code. The issue is also present in 4.0.
The issue is a bit different than I originally thought, it's actually easier to reproduce and understand, therefore I changed the title and description.

Basically when a Resource in the Dictionary is edited, the Dictionary is not set to edited, only the Resource inside is (and thus the editor is marked with the '*' as well). This is not checked when saving (e.g. we need to check for edited children here, but I don't know yet how I can retrieve this information/Or the edited flag should bubble up to the parent (Dictionary)).

When the Dictionary is collapsed/expanded, the Dictionary is set to edited, therefore it can be saved again.

If someone more into the editor code wants to have a look I'm glad to help if needed. :)

@Maran23 Maran23 changed the title .tres file with a Dictionary with a Resource is only saved the first time (or when the TreeItem is collapsed/expanded) .tres file with a Dictionary with a Resource is only saved the first time (or when the Dictionary is collapsed/expanded) Jan 7, 2022
@AnidemDex
Copy link

AnidemDex commented Jan 11, 2022

I had a similar issue for editor plugins when using arrays and dictionaries. Things are saved only the first time.

Related issue AnidemDex/Godot-EventSystem#23

@Maran23
Copy link
Contributor Author

Maran23 commented Jan 14, 2022

I had a similar issue for editor plugins when using arrays and dictionaries. Things are saved only the first time.

Related issue AnidemDex/Godot-EventSystem#23

Yes, I can confirm it does not work for Arrays either.
I found out, that the 'problem' (or more like the fix) might be in this code:

editor_node.cpp
image

In _save_external_resources, _find_edited_resources is not looking for the edited state of the children in case of Dictionary or Array. If we can somehow get the Array/Dictionary here and check, if there children are resources and if so check if they are marked as edited, we can fix it here.

Another way would be, that a Resource inside an Array/Dictionary will not just mark himself as edited but also their 'parent' (The Array/Dictionary himself). But don't know, if this is possible either currently.

@AnidemDex
Copy link

AnidemDex commented Jan 14, 2022

Another way would be, that a Resource inside an Array/Dictionary will not just himself as edited but also their 'parent' (The Array/Dictionary himself). But don't know, if this is possible either currently.

@Maran23
Well, what I did was to connect subresource changed signal to parent resource:

https://github.com/AnidemDex/Godot-EventSystem/blob/2b24de5c79cae88ff65db7ee8b08483d6f4069ae/addons/event_system_plugin/resources/timeline_class/timeline_class.gd#L22-L35
image

But is a bit annoying since not every change in subresource means that the main resource changed (wich is annoying when you make custom inspectors that relies on that signal).

@Maran23 Maran23 changed the title .tres file with a Dictionary with a Resource is only saved the first time (or when the Dictionary is collapsed/expanded) .tres file with a Dictionary/Array with a Resource is only saved the first time (or when the Dictionary/Array is expanded again) Jan 23, 2022
@Maran23
Copy link
Contributor Author

Maran23 commented Jan 25, 2022

@AnidemDex I finally found a good fix for the problem and opened a PR. :)

reduz added a commit to reduz/godot that referenced this issue Jun 23, 2022
Redo edited subresource (and resource) saving in a much more simplified way.
I think this should work (unless I am missing something) and be faster than what is there.
It should also supersede godotengine#55885.

I am not 100% entirely convinced that this approach works, but I think it should so please test.
@akien-mga akien-mga modified the milestones: 3.5, 4.0 Jun 27, 2022
akien-mga pushed a commit to akien-mga/godot that referenced this issue Aug 24, 2022
Redo edited subresource (and resource) saving in a much more simplified way.
I think this should work (unless I am missing something) and be faster than what is there.
It should also supersede godotengine#55885.

I am not 100% entirely convinced that this approach works, but I think it should so please test.

(cherry picked from commit 9eb5f2a)
Riordan-DC pushed a commit to Riordan-DC/godot that referenced this issue Jan 24, 2023
Redo edited subresource (and resource) saving in a much more simplified way.
I think this should work (unless I am missing something) and be faster than what is there.
It should also supersede godotengine#55885.

I am not 100% entirely convinced that this approach works, but I think it should so please test.

(cherry picked from commit 9eb5f2a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants