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

gencache: filename normalization & update #40

Open
wants to merge 7 commits into
base: master
from

Conversation

@carstene1ns
Copy link
Member

carstene1ns commented Feb 9, 2019

暗黒魔導士 (VIPRPG game) left new, right old:
20190209-130802


Fixes #33

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Feb 9, 2019

don't think you need this special codepage handling for Windows because ICU will use the default codepage on Windows when not specified.
... and I must fix the build ;)

@carstene1ns

This comment has been minimized.

Copy link
Member Author

carstene1ns commented Feb 9, 2019

@Ghabry: You mean the codepage handling that you introduced here? 👀

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Feb 9, 2019

seems so but now I know better xD

@carstene1ns carstene1ns force-pushed the carstene1ns:gencache-normalize branch from 9e703ea to 3a91553 Feb 10, 2019
@carstene1ns carstene1ns changed the title gencache: filename normalization gencache: filename normalization & update Feb 10, 2019
@carstene1ns

This comment has been minimized.

Copy link
Member Author

carstene1ns commented Feb 11, 2019

Ready for review. Needs test under windows.

@carstene1ns carstene1ns requested a review from Ghabry Feb 24, 2019
@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Feb 24, 2019

I'm currently against merging this because normalize is broken in emscripten and when we merge this the result of index.json will be different to what the Player does.

and still lacks a Windows test.

@carstene1ns

This comment has been minimized.

Copy link
Member Author

carstene1ns commented Feb 25, 2019

Right, but this needs either be solved before 0.6 or we need to change Player (at least for emscripten).

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Mar 26, 2019

today I will finally test on Windows...

nlohmann json becomes slowly really bloated btw, they added 8000 new lines o.O
Maybe we should consider making this an external dep someday.

@carstene1ns

This comment has been minimized.

Copy link
Member Author

carstene1ns commented Mar 26, 2019

Proposed new format:

[
  "metadata",
  {
    "date": "2019-03-26",
    "version": 2
  },
  "cache",
  {
    "battle": {
      "breath": "Breath.png",
      
    },
    "charset": {
      "0pict01": "0pict01.xyz",
      
    },
    
    "rpg_rt.exe": "RPG_RT.exe",
    "rpg_rt.ini": "RPG_RT.ini",
    "rpg_rt.ldb": "RPG_RT.ldb",
    "rpg_rt.lmt": "RPG_RT.lmt",
    
  }
]

Full file here: index.json

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Mar 26, 2019

okay, accepting this as "index v2" :)

When looking at this nice code, picojson is really terrible syntax-wise but I can handle this.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Apr 3, 2019

okay, so how to do the folder case mapping?

For simplicity I'm still in favor of a "_name" key in each folder object. Only issue is that there is unlikely possibility for a collision.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented May 6, 2019

Here is a patch which adds the suggested "_dirname" well-known key.
To make parsing easier (and against the JSON standard which doesn't define an order) I wanted to have that key as the first item but that's not as simple as I thought, so the index.json parsing in Player will be slightly more complex.

diff --git a/gencache/src/main.cpp b/gencache/src/main.cpp
index 7abebfa..1e4d9e0 100644
--- a/gencache/src/main.cpp
+++ b/gencache/src/main.cpp
@@ -28,10 +28,18 @@ using json = nlohmann::json;
 
 const icu::Normalizer2* icu_normalizer;
 
-std::string strip_ext(std::string in_file) {
+std::string strip_ext(const std::string& in_file) {
 	return in_file.substr(0, in_file.find_last_of("."));
 }
 
+std::string basename(const std::string& in_file) {
+	size_t pos = in_file.find_last_of("/");
+	if (pos == std::string::npos) {
+		return in_file;
+	}
+	return in_file.substr(pos + 1);
+}
+
 json parse_dir_recursive(const std::string& path, const int depth, const bool first = false) {
 	DIR *dir;
 	struct dirent *dent;
@@ -44,6 +52,11 @@ json parse_dir_recursive(const std::string& path, const int depth, const bool fi
 
 	dir = opendir(path.c_str());
 	if (dir != nullptr) {
+		std::string base_name = basename(path);
+		if (base_name != ".") {
+			r["_dirname"] = basename(path);
+		}
+
 		while ((dent = readdir(dir)) != nullptr) {
 			std::string dirname;
 			std::string lower_dirname;
@@ -67,6 +80,11 @@ json parse_dir_recursive(const std::string& path, const int depth, const bool fi
 				normalized_dirname.toUTF8String(lower_dirname);
 			}
 
+			if (dirname == "_dirname") {
+				std::cerr << "Skipping _dirname: File conflicts with reserved keyword!" << std::endl;
+				continue;
+			}
+
 			/* dig deeper, but skip upper and current directory */
 			if (dent->d_type == DT_DIR && dirname != ".." && dirname != ".") {
 				json temp = parse_dir_recursive(path + "/" + dirname, depth - 1);
@fdelapena

This comment has been minimized.

Copy link
Contributor

fdelapena commented Aug 29, 2019

Currently gencache seems to lack path traversal support, perhaps there's some interest to add it to this PR or to be handled after merging this one.

Test: https://easyrpg.org/play/master/?game=frozen_triggers

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Aug 30, 2019

imo the problem is that we don't make the path canonical before looking in the cache.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Sep 23, 2019

(note: the path traversal was fixed in easyrpg player by making the path canonical before looking into the cache. No change to gencache needed)

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Mar 3, 2020

@carstene1ns
Based on the forum post: Don't think this supports unicode path on Windows because the "w" APIs are not used.
Maybe grab the platform.cpp from Player (will relicense it) and use this for directory parsing.

And are you fine with the "_dirname" well-known key? If yes we can maybe finally resolve this soon ^^.

@Ghabry

This comment has been minimized.

Copy link
Member

Ghabry commented Mar 4, 2020

Due to popular demand (one user ;)) Unicode Windows support: https://github.com/EasyRPG/Tools/compare/master...Ghabry:gencache?expand=1

Only for parsing, command line still ANSI because there is little gain in supporting this.

@carstene1ns

This comment has been minimized.

Copy link
Member Author

carstene1ns commented Mar 5, 2020

Good stuff. Now I only need some time to finish this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.