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

Filefinder: Should emit a warning when multiple folders map to the same normalization #1594

Closed
talib1101 opened this issue Dec 26, 2018 · 6 comments

Comments

@talib1101
Copy link

commented Dec 26, 2018

Name of the game:

ゆめ2っき (Yume 2kki)

Player platform:

Ubuntu 18.10 64-bit

Log file:

easyrpg_log.txt

How to reproduce it:

Start new game or load save (Occurs in new and saved games)

What Happens:

Much of the game's music does not play. A notable example is the error
Debug: Cannot find: Music/zaza (!)
because zaza plays whenever entering the dream world (which can be done by sleeping in the bed)
Another good example is ym2-00_ano which should play with new games at intro
Debug: Cannot find: Music/ym2-00_ano deskwork (!)
Debug: Music not found: ym2-00_ano deskwork

Notes:

I have verified that I do have the file zaza.
I've tested it with the version at https://drive.google.com/open?id=1nkGMdjg1PDF89Le7mnMmsTLhoO-RJMsg
and the version at https://wikiwiki.jp/yumenikki-g3/

@Ghabry Ghabry added the FileFinder label Dec 26, 2018

@carstene1ns

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

This is interesting. First thing I see when opening the archive is this:
20181230-112225

Can you please merge both Music directories and the error should be gone. The uppercase folder comes from the included RTP.
This can not happen on case-insensitive filesystems, as they likely get merged when extracting.
However, I wonder how this messes up our filefinder, as both folders should have the same key.

@Ghabry

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

the problem is that we have a folder case-mapping, too, so both Music folders will map to the same folder key which means only the files in the last folder will be found (the other should be found, too, but the wrong folder is used so this still fails when opening the files, or the key is overwritten, then only the last files are listed).

This could be fixable as part of the VFS-branch or after as I don't want to touch FileFinder now for nice-to-have-features (rebase hell)

@Ghabry

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

Based on chat discussion the least-complex solution which has no huge performance penalty to handle this corner case is emitting a warning when this happens. When there are not too many games affected by this this is good enough imo.

@talib1101

This comment has been minimized.

Copy link
Author

commented Dec 30, 2018

Can you please merge both Music directories and the error should be gone. The uppercase folder comes from the included RTP.

Thanks for the timely and helpful response. I can confirm that merging the contents of both folders into 'music' and deleting 'Music' fixed the issue. It would have been good if an error message popped up on screen like when files are missing, or if the issue never occurred because it was handled automatically.

@carstene1ns

This comment has been minimized.

Copy link
Member

commented Dec 30, 2018

This error case should only happen very seldom, as you can only create such archives when moving around stuff and RPG Maker will never do it. Also, it cannot happen on Windows, as the filesystem is case-insensitive. Some archive formats (zip?) do not support this either.
In this case even wine has no workaround to look in similar folders, so RPG_RT will also play silence there.

However, as already said we can add a check and a warning for this as we are indexing the folders anyway, keeping track of them is easy.

I am going to edit the issue title.

@carstene1ns carstene1ns changed the title Cannot find Music/ files Filefinder: Should emit a warning when multiple folders map to the same normalization Dec 30, 2018

@carstene1ns carstene1ns added this to the 0.6.x milestone Dec 30, 2018

@carstene1ns

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

Here is a preliminary patch:

diff --git a/src/filefinder.cpp b/src/filefinder.cpp
index 1f0b6f74..54acc8b9 100644
--- a/src/filefinder.cpp
+++ b/src/filefinder.cpp
@@ -22,6 +22,7 @@
 #include <cstdlib>
 #include <cstring>
 #include <algorithm>
+#include <unordered_set>
 #include <fstream>
 #include <string>
 #include <vector>
@@ -717,6 +718,7 @@ FileFinder::Directory FileFinder::GetDirectoryMembers(const std::string& path, F
 	assert(FileFinder::IsDirectory(path));
 
 	Directory result;
+	std::unordered_set<std::string> found_dirs;
 
 	result.base = path;
 
@@ -805,6 +807,13 @@ FileFinder::Directory FileFinder::GetDirectoryMembers(const std::string& path, F
 		}
 		if (is_directory) {
 			result.directories[Utils::LowerCase(name)] = name;
+
+			auto search_dir = found_dirs.find(Utils::LowerCase(name));
+			if (search_dir != found_dirs.end()) {
+				Output::Warning("Duplicated directory \"%s\", this can lead to file not found errors.", name.c_str());
+			} else {
+				found_dirs.insert(Utils::LowerCase(name));
+			}
 		} else {
 			result.files[Utils::LowerCase(name)] = name;
 		}

Patch problems:

  • displays the message twice upon game load
  • displays message when game list is read from game browser

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Jun 12, 2019

@Ghabry Ghabry referenced this issue Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.