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

MacOS: set basepath to executablePath rather than one path above bundle #469

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

palmerj
Copy link

@palmerj palmerj commented Jul 4, 2022

Fixes an issue where the base path would typically be set to /Application if the bundle is installed into /Application/dhewm3.app. Now the default basepath will be /Application/dhewm3.app/Content/MacOS/ where the dhewm3 binary is located.

Fixes an issue where the base path would typically be set to /Application if the bundle is installed into  /Application/dhewm3.app. Now the default basepath will be /Application/dhewm3.app/Content/MacOS/ where the dhewm3 binary is located.
@DanielGibson
Copy link
Member

Is this desirable? I don't know, I'm not a Mac user..
I guess the existing behavior assumed that you have a Doom3 install dir with the game data in it, and copy dhewm3.app into it like you would with dhewm3.exe on Windows

@tomkidd @knev: Any opinion on this?

@knev
Copy link

knev commented Jul 7, 2022

Without really going deep in the code, I guessing this is the functionality you want, yes. The app is just a fluffy wrapper around binary. I run the actual binary when debugging, not the app wrapper, so that I can see the console output.

The save directory on mac is in an unrelated folder to the app. ~/Library/Application Support/.... You can see that code on the few lines directly after the changed line.

I'm pretty sure this is a legit fix, the only question I have is ... what does it fix? What was the problem when it was the bundle path? Was Posix_InitSignalHandlers failing on installing a signal handler or something?

@knev
Copy link

knev commented Jul 7, 2022

As far as I can tell, the exe path doesn't get used in Posix_InitSignalHandlers except if you have libbacktrace installed. So, the issue the OP is referring to is only when tracing with libbacktrace?

@DanielGibson
Copy link
Member

If that really causes a bug with libbacktrace can be tested by building with libbacktrace and then entering the following in the doom3 console: developer 1 then crash.
If it prints a proper backtrace to the console, libbacktrace was initialized correctly

@knev
Copy link

knev commented Jul 7, 2022

I make installed libbacktrace on my system, how do I configure D3_HAVE_LIBBACKTRACE to be set with cmake?

@palmerj
Copy link
Author

palmerj commented Jul 7, 2022

Is this desirable? I don't know, I'm not a Mac user.. but

I'm pretty sure it is, and is comparable to simaiar app bundles I've used i.e OpenJK or RTCW (both quake 3 engine based). Otherwise the current implementation will look for the basepath one directory above the app bundle, and in most cases that's /Application where all *.app bundles are store, which is just wrong on a mac. Even if the app bundle is placed in a non-standard path such as ~/my_games/dhewm3.app that means the basepath for the game data will be ~/my_games and that still doesn't seem ideal.

@DanielGibson
Copy link
Member

I make installed libbacktrace on my system, how do I configure D3_HAVE_LIBBACKTRACE to be set with cmake?

Good question - running CMake should find it automatically, if it's installed systemwide (the compiler can find #include <backtrace.h> and the linker can find -lbacktrace).
If the compiler or linker can't find it without further options, I'm not sure..

If CMake found libbacktrace, it will print "Using libbacktrace" instead of "libbacktrace wasn't found. It's not required but recommended, because it provides useful backtraces if dhewm3 crashes"


So I guess @palmerj wants to put the game data into the dhewm3.app bundle, instead of putting the dhewm3.app bundle into the game data?
Doesn't sound that great - the way it is now you can easily replace dhewm3.app with an updated version without copying gamedata around.
(As far as I understand, you shouldn't put dhewm3.app in my_games/ but in my_games/Doom3/, which should also contain base/ with the .pk4 files)

@palmerj
Copy link
Author

palmerj commented Jul 7, 2022

So I guess @palmerj wants to put the game data into the dhewm3.app bundle, instead of putting the dhewm3.app bundle into the game data?

Correct, and at the moment one of the default's is /Application (in the normal install path case) which makes no sense. If you want to put the data it another place to keep the data and app separate then the following other search path are there too: ~/Library/Application Support/dhewm3/base, and of course you could set an environment variable.

@knev
Copy link

knev commented Jul 8, 2022

I think the first path given by the bundlePath is for debugging purposes. If you install the game normally on the system, I think ~/Library/Application Support/dhwem3 is the correct place for the base files. The base directory also has a config file in it. It would be strange to embed that config file in the app.

@palmerj
Copy link
Author

palmerj commented Jul 8, 2022

I think the first path given by the bundlePath is for debugging purposes. If you install the game normally on the system, I think ~/Library/Application Support/dhwem3 is the correct place for the base files.

Hmm, not if you are using a multi-user OS or want to copy the bundle around in one unit.

The base directory also has a config file in it. It would be strange to embed that config file in the app.

The only config I have is base/default.cfg, the other user ones which get changed by the UI are in ~/Library/Application Support/dhwem3

@knev
Copy link

knev commented Jul 8, 2022

Hmm, not if you are using a multi-user OS or want to copy the bundle around in one unit.

I agree that putting the base files in app bundle is a good option.
Perhaps the best solution would be to add the path inside the app bundle to the search paths for the base files.

@palmerj
Copy link
Author

palmerj commented Jul 8, 2022

I agree that putting the base files in app bundle is a good option.
Perhaps the best solution would be to add the path inside the app bundle to the search paths for the base files.

That would work, but what's wrong with updating the code as proposed. I can't see why we want it the other way.

@knev
Copy link

knev commented Jul 8, 2022

You mean that your patch should be accepted so that you have the option to bundle the base files with the app.
But, all developers lose the ability to have the base directory in dhewm project directory; forming a self-contained project folder.
Sure, that is possible.

It is also possible to fork the repo and build options precisely as you want them.

@palmerj
Copy link
Author

palmerj commented Jul 8, 2022

But, all developers lose the ability to have the base directory in dhewm project directory; forming a self-contained project folder.

If you are referring to ~/Library/Application Support/dhewm3 as the dhewm project directory then this PR still provides that:

Current search path:
/Users/xxxx/Library/Application Support/dhewm3/base
/Applications/Doom3.app/Contents/MacOS/base

It is also possible to fork the repo and build options precisely as you want them.

Sure I've already done that. Just thought this might be what most MacOS users really want.

@knev
Copy link

knev commented Jul 9, 2022

If you are referring to ~/Library/Application Support/dhewm3 as the dhewm project directory then this PR still provides that:

Current search path:
/Users/xxxx/Library/Application Support/dhewm3/base
/Applications/Doom3.app/Contents/MacOS/base

You are assuming that the app is in the /Applications folder. During development, it is built in its project folder. That is the one I am referring to, which will be lost; dhewm3.git/base. All develops would have to either copy base to Library/Application Support/dhewm3/base or to dhewm3.git/dhewm3.app/Contents/MacOS/base; the latter of which is absurd during development. If you are making changes to base, then a developer would constantly have to work in to directories and also lose the ability to just zip the project folder for a complete backup.

@palmerj
Copy link
Author

palmerj commented Jul 9, 2022

During development,

But are bundles really for dev? I doubt it - they can config and run stuff however. Bundles are for distribution for end users.

@knev
Copy link

knev commented Jul 9, 2022

But are bundles really for dev? I doubt it - they can config and run stuff however. Bundles are for distribution for end users.

I'm done discussing.

@palmerj
Copy link
Author

palmerj commented Jul 9, 2022

@DanielGibson do you have view on if this can be merged? I think @knev's comments above more come from being a developer and not a user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants