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

"A file or folder with this name already exists" errors when renaming folder to a different name #82543

Closed
akien-mga opened this issue Sep 29, 2023 · 12 comments · Fixed by #82075

Comments

@akien-mga
Copy link
Member

Godot version

Godot v4.2.dev (44e399e)

System information

Mageia 9 - X11 - GLES3 (Compatibility) - AMD Radeon RX Vega M GL Graphics (vegam, LLVM 15.0.6, DRM 3.52, 6.4.9-desktop-4.mga9) () - Intel(R) Core(TM) i7-8705G CPU @ 3.10GHz (8 Threads)

Issue description

I have this simple setup, using Split Mode:
image

If I try to rename a folder (e.g. player) in the thumbnails view, a number of bugs occur:

Start renaming with F2 or Right click > Rename

  • Keep the same name, and click in the FileSystem dock: no change, no error, as expected
  • Keep the same name, and click outside the FileSystem dock:
    • If clicking in the Scene tree (positioned in the same column for me), no change, no error
    • If clicking in the Inspector, or 3D viewport, an error is shown that it can't be renamed as the name is taken

image

And a transient window elusive child error from that popup.

  • Change the name to something unique (e.g. player2), then confirm the change with the mouse:
    • If clicking in the FileSystem dock, the rename works sometimes, and the folder is entered automatically, which seems weird to me. The selection is also out of sync between the tree view and the thumbnails view:

image

  • But in other times with the same steps, it fails with the error about a file name conflict. Not sure how to reproduce reliably, it seems to happen maybe 10% of the time for me.
  • If clicking in the Scene tree, same behavior as above.
  • If clicking in the Inspector, or 3D viewport, same file conflict error as above.
  • Change the name to something unique (e.g. player2), then confirm the change with the Enter:
    • Same behavior as with mouse click.

Some of these bugs seem specific to the Split Mode, but others seem to also happen in Tree view. It feels very buggy overall, it needs a thorough fixup pass.

Seems to be a regression which happened between 4.2-dev1 and 4.2-dev3. 4.2-dev2 is outright crashing on start so I can't test further. In dev1, I don't get the dialog about file name conflicts. I do get the weird behavior that editing a folder name enters the folder, and makes the selection out of sync in Split Mode, so that's a pre-existing and different bug.

Steps to reproduce

See above.

Minimal reproduction project

Reproducible in a brand new project.

@akien-mga
Copy link
Member Author

#75137 sounds like a potential culprit, but I didn't test a local revert to confirm.

CC @KoBeWi @nongvantinh

@nongvantinh
Copy link
Contributor

nongvantinh commented Sep 29, 2023

I pulled the latest code from master, reverted the #75137, and built the project. The bug still occurs, so #75137 is not causing this issue. However, I will take some time to bisect it.

@nongvantinh
Copy link
Contributor

The actual commit that causes the issue is 1c3c17c by @Sauermann. Unfortunately, I don't have more time to think about why it causes the bug. But after a while of bisecting, it actually causes the bug. I leave the further work for you @akien-mga.

@akien-mga
Copy link
Member Author

  • If clicking in the FileSystem dock, the rename works sometimes, and the folder is entered automatically, which seems weird to me. The selection is also out of sync between the tree view and the thumbnails view:

This part seems to be this pre-existing bug report: #80940.

@Sauermann
Copy link
Contributor

Start renaming with F2 or Right click > Rename

  • Keep the same name, and click outside the FileSystem dock:
    • If clicking in the Inspector, or 3D viewport, an error is shown that it can't be renamed as the name is taken

I have tested this behavior (in a newly created project) and I was able to replicate this bug in v4.2.dev1.official [0c2144d] on Debian X11 Linux.
So this seems also to be a preexisting issue before 1c3c17c.

#75137 got merged after dev1, so it is likely, that it isn't the cause of the problem.

I did check this bug with previous versions and I found, that it got introduced between v4.1.dev4.official [5c2295f] and v4.1.beta1.official [828ec2c]

@nongvantinh
Copy link
Contributor

Weird, I just downloaded https://github.com/godotengine/godot-builds/releases/download/4.2-dev1/Godot_v4.2-dev1_win64.exe.zip and tested it on my Windows 10, and the bug definitely did not occur. However, when I was bisecting the commit range from 4.2-dev1 to 4.2-dev5, 1c3c17c turned out to be the last bad commit. I took the time to revert it, fixed some compile-time errors, and then tested the code again before coming to a conclusion.

@Sauermann
Copy link
Contributor

I find it also confusing, that we come to different conclusions.
After doing more tests, I found, that the setting "single window mode" affects the bug.

If "single window mode" is on, then the bug starts appearing between v4.1-dev4 and v4.1-beta1
If "single window mode" is off, then the bug starts appearing in 1c3c17c

So I can confirm your bisection.

@akien-mga
Copy link
Member Author

More weirdness with the FileSystem dock that might be related to this.

With the following setup:

icon.svg
node2d.tscn
  • Select icon.png
  • Navigation to node2d.tscn with Down

image

  • Press F2 to rename. The rename box appears to be changing the focused item (as it should, and it works well in 4.1), but it actually highlights a section of the name that matches the size of icon (4 chars).

image

  • Losing focus triggers the bug described above, and actually renames the file to icon.svg, so both files end up having the same name:

image

  • But it's actually a lie, it's icon.svg which got renamed to its existing name. Double clicking res:// to force a refresh shows that the node2d.tscn file wasn't renamed:

image

  • If I repeat the first steps, and actually rename what seems to be node2d.tscn, it renames icon.svg, breaking it because it changing the extension:

image

image

@nongvantinh
Copy link
Contributor

nongvantinh commented Oct 6, 2023

The new mouse signal causes the following check to be ignored because none of them has focus()

String new_name;
TreeItem *s = tree->get_selected();
int col_index = tree->get_selected_column();
if (tree->has_focus()) {
new_name = s->get_text(col_index).strip_edges();
} else if (files->has_focus()) {
new_name = files->get_edit_text().strip_edges();
}

Perhaps, @Sauermann can explain why it does not have focus in Split Mode, especially in two different scenarios:

  1. Clicking outside FileSystemDock: the tree does not have focus().
  2. Clicking inside FileSystemDock: the tree has focus().

To address @akien-mga's second test with scene and icon: the cursor_item->is_selected(0) returned false, causing it to fallback to the previously selected item, which is the icon, and the rest happens naturally. However, it only affects the name of TreeItem and does not actually rename it.

TreeItem *favorites_item = tree->get_root()->get_first_child();
TreeItem *cursor_item = tree->get_selected();
if (cursor_item && cursor_item->is_selected(0) && cursor_item != favorites_item) {
selected_strings.push_back(cursor_item->get_metadata(0));
}

@Sauermann
Copy link
Contributor

Sauermann commented Oct 7, 2023

When renaming a file, a LineEdit Control-node opens and grabs focus from the FileSystemList, since they are in the same Window:

line_editor->grab_focus();

So the FileSystemList doesn't have focus.

@JosephCatrambone
Copy link
Contributor

JosephCatrambone commented May 7, 2024

I'm seeing this pop up again in the latest 4.3 dev 6 [89850d5]. It's 100% reproducible on Windows 10 Pro. Happy to open as a new issue if we think it's unconnected to the previous causes.

@KoBeWi
Copy link
Member

KoBeWi commented May 7, 2024

It was likely fixed by #91112, which was merged today.

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.

5 participants