Fix FileSystemDock behavior when dropping an item in the current folder

[AlexOtsuka]

Fixes #90046.

Fully gets rid of the issue showcased in the above thread. Dragging a Node branch over the empty region of a split FileSystemDock now results in proper directory highlighting, in both list view and thumbnail view. Dropping the item results in the proper directory being selected in the "Save New Scene As..." window as well.

Functionally, this is also fully fixed for Resources, except for the highlighting that was absent from the start.
Enabling highlighting in the file tree is actually quite trivial, as it's only being prevented by this single condition:

if (mm.is_valid() && holding_branch) on line 3457, where holding_branch is only true when Node branches are being dragged.

If we want highlighting for those items, we could simply introduce a new boolean and check for either of them to be true. I have no idea if this is by design, so I haven't touched it yet.

Thanks for your interest and I'm looking forward to everyone's feedback.

[mieldepoche]

I tested this pr and it seems to work as expected.

If we want highlighting for those items, we could simply introduce a new boolean and check for either of them to be true. I have no idea if this is by design, so I haven't touched it yet.

I'd say that this is wanted, see #63279

[AlexOtsuka]

If we want highlighting for those items, we could simply introduce a new boolean and check for either of them to be true. I have no idea if this is by design, so I haven't touched it yet.

I'd say that this is wanted, see #63279

Fair point, I'll wait to see what reviews I get on this and I'll add the change to the PR if people agree that it should be. Just wondering if there are weird edge cases I'm not aware of that would warrant not having it.

Edit: Yeah, I'm pretty sure all it takes is removing the Tree::DROP_MODE_INBETWEEN flag and flipping the boolean and it immediately works the exact same way, should be a very trivial fix.

[AlexOtsuka]

I changed a condition for the tree highlighting logic for resources and nodes to work the same way, which fixes #63279.

I've explored some other improvements:

• Applying the same highlighting to files and folders unless we're dragging over the favorites.
• Adding the ability to drop files in Favorites exactly where we're pointing at (similarly to when drag and dropping favorites around).

They work, but they feel like QoL improvements over bug fixes, so I'm purposefully excluding them from this PR.
However, I'd love to know if those are things you'd like to see added in a following PR!

Edit: Another update!

• Fixed a small bug where dropping resources onto the tree was sometimes not allowed when it should be.
• Fixed another bug I just discovered, which happens even in non-split mode. Dropping nodes and resources near the edges of a TreeItem folder would wrongfully pick the res:// folder as the save path. It's consistently doable on current master, try it out!

I think filesystem_dock.cpp would really benefit from a light rewrite and refactoring.

[KoBeWi]

Looks fine, aside from the duplicated code I commented.

[AlexOtsuka]

Updated this one last time.
I found a much more efficient way of achieving what I wanted instead of refactoring the aforementioned piece of code into a method.

The drop mode flag is being set right before calling _get_drag_target_folder() to get the desired behavior without messing up the way highlighting looks.

Thank you for looking at my PR and please let me know if this is satisfactory!

[mieldepoche]

edit: I tested it again and it works.

Additionally, I found an edge case visual inconsistency:

Video clip of the inconsistency

simplescreenrecorder-2024-04-05_07.39.40.mp4

as shown in this clip, the target (res://) is correct but sometimes not highlighted when dropping in the left half of the split mode.
I'm not sure it is related to the issue at hand, but might as well point it now.

[AlexOtsuka]

I tested it again and it works, but still doesn't fix #63279 as the modified op text claims. did I do something wrong? I'm pretty sure it's fine to fix here, as the original proposal was moved to the bug issues by a maintainer.

Hi, I fixed it for resources, but not for files and folders because of the extra changes required to properly deal with the Favorites. Also, I think it'd be cool to implement drag and dropping directly at the right location when adding a file or folder to the favorites, so the current highlighting style would make sense. I guess I forgot #63279 mentioned files (my PR does cover the video example there), so I'll remove it from the OP for now.

Additionally, I found an edge case visual inconsistency:
as shown in this clip, the target (res://) is correct but sometimes not highlighted when dropping in the left half of the split mode. I'm not sure it is related to the issue at hand, but might as well point it now.

Yeah, I'm aware of that one, but it's a little tricky, because I can make it select res:// when exiting the tree, but then it also messes with auto-selecting the folder that's currently open in split view. I'm not exactly sure how to fully solve it to make it look perfect, but I agree that it doesn't behave the ideal way.

Anyway, this PR is mostly about fixing the weird targeting issues and the wrong save path choice from #90046. I do see a lot of room for improving FileSystemDock and I wanna implement a bunch of changes, but I don't wanna overload the current PR with minor visual improvements.

[mieldepoche]

Hi, I fixed it for resources, but not for files and folders because of the extra changes required to properly deal with the Favorites. Also, I think it'd be cool to implement drag and dropping directly at the right location when adding a file or folder to the favorites, so the current highlighting style would make sense. I guess I forgot #63279 mentioned files (my PR does cover the video example there), so I'll remove it from the OP for now.

actually, this was about the resources part, but I fetched the wrong commit and modified my post shortly after to remove the part you quote, I didn't even think of dragging files 😅 I was too focused on the present issue. But as you say, it doesn't fully close my original report.

[akien-mga]

Thanks!