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

Settings: Fixed loading and defaults. #121

Merged
merged 1 commit into from
Oct 11, 2017
Merged

Conversation

madmaxoft
Copy link
Contributor

The "use default location" was not applied when starting Minutor. This change produces more understandable flow of the settings initialization.

Fixes #93, at least on Windows.

@EtlamGit
Copy link
Collaborator

Hi @madmaxoft , as it seems that there are some ambiguities what location to choose in the settings dialog, I see these options for us to improve:

  • write an extra comment to use the Minecraft base folder not the saves folder
  • automatically detect a wrong chosen saves folder and remove it
  • work with both variants, as the saves folder may be named differently anyway

While thinking more about it, I prefer the last option. What do you think?

@madmaxoft
Copy link
Contributor Author

I'm always for making things easier for the end user, so working with both would be my choice. However, it is outside of the scope of this PR, I think.

@mrkite mrkite merged commit 2e757a1 into mrkite:master Oct 11, 2017
@madmaxoft
Copy link
Contributor Author

@EtlamGit How about this behavior:

  • The Settings object always contains the base folder
  • When the user browses for the "Minecraft Location", the code in the Browse button handler inspects the folder they choose. If it is named "saves" and has no "saves" subfolder in it, the code uses the parent folder silently.

This should work for all the cases and prevent any confusion. The only case it wouldn't work would be if the user had a world named "saves" and they chose the saves folder instead of the base folder. But I think it's better to leave it like that.

@EtlamGit
Copy link
Collaborator

(The only save method would be to search for a folder that contains subfolders which themselves contain a file called level.dat that parses to valid NBT data.)

I like your approach...

@mrkite
Copy link
Owner

mrkite commented Oct 17, 2017

My only concern is you don't want the program walking all over the place, recursively checking directories for level.dat. Especially if they point to something really slow like a network drive. So we definitely want to keep the automatic checking to a minimum.

@@ -15,6 +15,10 @@ class Settings : public QDialog {
bool verticalDepth;
QString mcpath;


/** Returns the default path to be used for Minecraft location. */
static QString getDefaultLocation();
Copy link

Choose a reason for hiding this comment

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

@madmaxoft xoft, is this by any chance, a Cuberite-style comment, in minutor?

Copy link

Choose a reason for hiding this comment

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

What is this, some sort of crossover episode?

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.

4 participants