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

Handle unexpected (old or weird) OSes that may not support our directory detection. #3823

Open
wants to merge 4 commits into
base: develop
from

Conversation

@sladyn98
Copy link

sladyn98 commented Jan 13, 2020

Contains

This PR deals with errors thrown during directory detection, specifically giving a chance to run the game again if the default directory detection fails.
In case of headless servers we have defaulted to choosing the home path as the current directory.
Fixes #3566

How to test

Run the game on Windows XP. If you can find something with XP on it or a wierd OS that throws our directory detection out of the window.

Outstanding before merging

If anything. You can use neat checkboxes! Feel free to delete if not needed

  • [ x ] Still have to adjust the wiki doc
    The doc has to be updated if in case we hit any such OSes in the future.
sladyn98 added 2 commits Jan 9, 2020
…sXP. Giving the user a second chance to run the game.

Added a headless server condition to set the homepath as current directory.
@GooeyHub

This comment has been minimized.

Copy link
Member

GooeyHub commented Jan 13, 2020

Can one of the admins please verify this patch?

@Qwertygiy

This comment has been minimized.

Copy link
Contributor

Qwertygiy commented Jan 13, 2020

@GooeyHub add to whitelist.

} catch (IOException ex) {
reportException(e);
System.exit(0);
}
System.exit(0);

This comment has been minimized.

Copy link
@SoniEx2

SoniEx2 Jan 15, 2020

Contributor

shouldn't this be taken out?

This comment has been minimized.

Copy link
@sladyn98

sladyn98 Jan 16, 2020

Author

Yes it should 👍

* Gives user the option to manually choose home path.
* @throws IOException Thrown when required directories cannot be accessed.
*/
public void chooseHomePathManually() throws IOException {

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jan 20, 2020

Member

Should/can this be part of useDefaultHomePath?

This comment has been minimized.

Copy link
@sladyn98

sladyn98 Jan 21, 2020

Author

It could be but then it would have to catch the exception inside the useDefaultHomePath, moreover this provides a much modular approach and a generic solution to all further weird OSeS

@@ -363,7 +364,12 @@ private static void handleLaunchArguments(String[] args) {

} catch (IOException e) {
reportException(e);
System.exit(0);
try {
PathManager.getInstance().chooseHomePathManually();

This comment has been minimized.

Copy link
@skaldarnar

skaldarnar Jan 20, 2020

Member

If this can recover any exception that was thrown before, there is no need to reportException before trying the backup. At least, it should not be reported as an error (maybe as a warning, or probably just on INFO level).

This comment has been minimized.

Copy link
@sladyn98

sladyn98 Jan 21, 2020

Author

Yeah , you have a valid point I just thought that it would be good to report an exception without stopping execution. A Warning sounds like a much better approach :)

@sladyn98 sladyn98 requested a review from skaldarnar Jan 22, 2020
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.

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