Saving embedded resource to overwrite previously used resource file causes error "possible cyclic resource inclusion" and fails to update resource reference, keeping embedded data

[hsandt]

Godot version

v4.0.stable.official [92bee43ad]

System information

Linux Ubuntu 22.04 with Unity desktop

Issue description

When saving an embedded resource in the node inspector as a resource file, and selecting an existing resource file that was previously set as target to overwrite it, the target file is overwritten, but you get an error:

Another resource is loaded from path 'res://target_resource.tres' (possible cyclic resource inclusion).

and the reference will not be updated to the path to target file, so the node will still uses the embedded resource (so the target file is a mere unused copy of it).

Note that the error message is the same as in other issues like https://github.com/godotengine/godot/issues/70273 (but we trigger it in a different way).

This is also reproducible with the Animation Library manager.

Steps to reproduce

Reproducing the bug with AnimatedSprite2D and Sprite Frames

1. Open demo scene
2. Select AnimatedSprite2D
3. In inspector, unfold Sprite Frames and see how it points to sprite_frames2.tres
4. Make Sprite Frames unique (you can also replace it with New SpriteFrames). Note that if you save now (optional), the path will get filled with an embedded resource path such as "res://demo.tscn::SpriteFrames_irpuo".
5. Save Sprite Frames > in file selection popup, select sprite_frames2.tres (it is important to select this file if you want to get the error immediately, because it's the one that was previously assigned).
6. When prompted "Do you want to overwrite it?", answer OK
7. Check the Output console for the error: "Another resource is loaded from path 'res://sprite_frames2.tres' (possible cyclic resource inclusion).". Note that the inspector doesn't show that the node is referencing the target resource (path is empty).
8. Save to confirm the change: note how path gets filled with embedded resource path such as res://demo.tscn::SpriteFrames_irpuo".
9. Check VCS to confirm that node is using embedded resource, not target file.
10. Repeat steps 4-6 except with sprite_frames1.tres to demonstrate how this only triggers for resource previously used.
11. Note how everything is normal, and inspector shows reference to res://sprite_frames1.tres.
12. Save: inspector still looks good. Check VCS: node refers to res://sprite_frames1.tres indeed, and sprite_frames1.tres content has been overwritten with sprite_frames2.tres content.
13. However, if you check sprite_frames1.tres in Godot, note that it still shows the old sprite_frames1.tres content. This is a bug in the editor, and a restart will show the new content properly (may be related to https://github.com/godotengine/godot/issues/30302)
14. Repeat step 10. This time, you get the same error as with sprite_frames2.tres, because we used the same resource twice.
15. Repeat those steps as many times as you want. If you restart the editor, it will clear the memory of past references not currently set in the inspector.

Reproducing the bug with CollisionShape2D and Collision Shape

Same as above, but select CollisionShape2D, Shape and collision_shape1/2.tres

Reproducing the bug with AnimationPlayer and AnimationLibrary

1. Open demo scene
2. Select AnimationPlayer
3. In Animation panel, select Animation > Manage Animations...
4. Click on the save icon on the right of [Global] > animation_library2.tres, and select Make Unique
5. Click on the save icon again and Save. In the prompt window, select animation_library2.tres and confirm overwrite.
6. Observe an error similar to step 7. in Reproducing the bug with AnimatedSprite2D and Sprite Frames
7. The rest behaves the same way: if you save over animation_library1.tres which was not used previously, you need to do it twice to cause the error.
8. You can also save individual animations like new_animation2 > [built-in] in the manager popup.
9. Note that Save As does not cause the error. It properly overwrites the target file and binds the reference to it. You can repeat Save As as many times as you want.

I think the Animation Library manager is interesting for resolution, because it has an extra Save As action that other resources don't have, and it works as expected. So looking at its implementation may be a hint to fix the issue.

In fact, we could even add a Save As action to the inspector embedded resource contextual menu, for a quick replacement of Make Unique + Save. But even then, we'd need to fix the issue when using Make Unique + Save separately.

Minimal reproduction project

Saving embedded resource overwriting existing one doesnt update resource reference.zip

[hsandt]

I reproduced part of the error with a Theme: I created a new theme and saved it, overwriting an existing theme.

This time, I didn't get the "possible cyclic resource inclusion" error. However, the reference failed to be updated in the inspector. From here, I can save and save again and it will keep prompting me for a path to save, until I actually save in a new file and not override an existing one.

So, the two errors (cyclic inclusion and reference not updating) may be unrelated.