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

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

Open
rdebath opened this issue Sep 7, 2023 · 2 comments
Open

Comments

@rdebath
Copy link
Contributor

rdebath commented Sep 7, 2023

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
Copy link
Collaborator

UnknownShadow200 commented Sep 7, 2023

  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
Copy link
Contributor Author

rdebath commented Sep 7, 2023

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 😁.

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)

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

No branches or pull requests

2 participants