Added check for insufficient space #392

iojw · 2016-12-18T13:23:33Z

Addresses #390

The launcher now checks if there's sufficient space in the download directory before downloading the zip from Jenkins. It also sends a warning message that space is running low if the available space is less than twice the size of the zip file.

As for the warning sign, I haven't added it in first as I'm not sure what would be the best approach:

I was thinking about the download icon turning yellow whenever the space is low, but that would involve setting the style using code so I'm not very sure if it's ok. Or would it be better if something like this appeared in the lower left corner as a sign? For the threshold, I'm thinking about maybe when the available space goes below 200MB?

Let me know what you think 🙂

GooeyHub · 2016-12-18T13:23:34Z

Can one of the admins please verify this patch?

iojw · 2016-12-18T15:05:12Z

Took a look into #386 as well and seems it's just a simple issue of the launcher not updating. I should have fixed it by adding the update calls.

Cervator · 2016-12-18T21:26:03Z

@GooeyHub: add to whitelist

@iojw: thanks! @skaldarnar mentioned he'd be able to look at this tomorrow. Feel free to poke at other stuff until then :-)

I personally like the idea of the warning icon appearing somewhere, then if you click it (or hover over?) it could give you some details of the warning.

Maybe an ideal approach would be adding a tooltip summary of the warning as you hover over the warning icon, and if applicable clicking the icon would open some relevant UI element.

Over in engine land with MovingBlocks/Terasology#1402 there is a feature request for something similar, but much bigger, on trying to better indicate in the game UI when warnings or errors have been issued in the log. The launcher has the nice log tab right in the UI. Could be neat to have something similar in the game, with a line per "warning" of some sort, and the icon could open that screen.

GooeyHub · 2016-12-18T21:27:06Z

Hooray Jenkins reported success with all tests good!

iojw · 2016-12-19T05:29:48Z

Cool! based on @Cervator's comments, I managed to come up with this:

Clicking on it takes you to the Log tab, where you can see a logged warning regarding the low space. Also removed the warning dialog that appears when you click the Download button since the warning sign already accomplishes the same role. I was thinking that this could also be extended to other types of warnings as well in the future.

This is my first time with JavaFX so let me know if there's anything wrong with the FXML and all that. If it's alright, I'll still need to add the image source to the credits page 🙂.

GooeyHub · 2016-12-19T05:31:29Z

Hooray Jenkins reported success with all tests good!

rzats

Looks good to me - appreciate the misc. fixes in different files 🙂 The icon should probably be made grayscale to fit with the general look of the launcher.

rzats · 2016-12-19T14:13:54Z

src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java

@@ -501,6 +512,15 @@ private void updateButtons() {
            downloadButton.setVisible(true);
            cancelDownloadButton.setVisible(false);
        }
