ppy / osu

rhythm is just a *click* away!
https://osu.ppy.sh
MIT License
15.39k stars 2.29k forks source link

"Edit externally" fails: "Export failed" if folder is in use #29029

Open rvneXe opened 3 months ago

rvneXe commented 3 months ago

Type

Game behaviour

Bug description

Hi. I was editing my map and was using also "Edit externally" option a lot. All of sudden, when I imported one of those, its folder (in Temp directory) didn't remove after that, and since then, it doesn't work anymore for this map ONLY: image

And the folder is also unremovable: image

It didn't fix by relaunch.

Screenshots or videos

No response

Version

2024.718.1

Logs

compressed-logs.zip (The session which I got this problem)

compressed-logs.zip (First session after relaunch)

rvneXe commented 3 months ago

Update: Folder is still unremovable after closing osu. (the red line indicates where osu has to be) image

rvneXe commented 3 months ago

Update: That got fixed by restarting the PC.

I think I found the cause. When you open one of those hitsounds (for example) and don't close the player, then "finish editing", it will behave similar until you close the player, and I think that case was similar, but I didn't leave anything open in that folder, so it's still strange for me. Oh god it's so obivious, there's no problem with osu if it wasn't removable while osu wasn't openπŸ˜‚πŸ˜‚πŸ˜‚ FORGIVE ME

peppy commented 3 months ago

This means you had something open in an external app. You'll have to close it first.

Maybe the flow can better explain this to the user.

smallketchup82 commented 3 months ago

Coming from me while I was developing the first iteration of this feature, I honestly expected this issue to show up at some point.

I had thought about fixing it by ignoring such errors when attempting to delete the directory, and instead only preforming the read. Which shouldn't, in theory, cause the error. But it seems as though this is already being done. So I guess adding an error message for this is probably the best way to go about it.

Though, in general, I think adding a sort of "recovery" flow would also be a good idea. I'm more concerned about a user experiencing something like this, not having their changes saved, then re-mounting, and having their changes overwritten (see https://github.com/peppy/osu/blob/15220f3bf4697adfdd98b0d7d53e6a9c8b616172/osu.Game/Database/RealmArchiveModelImporter.cs#L186-L187), essentially losing all of their changes. I probably should've brought this up in review but didn't notice.

Also, for anyone that stumbles upon this issue in the future, I'd probably suggest using something like https://learn.microsoft.com/en-us/windows/powertoys/file-locksmith to find any processes using the files and killing them prior to accepting changes

bdach commented 3 months ago

The best ending i can see here is somehow listing which apps have the files still open, if possible. Not sure it will be without elevated permissions.

This is apparently a thing. It's windows-specific, but that's maybe fine because this is mostly a windows issue (unices don't care if another process has fds open to the same file).