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

Deleting a resources in FileSystem doesn't remove references to it #78233

Open
ajreckof opened this issue Jun 14, 2023 · 11 comments
Open

Deleting a resources in FileSystem doesn't remove references to it #78233

ajreckof opened this issue Jun 14, 2023 · 11 comments

Comments

@ajreckof
Copy link
Member

ajreckof commented Jun 14, 2023

Godot version

v4.1.beta.custom_build [01453ab]

System information

macOS 13.3.0 - Vulkan (Forward+) - integrated Apple M1 Pro - Apple M1 Pro (8 Threads)

Issue description

When deleting a resource from the FilesSystem dock. The resource won't be removed from scenes but won't immediately be saved internally. As a workaround if the scene is opened you can save it immediatelly before closing but it won't warn you when closing that there are unsaved action.
This will lead to multiple type of errors varying from cannot open resource or cannot find script to cannot open sceneor even in the worst cases to outright crashes like in here (#78238)

Steps to reproduce

Here is what happen for script

  1. Create a scene with one nodes
  2. attach a script to the root
  3. Save the scene
  4. Delete the script
  5. click on the script it will show as [unsaved] trying to save the script (not the scene) won't work
  6. close the scene and it won't tell you you have something to save
  7. reopen the scene now it will state he couldn't find the script.

For ressources it will be a bit different :

  1. Create a scene with one nodes
  2. attach a script to the root exporting a resource
  3. create a resource and save it on disk
  4. Save the scene
  5. delete the resource
  6. click on the script it will show that there is no resource path meaning it hasn't been saved inside the scene
  7. close the scene and it won't tell you you have something to save
  8. try to reopen the scene and it will fail because of missing resources

Minimal reproduction project

with the MRP you can start reproduction steps at the sixth step.
Test.zip

Edit : separated the crash from the rest of the bug as even we handle correctly how we delete a resource in FileSystem. The crash seems to come from Array[Node] specifics

@AThousandShips
Copy link
Member

AThousandShips commented Jun 14, 2023

Is this limited to only scripts, since you mentioned scripts, or general?

@AThousandShips AThousandShips changed the title When deleting a resource in FIleSystem. The resource won't be removed from nodes but won't be saved internally leading to errors and even crash in some cases Deleting a resources in FIleSystem doesn't remove references to it Jun 14, 2023
@ajreckof
Copy link
Member Author

This is affecting all ressource type

@AThousandShips
Copy link
Member

AThousandShips commented Jun 14, 2023

I suspect the focus here should be on graciously handling the incorrect references, as opposed to deleting the references, as that can cause problems if you accidentally delete and then restore the resources, but erroring and crashing isn't good

Added regression based on comment in rocket chat

@ajreckof
Copy link
Member Author

ajreckof commented Jun 14, 2023

I see two major possibility.
The first one is to resave all scenes containing the resource with the resource saved as internal. This feels hard to implement and might be take some time as we need to open the scene make the modification and resave the scene. This could be made optional if the second possibility has been implemented.
The second one is to better handle missing dependencies this would be a bit simpler since we already have a edit dependencies window. So we could just show it immediately when we find missing dependencies. And only open scene afterwards. I feel like this one use to be the case.

Both can and I think should be implemented as they are complementary to solve the problem shown above

@AThousandShips
Copy link
Member

I mean the way this is usually intended to be solved, at least for non-scripts, is using UIDs, and modifying references is something I think should be avoided, to prevent accidental issues as mentioned above. But we should and can ensure that it navigates this gracefully, can't quite tell what might have gone wrong here as I haven't dug into it today, will see if I have time later to investigate further

@ajreckof
Copy link
Member Author

modifying references is already the way it is handled as if you have the scene opened saving it will save the resources internally. The first solution is to make this automatic on deletion of the resource.
If we wish to properly handle miss fired deletion I think the best way would be to add undo redo action to the FileSystem dock.
Only showing the edit dependencies when reopening while being inconsistent (between opened and not opened scene) will still be afix to this issue

@ajreckof
Copy link
Member Author

So took a look and effectively Godot 3 had a popup about load failing because of missing dependencies
Capture d’écran 2023-06-14 à 21 33 16
(sorry for my atrocious theme)

@theraot
Copy link
Contributor

theraot commented Jun 14, 2023

Yes, Godot does not prompt to fix broken references anymore.

By the way, see also: #76941

@akien-mga akien-mga changed the title Deleting a resources in FIleSystem doesn't remove references to it Deleting a resources in FileSystem doesn't remove references to it Jun 23, 2023
@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 23, 2023
@KoBeWi
Copy link
Member

KoBeWi commented Nov 6, 2023

Added regression based on comment in rocket chat

What comment? IIIRC deleting resources never updated references. Only moving files does so.

Also the dependency dialog missing was fixed.

@AThousandShips
Copy link
Member

What comment?

I do not remember, will try find

@YuriSizov YuriSizov modified the milestones: 4.2, 4.3 Nov 15, 2023
@KoBeWi
Copy link
Member

KoBeWi commented Jul 25, 2024

The crashes and corruption bugs are all resolved now, missing resources only result in error prints. Unless it breaks after export, it's harmless.

@KoBeWi KoBeWi removed this from the 4.3 milestone Jul 25, 2024
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

7 participants