Skip to content

Standardize paths used under drag and drop action in Windows platform specfically#114555

Open
onequid wants to merge 1 commit intogodotengine:masterfrom
onequid:drag-n-drop-same-location
Open

Standardize paths used under drag and drop action in Windows platform specfically#114555
onequid wants to merge 1 commit intogodotengine:masterfrom
onequid:drag-n-drop-same-location

Conversation

@onequid
Copy link
Copy Markdown
Contributor

@onequid onequid commented Jan 3, 2026

The paths of files/directories under drag-n-drop action from system file explorer to godot assets dock in Windows are standardized now.

It fixes the check here:

Error DirAccess::copy(const String &p_from, const String &p_to, int p_chmod_flags) {
ERR_FAIL_COND_V_MSG(p_from == p_to, ERR_INVALID_PARAMETER, "Source and destination path are equal.");

This also fixes the freeze of godot editor and duplicated tmp files after dragging and dropping files(assets) from OS File Explorer to where they already are, as is described here:
#114218 (comment)

Additionally, create_temp_dir, which is used by handle_filedescriptor_format, also returns a standardized path string.


Update(Incapable)

The `DirAccessWindows::is_equivalent(a, b)` function compared hard driver information of paths, namely volume serial number, LowPart, HighPart. Only `DirAcessUnix` and `DirAccessWindows` implement the `is_equivalent(a, b)` function. `DirAcessUnix::is_equivalent(a, b)` is **NOT** tested yet, though it seems to work as it returns string comparison result if it failed checking file system ID.

Marked the PR as draft, before a Unix test or approval.

And though the DirAccessWindows::is_equivalent(a, b) formats both the given path arguements with fix_path(p_path) function, the calling of simplify_path() should remain as they should be standardized.

Copy link
Copy Markdown
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ERR_FAIL_COND_V_MSG(p_from == p_to, ERR_INVALID_PARAMETER, "Source and destination path are equal.");

This check likely should be DirAccess::is_equivalent(a, b) (if it works on directories on all platform, the method might need updating), which checks file system internal IDs instead of paths. simplify_path might be not sufficient for all cases.

@onequid
Copy link
Copy Markdown
Contributor Author

onequid commented Jan 8, 2026

I see. I was not aware of the is_equivalent(a, b) function.
I will check what it does later.

And the simplify_path() method should still be used to standardize the paths, right? In #50 the return before the change is something with mixed slashes:
"C:\path\to\tmpdir\/tmp_filename"

@onequid onequid force-pushed the drag-n-drop-same-location branch from 3a2e90d to b475845 Compare January 8, 2026 14:30
@onequid onequid requested a review from a team as a code owner January 8, 2026 14:30
@onequid onequid marked this pull request as draft January 8, 2026 14:52
Copy link
Copy Markdown
Contributor

@AR-DEV-1 AR-DEV-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested on Linux (Godot v4.6.beta.mono (5abc97c29) - Zorin OS 18 on Wayland - X11 display driver, Multi-window, 1 monitor - Vulkan (Forward+) - integrated Intel(R) HD Graphics 5500 (BDW GT2) - Intel(R) Core(TM) i3-5005U CPU @ 2.00GHz (4 threads) - 7.67 GiB memory)
The PR works as described. It would detect if I was dragging & dropping the same file & would give the error. Though, the editor does freeze sometimes. Don't know if it's my computer or Godot. Would you take a look at it?

@onequid
Copy link
Copy Markdown
Contributor Author

onequid commented Jan 9, 2026

Thank you very much for the test .
I don't think the change would make such a difference in performance. As to linux, the only change would be replacing "==" with the is_equivalent() function.

Did it freeze generally or just when assets were added?

@onequid
Copy link
Copy Markdown
Contributor Author

onequid commented Jan 9, 2026

After a second thought I'd rather not change the == with is_equivalent in DirAccess::copy():

  1. it works fine before and this PR was itended to standardize the path variables passing to it.
  2. The reason is unknonwn why there are only DirAccessUnix::copy and DirAccessWindows::copy. Were overrides for other platforms interupted for some reason, or the == check was determined to be sufficient already? And the DirAccess::copy is just == itself.
  3. In a long run, it's much more complex to maintain the codes even if all the overrides for platforms were completed.

And thanks a lot for all the help and advice here.

Standardize file paths and the temp dir path used under drag and drop action in Windows platform specifically using `simplify_path()` method.
@onequid onequid force-pushed the drag-n-drop-same-location branch from b475845 to 7e05ac9 Compare January 9, 2026 13:38
@AR-DEV-1
Copy link
Copy Markdown
Contributor

Did it freeze generally or just when assets were added?

Generally, caused my computer to also crash. But, the logic seems fine to me.

@onequid onequid marked this pull request as ready for review January 12, 2026 00:51
@onequid
Copy link
Copy Markdown
Contributor Author

onequid commented Jan 23, 2026

Adding a .png file from a web browser by drag-and-drop works on MacOS, as I was told. But it didn't work on Windows yet.
Based on the changes here, I tried to implement it.

Firstly with HTTPRequest (see "scene/main/http_request.cpp"), and HTTPRequest is only permitted to be used in a tree.
Then with the WinHttp api, this might be the last way to implement it. As we have HTTP implemented in core/io and related to "Asset Libary", so the new codes would not be reused anywhere else.

Before more efforts, whether this feature (drag-n-drop from a browser) is wanted or not?
We have the "Asset Library" already, the feature may not be used often, but personally it is still cool to just drag the assets from a web browser, and maybe further it can be integrated with the "Asset Library" so that even extensions can be installed via drag. (Should a proposal be opened?)

If it is wanted, should it create a download task like those are downloaded from "Asset Library", which it probably does not on MacOS/Unix now(need comfirm)?

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants