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

Use logical file/folder names, Fix Linux file locations #100

Merged
merged 2 commits into from Feb 11, 2022

Conversation

dginovker
Copy link
Contributor

CONTAINS BREAKING CHANGES

Users will have to move their scripts and accounts file out of "GService" and into "OsrsBot"


Windows: C:\Users\[username]\AppData\Roaming\OsrsBot\Scripts\Precompiled
Linux: /home/[username]/.config/OsrsBot/Scripts/Precompiled
MacOS: /Users/[username]/Library/Application Support/OsrsBot/Scripts/Precompiled
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't test Windows or MacOS but I assume it's something like that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Meyer can find out for us later.

} else if (GlobalConfiguration.getCurrentOperatingSystem() == OperatingSystem.LINUX) {
path = Paths.getUnixHome() + File.separator
+ ".config" + File.separator
+ GlobalConfiguration.NAME_LOWERCASE + "_acct.ini";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linux users don't like when you pollute ~ - Config files should be in .config

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable. I don't have a Linux environment and am not heavy into it at the moment.

} else {
path = Paths.getUnixHome() + File.separator + "."
+ GlobalConfiguration.NAME_LOWERCASE + "acct";
}
return path;
}

public static String getHomeDirectory() {
public static String GetOsrsBotDirectory() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed method to be more useful

Flipped if statement to make smaller branch on top

Made Linux directory go inside .config again

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Java methods in the repo should be camelcase with a lowercase initial letter.
Not a big deal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops used to C#

@@ -149,7 +160,7 @@ public static String getUnixHome() {
}
}

public static final String NAME = "GService";
public static final String NAME = "OsrsBot";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should never have been called GService (BREAKING CHANGE) - People will have to move their stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better now than later tho

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hard agree.

else {
homeDirBuilder = Paths.getUnixHome();
}
return (homeDirBuilder + File.separator + GlobalConfiguration.NAME);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can actually remove the else statement and let env be the default return right?
Could also opt for a switch case on the if else.
Not sure what would look cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then MacOS users would be screwed

I mean they already were when they bought a Macbook but no need to rub it in their face

Copy link
Collaborator

@GigiaJ GigiaJ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@GigiaJ GigiaJ merged commit 164415d into OSRSB:master Feb 11, 2022
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

Successfully merging this pull request may close these issues.

None yet

2 participants