+
+        // if less than 200MB available
+        if (downloadDirectory.getUsableSpace() <= 200 * 1000000) {


Shouldn't this be 1048576? (1024 * 1024)

I tried to do a bit of research and this seems to be quite a contentious issue. There seem to be 2 units (1 megabyte = 1000 bytes, 1 mebibyte = 1024 bytes) and different platforms seem to use different units (Windows uses mebibytes, Mac uses megabytes). No idea which one would better.... 😐

in any case, we should get rid of the magic numbers (I don't mind the small error induced by mega vs mebibyte). Maybe use something like
private static final int MINIMAL_FREE_SPACE_IN_MB = 200 * 1000 * 1000

You can use something like

private static final long MB = 1024L * 1024; private static final long MINIMUM_FREE_SPACE_IN_MB = 200 * MB;

or just the MB if that's the only location where it is used. If it happens to be used more often, we could aim for a ByteUnit enum containing these constants...

skaldarnar

Your changes look good! I'll merge it real quick and apply the little changes myself 😉
I like the color icon as it brings the attention down there, and it fits with the social media buttons in the bottom...

skaldarnar · 2016-12-19T20:01:08Z

src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java

+        if (downloadDirectory.getUsableSpace() <= 200 * 1000000) {
+            warningButton.setVisible(true);
+            warningButton.setTooltip(new Tooltip(BundleUtils.getLabel("message_warning_lowOnSpace")));
+            logger.warn(BundleUtils.getLabel("message_warning_lowOnSpace"));


The logged warning should contain some more info on how much space is actually left on disk, or how much free space is required to successfully perform the action.

skaldarnar · 2016-12-19T20:04:38Z

src/main/java/org/terasology/launcher/gui/javafx/ApplicationController.java

@@ -501,6 +512,15 @@ private void updateButtons() {
            downloadButton.setVisible(true);
            cancelDownloadButton.setVisible(false);
        }
+
+        // if less than 200MB available
+        if (downloadDirectory.getUsableSpace() <= 200 * 1000000) {


You can use something like

private static final long MB = 1024L * 1024; private static final long MINIMUM_FREE_SPACE_IN_MB = 200 * MB;

or just the MB if that's the only location where it is used. If it happens to be used more often, we could aim for a ByteUnit enum containing these constants...

skaldarnar · 2016-12-19T20:37:48Z

Merged with minimal changes.

iojw · 2016-12-20T04:11:53Z

@skaldarnar: Thanks! 😄

Cervator · 2016-12-28T06:06:07Z

Belated "aw man sweet!" and reminder to please do this for the game itself sometime :D

Added check for insufficient space

be80f01

iojw force-pushed the insufficient_space branch from 0b4fbe1 to be80f01 Compare December 18, 2016 14:39

Fix for MovingBlocks#386

5a59d5c

Added warning sign

a6faa6e

rzats reviewed Dec 19, 2016

View reviewed changes

skaldarnar approved these changes Dec 19, 2016

View reviewed changes

skaldarnar mentioned this pull request Dec 19, 2016

"Installed" in the available versions drop-down does not seem to update without a restart #386

Closed

skaldarnar closed this Dec 19, 2016

skaldarnar mentioned this pull request Dec 21, 2016

Detect free space on drive before Download #390

Closed

skaldarnar added Type: Enhancement New features or noticable improvements. Topic: UI Related to user interface (UI) or design. labels Dec 21, 2016

skaldarnar added this to the v3.1.1 milestone Dec 21, 2016

iojw deleted the insufficient_space branch December 28, 2016 06:28

Cervator mentioned this pull request Dec 30, 2016

Add a button to display the CrashReporter in the main menu MovingBlocks/Terasology#2577

Closed

This was referenced Jan 29, 2017

CrashReporter improvements: Full reporting system? MovingBlocks/Terasology#2765

Closed

Improve visibility of missing resource / asset scenarios MovingBlocks/Terasology#1402

Open

Cervator mentioned this pull request Sep 2, 2017

Initial follow-ups to the new metrics system MovingBlocks/Terasology#3011

Open

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added check for insufficient space #392

Added check for insufficient space #392

iojw commented Dec 18, 2016 •

edited

Loading

GooeyHub commented Dec 18, 2016

iojw commented Dec 18, 2016 •

edited

Loading

Cervator commented Dec 18, 2016

GooeyHub commented Dec 18, 2016

iojw commented Dec 19, 2016

GooeyHub commented Dec 19, 2016

rzats left a comment

rzats Dec 19, 2016

iojw Dec 19, 2016 •

edited

Loading

skaldarnar Dec 19, 2016

skaldarnar Dec 19, 2016

skaldarnar left a comment

skaldarnar Dec 19, 2016

skaldarnar Dec 19, 2016

skaldarnar commented Dec 19, 2016

iojw commented Dec 20, 2016

Cervator commented Dec 28, 2016

Added check for insufficient space #392

Added check for insufficient space #392

Conversation

iojw commented Dec 18, 2016 • edited Loading

GooeyHub commented Dec 18, 2016

iojw commented Dec 18, 2016 • edited Loading

Cervator commented Dec 18, 2016

GooeyHub commented Dec 18, 2016

iojw commented Dec 19, 2016

GooeyHub commented Dec 19, 2016

rzats left a comment

Choose a reason for hiding this comment

rzats Dec 19, 2016

Choose a reason for hiding this comment

iojw Dec 19, 2016 • edited Loading

Choose a reason for hiding this comment

skaldarnar Dec 19, 2016

Choose a reason for hiding this comment

skaldarnar Dec 19, 2016

Choose a reason for hiding this comment

skaldarnar left a comment

Choose a reason for hiding this comment

skaldarnar Dec 19, 2016

Choose a reason for hiding this comment

skaldarnar Dec 19, 2016

Choose a reason for hiding this comment

skaldarnar commented Dec 19, 2016

iojw commented Dec 20, 2016

Cervator commented Dec 28, 2016

iojw commented Dec 18, 2016 •

edited

Loading

iojw commented Dec 18, 2016 •

edited

Loading

iojw Dec 19, 2016 •

edited

Loading