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

Check for diablo.ini in PWD, if found use that and save files into ./ for portable mode. #1736

Merged
merged 2 commits into from
May 15, 2021

Conversation

Trihedraf
Copy link
Collaborator

No description provided.

@Trihedraf Trihedraf requested a review from AJenbo April 27, 2021 23:49
Source/utils/paths.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@julealgon julealgon left a comment

Choose a reason for hiding this comment

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

Totally against this idea. It all sounds like a big hack to me. IMHO, we are already doing the correct thing to do and shouldn't make any changes to it.

@Trihedraf
Copy link
Collaborator Author

We already check for the mpqs in multiple places. Why can’t we do that for the ini and saves so that it can be setup to run out of a single contained folder without having to use to shortcuts or scripts.

@AJenbo
Copy link
Member

AJenbo commented Apr 28, 2021

I knew looking for multiple files in multiple folder would be a slippery slope :D

The reason not to do things like that is that it's actually caused an issue with how the game loads assets directly from disk (as opposed to an MPQ). It makes things more complicated and complicated things have bugs. So that has to be weighted with the benefit to the user. But quite a lot of Windows users are surprised when they dump a new exe in their existing diablo folder and it doesn't load files from the same location, so there are some reasonable benefits to attempting to figure out the users intent.

@Trihedraf Trihedraf changed the title Check for diablo.ini in PWD, if found use that and save files into ./save for portable mode. Check for diablo.ini in PWD, if found use that and save files into ./ for portable mode. Apr 28, 2021
@Trihedraf Trihedraf force-pushed the PWD-ini branch 2 times, most recently from d988747 to 16ee90f Compare April 28, 2021 02:59
@galaxyhaxz
Copy link
Member

Great work @Trihedraf! I've been saying this should have been a thing for a long time now. It's especially useful as now you can have DevilutionX on a flash drive and play it at school or another persons computer where you may not have access/permission to get into the hidden user directory.

@AJenbo
Copy link
Member

AJenbo commented Apr 28, 2021

You could already do that just with a shortcut instead of an ini, this just makes it detect if it's in an original installation with write access.

@galaxyhaxz
Copy link
Member

You could already do that just with a shortcut instead of an ini, this just makes it detect if it's in an original installation with write access.

More streamlined and easier now though ;)

Source/utils/paths.cpp Outdated Show resolved Hide resolved
Source/utils/paths.cpp Outdated Show resolved Hide resolved
@malvarenga123
Copy link

It's as you say @AJenbo, it's quite annoying to have those files in different places. This makes it much easier to run have multiple versions of the exe in the same folder.

Source/storm/storm.cpp Outdated Show resolved Hide resolved
@Trihedraf Trihedraf force-pushed the PWD-ini branch 3 times, most recently from 0efc993 to 4cf7964 Compare May 2, 2021 09:26
Source/utils/file_util.cpp Outdated Show resolved Hide resolved
Source/utils/paths.cpp Outdated Show resolved Hide resolved
@AJenbo
Copy link
Member

AJenbo commented May 2, 2021

Nice, this is now clean enough that it's not an issue to include, compared to the benifit.

Source/utils/file_util.cpp Outdated Show resolved Hide resolved
@Trihedraf
Copy link
Collaborator Author

@AJenbo is there something that we are waiting for on this one?

@AJenbo
Copy link
Member

AJenbo commented May 5, 2021

Not much, I just need time to test it :)

There has been a lot to see to as of late, so haven't had as much time to review and merge PRs as I would have liked, but will probably get to this one this week.

@Trihedraf
Copy link
Collaborator Author

@AJenbo Done

@Trihedraf Trihedraf force-pushed the PWD-ini branch 2 times, most recently from 2f34e74 to 1989f43 Compare May 11, 2021 00:46
Source/utils/file_util.cpp Outdated Show resolved Hide resolved
Source/utils/file_util.cpp Outdated Show resolved Hide resolved
@Trihedraf Trihedraf force-pushed the PWD-ini branch 5 times, most recently from 9e006df to 0b4d01f Compare May 11, 2021 02:47
@Trihedraf Trihedraf requested review from glebm and AJenbo May 11, 2021 03:02
Source/utils/file_util.cpp Outdated Show resolved Hide resolved
Source/utils/file_util.h Outdated Show resolved Hide resolved
Source/utils/file_util.cpp Outdated Show resolved Hide resolved
@Trihedraf Trihedraf requested review from AJenbo and removed request for AJenbo May 13, 2021 06:16
@qqkookie
Copy link

qqkookie commented May 14, 2021

How about checking $PWD\SAVE\diablo.ini first and then checking $PWD\diablo.ini next? I disliked multiple save files scattered in game directory itself as original Diablo did. Please use separate subdirectory of $PWD for diablo.ini and save files and keep the game install directory clean and tidy.

I suggest to check "./SAVE" first and if it does not exist or not writable then use `"./" as perfPath and configPath.

@Trihedraf
Copy link
Collaborator Author

I originally had it put all the saves in a ./save folder, but @AJenbo said to take it out.

@AJenbo
Copy link
Member

AJenbo commented May 15, 2021

Hopefully this won't cause to many issues for players that extracted DevilutionX to there original diablo folder but have there save games in the current pref path.

@AJenbo AJenbo merged commit de3f306 into diasurgical:master May 15, 2021
@Trihedraf Trihedraf deleted the PWD-ini branch May 15, 2021 13:15
prefPath = FromSDL(SDL_GetPrefPath("diasurgical", "devilution"));
if (FileExistsAndIsWriteable("diablo.ini")) {
prefPath = "./";
Copy link
Member

Choose a reason for hiding this comment

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

Apparently this doesn't work on GCC 5.5

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sent PR to fix #1984

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

7 participants