Bugfix: user-provided paths may now contain spaces

Previously, paths containing spaces would be interpreted by python's argument
parser as multiple separate arguments, causing it to fail. Now we escape paths
inside PythonWrapper using std::quoted().

* Improve PII filtering. Python output would contain multiple path separators
  (like C:\\Users\\foo\\), defeating the PII regex.
* Silence compiler warning in PII filter.
* Document usability improvements.
* Transcription layer exponential backoff goes to ~infinity when paused.
  This is a hack, since we really don't need to transcribe at all when paused,
  but it lets us keep the code simple. Good enough until the next rewrite.
* Shader only samples background when necessary.
* Limit matchStrings() print()s to DEBUG mode

4 files changed, 72 insertions, 67 deletions

diff --git a/GUI/GUI/GUI/Frame.cpp b/GUI/GUI/GUI/Frame.cpp
index cfb2060..5fb8dd9 100644
--- a/GUI/GUI/GUI/Frame.cpp
+++ b/GUI/GUI/GUI/Frame.cpp
@@ -730,46 +730,40 @@ void Frame::OnDumpMics(wxCommandEvent& event)
     Log(transcribe_out_, "{}\n", PythonWrapper::DumpMics());

 }

 

-#define DEBUG

+bool GetUserPath(const std::string& raw, std::filesystem::path& clean, const std::string& err_prefix = "", bool must_exist = true) {

+    clean = raw;

+    if (must_exist && !std::filesystem::exists(clean)) {

+        std::ostringstream oss;

+        oss << err_prefix << ": User-provided path does not exist at " << clean << std::endl;

+        wxLogError(oss.str().c_str());

+        return false;

+    }

+    return true;

+}

 

 void Frame::OnGenerateFX(wxCommandEvent& event)

 {

-    std::filesystem::path unity_assets_path = unity_assets_file_picker_->GetPath().ToStdString();

-#ifndef DEBUG

-    if (!std::filesystem::exists(unity_assets_path)) {

-        std::ostringstream oss;

-        oss << "Cannot generate FX layer: assets directory does not exist at " << unity_assets_path << std::endl;

-        wxLogError(oss.str().c_str());

+    std::filesystem::path unity_assets_path;

+    if (!GetUserPath(unity_assets_file_picker_->GetPath().ToStdString(), unity_assets_path,

+        "Cannot generate FX layer: Failed to validate assets directory")) {

         return;

     }

-#endif

-    std::filesystem::path unity_animator_path = unity_animator_file_picker_->GetPath().ToStdString();

-#ifndef DEBUG

-    if (!std::filesystem::exists(unity_animator_path)) {

-        std::ostringstream oss;

-        oss << "Cannot generate FX layer: animator does not exist at " << unity_animator_path << std::endl;

-        wxLogError(oss.str().c_str());

+    std::filesystem::path unity_animator_path;

+    if (!GetUserPath(unity_animator_file_picker_->GetPath().ToStdString(), unity_animator_path,

+        "Cannot generate FX layer: Failed to validate animator directory")) {

         return;

     }

-#endif

-    std::filesystem::path unity_parameters_path = unity_parameters_file_picker_->GetPath().ToStdString();

-#ifndef DEBUG

-    if (!std::filesystem::exists(unity_parameters_path)) {

-        std::ostringstream oss;

-        oss << "Cannot generate FX layer: parameters do not exist at " << unity_parameters_path << std::endl;

-        wxLogError(oss.str().c_str());

+    std::filesystem::path unity_parameters_path;

+    if (!GetUserPath(unity_parameters_file_picker_->GetPath().ToStdString(), unity_parameters_path,

+        "Cannot generate FX layer: Failed to validate parameters directory")) {

         return;

     }

-#endif

-    std::filesystem::path unity_menu_path = unity_menu_file_picker_->GetPath().ToStdString();

-#ifndef DEBUG

-    if (!std::filesystem::exists(unity_menu_path)) {

-        std::ostringstream oss;

-        oss << "Cannot generate FX layer: menu does not exist at " << unity_menu_path << std::endl;

-        wxLogError(oss.str().c_str());

+    std::filesystem::path unity_menu_path;

+    if (!GetUserPath(unity_menu_file_picker_->GetPath().ToStdString(), unity_menu_path,

+        "Cannot generate FX layer: Failed to validate menu directory")) {

         return;

     }

-#endif

+

     std::string unity_animator_generated_dir = unity_animator_generated_dir_->GetLineText(0).ToStdString();

     std::string unity_animator_generated_name = unity_animator_generated_name_->GetLineText(0).ToStdString();

     std::string unity_parameters_generated_name = unity_parameters_generated_name_->GetLineText(0).ToStdString();

@@ -804,10 +798,10 @@ void Frame::OnGenerateFX(wxCommandEvent& event)
 

     std::string out;

     if (!PythonWrapper::GenerateAnimator(

-        unity_assets_path.string(),

-        unity_animator_path.string(),

-        unity_parameters_path.string(),

-        unity_menu_path.string(),

+        unity_assets_path,

+        unity_animator_path,

+        unity_parameters_path,

+        unity_menu_path,

         unity_animator_generated_dir,

         unity_animator_generated_name,

         unity_parameters_generated_name,

diff --git a/GUI/GUI/GUI/Logging.cpp b/GUI/GUI/GUI/Logging.cpp
index 6727ba1..b9f3be4 100644
--- a/GUI/GUI/GUI/Logging.cpp
+++ b/GUI/GUI/GUI/Logging.cpp
@@ -6,12 +6,13 @@
 std::string Logging::HidePII(const std::string&& str,

 	const std::string& replacement) {

 	try {

-		std::regex c_users("(C:\\\\Users\\\\)[a-zA-Z0-9_]+");

+		std::regex c_users("(C:\\\\+Users\\\\+)[a-zA-Z0-9_ ]+");

 		std::string real_replacement = "$1" + replacement;

 		return std::regex_replace(str, c_users, real_replacement);

 	}

-	catch (const std::regex_error& e) {

+	catch (const std::exception& e) {

 		wxLogFatalError(e.what());

 	}

 	wxLogFatalError("Unhandled regex error (HidePII)");

+	return "";  // Compiler thinks we can get here (we can't) and prints a warning.

 }

diff --git a/GUI/GUI/GUI/PythonWrapper.cpp b/GUI/GUI/GUI/PythonWrapper.cpp
index 81366e5..a38ee4a 100644
--- a/GUI/GUI/GUI/PythonWrapper.cpp
+++ b/GUI/GUI/GUI/PythonWrapper.cpp
@@ -163,11 +163,18 @@ wxProcess* PythonWrapper::StartApp(
 		std::move(exit_callback));
 }
 
+// Wrap the filesystem path in quotes, escaping intermediate quotes with \\.
+std::string Quote(const std::filesystem::path& p) {
+	std::ostringstream oss;
+	oss << std::quoted(p.string());
+	return oss.str();
+}
+
 bool PythonWrapper::GenerateAnimator(
-	const std::string& unity_assets_path,
-	const std::string& unity_animator_path,
-	const std::string& unity_parameters_path,
-	const std::string& unity_menu_path,
+	const std::filesystem::path& unity_assets_path,
+	const std::filesystem::path& unity_animator_path,
+	const std::filesystem::path& unity_parameters_path,
+	const std::filesystem::path& unity_menu_path,
 	const std::string& unity_animator_generated_dir,
 	const std::string& unity_animator_generated_name,
 	const std::string& unity_parameters_generated_name,
@@ -305,9 +312,9 @@ bool PythonWrapper::GenerateAnimator(
 	{
 		Log(out, "Generating guid.map... ");
 		std::string py_stdout, py_stderr;
-		if (InvokeWithArgs({ libunity_path, "guid_map",
-			"--project_root", unity_assets_path,
-			"--save_to", guid_map_path.string() },
+		if (PythonWrapper::InvokeWithArgs({ libunity_path, "guid_map",
+			"--project_root", Quote(unity_assets_path),
+			"--save_to", Quote(guid_map_path), },
 			&py_stdout, &py_stderr)) {
 			Log(out, "success!\n");
 			Log(out, py_stdout.c_str());
@@ -329,8 +336,8 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Generating animations... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ libtastt_path, "gen_anims",
-			"--gen_anim_dir", tastt_animations_path.string(),
-			"--guid_map", guid_map_path.string(),
+			"--gen_anim_dir", Quote(tastt_animations_path),
+			"--guid_map", Quote(guid_map_path),
 			"--chars_per_sync", chars_per_sync,
 			"--bytes_per_char", bytes_per_char,
 			"--rows", std::to_string(rows),
@@ -356,9 +363,9 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Generating FX layer... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ libtastt_path, "gen_fx",
-			"--fx_dest", tastt_fx0_path.string(),
-			"--gen_anim_dir", tastt_animations_path.string(),
-			"--guid_map", guid_map_path.string(),
+			"--fx_dest", Quote(tastt_fx0_path),
+			"--gen_anim_dir", Quote(tastt_animations_path),
+			"--guid_map", Quote(guid_map_path),
 			"--chars_per_sync", chars_per_sync,
 			"--bytes_per_char", bytes_per_char,
 			"--rows", std::to_string(rows),
@@ -384,10 +391,10 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Adding enable/disable toggle... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ libunity_path, "add_toggle",
-			"--fx0", tastt_fx0_path.string(),
-			"--fx_dest", tastt_fx1_path.string(),
-			"--gen_anim_dir", tastt_animations_path.string(),
-			"--guid_map", guid_map_path.string() },
+			"--fx0", Quote(tastt_fx0_path),
+			"--fx_dest", Quote(tastt_fx1_path),
+			"--gen_anim_dir", Quote(tastt_animations_path),
+			"--guid_map", Quote(guid_map_path), },
 			&py_stdout, &py_stderr)) {
 			Log(out, "success!\n");
 			Log(out, py_stdout.c_str());
@@ -409,9 +416,9 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Merging with user animator... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ libunity_path, "merge",
-			"--fx0", unity_animator_path,
-			"--fx1", tastt_fx1_path.string(),
-			"--fx_dest", tastt_fx2_path.string() },
+			"--fx0", Quote(unity_animator_path),
+			"--fx1", Quote(tastt_fx1_path),
+			"--fx_dest", Quote(tastt_fx2_path), },
 			&py_stdout, &py_stderr)) {
 			Log(out, "success!\n");
 			Log(out, py_stdout.c_str());
@@ -433,10 +440,10 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Setting noop animations... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ libunity_path, "set_noop_anim",
-			"--fx0", tastt_fx2_path.string(),
-			"--fx_dest", tastt_animator_path.string(),
-			"--gen_anim_dir", tastt_animations_path.string(),
-			"--guid_map", guid_map_path.string() },
+			"--fx0", Quote(tastt_fx2_path),
+			"--fx_dest", Quote(tastt_animator_path),
+			"--gen_anim_dir", Quote(tastt_animations_path),
+			"--guid_map", Quote(guid_map_path), },
 			&py_stdout, &py_stderr)) {
 			Log(out, "success!\n");
 			Log(out, py_stdout.c_str());
@@ -458,8 +465,8 @@ bool PythonWrapper::GenerateAnimator(
 		Log(out, "Generating avatar parameters... ");
 		std::string py_stdout, py_stderr;
 		if (InvokeWithArgs({ generate_params_path,
-			"--old_params", unity_parameters_path,
-			"--new_params", tastt_params_path.string(),
+			"--old_params", Quote(unity_parameters_path),
+			"--new_params", Quote(tastt_params_path),
 			"--chars_per_sync", chars_per_sync,
 			"--bytes_per_char", bytes_per_char },
 			&py_stdout, &py_stderr)) {
@@ -482,9 +489,11 @@ bool PythonWrapper::GenerateAnimator(
 	{
 		Log(out, "Generating avatar menu... ");
 		std::string py_stdout, py_stderr;
-		if (InvokeWithArgs({ generate_menu_path,
-			"--old_menu", unity_menu_path,
-			"--new_menu", tastt_menu_path.string()},
+		// No idea why, but inlining this into `InvokeWithArgs` confuses the compiler.
+		std::vector<std::string> args = { generate_menu_path,
+			"--old_menu", Quote(unity_menu_path),
+			"--new_menu", Quote(tastt_menu_path), };
+		if (InvokeWithArgs( std::move(args),
 			&py_stdout, &py_stderr)) {
 			Log(out, "success!\n");
 			Log(out, py_stdout.c_str());
diff --git a/GUI/GUI/GUI/PythonWrapper.h b/GUI/GUI/GUI/PythonWrapper.h
index a60bdae..fed8e7b 100644
--- a/GUI/GUI/GUI/PythonWrapper.h
+++ b/GUI/GUI/GUI/PythonWrapper.h
@@ -8,6 +8,7 @@
 
 #include <wx/process.h>
 
+#include <filesystem>
 #include <string>
 #include <vector>
 
@@ -56,10 +57,10 @@ namespace PythonWrapper
 		);
 
 	bool GenerateAnimator(
-		const std::string& unity_assets_path,
-		const std::string& unity_animator_path,
-		const std::string& unity_parameters_path,
-		const std::string& unity_menu_path,
+		const std::filesystem::path& unity_assets_path,
+		const std::filesystem::path& unity_animator_path,
+		const std::filesystem::path& unity_parameters_path,
+		const std::filesystem::path& unity_menu_path,
 		const std::string& unity_animator_generated_dir,
 		const std::string& unity_animator_generated_name,
 		const std::string& unity_parameters_generated_name,