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

Moving a resource via the editor filesystem to another directory fails on Windows (and can even result in data loss) #96257

Closed
SaracenOne opened this issue Aug 29, 2024 · 0 comments · Fixed by #96258

Comments

@SaracenOne
Copy link
Member

Tested versions

e439154

System information

Windows 11

Issue description

Any attempt to move a resource via the filesystem dock will cause an error and the operation to fail. This appears to have been caused by a typo introduced during @bruvzg's modernization of the API in #91902, where the final line of the method DirAccessWindows::rename is:

return MoveFileW((LPCWSTR)(path.utf16().get_data()), (LPCWSTR)(p_new_path.utf16().get_data())) != 0 ? OK : FAILED;
when it should actually be:
return MoveFileW((LPCWSTR)(path.utf16().get_data()), (LPCWSTR)(new_path.utf16().get_data())) != 0 ? OK : FAILED;

This typo is easily fixed, but a broader concern is that in the event of file operation like this failing, it can easily result in crashes and even permenant data loss. This appears to be a result of the fact that the code in FileSystemDock::_move_operation_confirm has basically no checks for failed move operations, with the engine assuming a resource move was completely successful.

While the main bug is easy to fix, this lack of error checking should also be considered as a point of concern due to the risk of data loss.

Steps to reproduce

  • Open the MRP
  • Open test.tscn
  • In the filesystem dock, drag icon.svg into the test folder
  • The engine will crash and test.tscn will be permenantly lost.

Minimal reproduction project (MRP)

rename_fail.zip

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

Successfully merging a pull request may close this issue.

1 participant