ClassiCube / MCGalaxy

A Minecraft Classic / ClassiCube server software
GNU General Public License v3.0
160 stars 76 forks source link

MCGalaxyCLI switches to unusable directory on startup, then crashes. #775

Open rdebath opened 10 months ago

rdebath commented 10 months ago

It would be nice to have a quick check to see if the executable directory is probably usable before jumping there. For example like below.

Also can we delay calling "CheckFile" to download SQL DLLs until we know they're needed; so, like the SQLlite DLL check only does it on windows both the MySQL and SQLlite checks should only do the download if they are configured to be used.

diff --git a/CLI/Program.cs b/CLI/Program.cs
index 23dadec4c..e4f4032dd 100644
--- a/CLI/Program.cs
+++ b/CLI/Program.cs
@@ -46,6 +46,7 @@ namespace MCGalaxy.Cli {
         static void SetCurrentDirectory() {
             string path = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
             try {
+                if (!Directory.Exists(path + "/logs")) Directory.CreateDirectory(path + "/logs");
                 Environment.CurrentDirectory = path;
             } catch {
                 // assembly.Location usually gives full path of the .exe, but has issues with mkbundle
UnknownShadow200 commented 10 months ago

1) I'd actually prefer to get rid of altering the current directory altogether, but I am unsure if making that change would break someone's setup

2) I download both to handle the admittedly extremely unlikely case that the server owner decides to change from SQLite to MySQL (or vice versa) and then does /server reload, but doesn't restart the server

Admittedly, this might be such an edge case that it isn't worth worrying about trying to handle

rdebath commented 10 months ago

Yesss, I came across your message where you were congratulating someone on being the only person who was actually using MySQL. I'd say the set of people who ... (1) don't follow instructions to download the DLL themselves, (2) work out how to turn on mysql and (3) decide to use /server reload rather than /restart is exactly zero. What's more you already have them covered by the warning that they might have to restart anyway.

But, actually, it only bothers me in terms of patching out the directory change so if the checkFile() checks both the current directory and the executable directory I'd be happy :grin:.

OTOH the number of people who have managed to create a link to start mcgalaxy without specifying the right starting directory is going to be rather larger. Especially if they're starting it from task scheduler or something similar.

While checking for write permission would be sufficient in my case I'd suggest adding the -d option from Classicube is lots safer than just deleting the facility. That way it still always changes directory, it just that it can be controlled. (NB: "." should be allowed)