Attnam / ivan

Iter Vehemens ad Necem - a continuation of the graphical roguelike by members of http://attnam.com
GNU General Public License v2.0
301 stars 43 forks source link

Bone and high score files #488

Closed ghost closed 5 years ago

ghost commented 5 years ago

Hi! Thanks for the new release! :alien: I have created and maintain ivan package for Nix/NixOS. Due to the nature of Nix, all files created by the application must be stored inside the user's home directory. The package itself is stored in the Nix store and its contents is controlled solely by the Nix. Thus, it was impossible to save bone and high score files. I had to make a patch to store bone and high score files in ~/.ivan of the current user. Unfortunately, this worked for 052 and 053. Since this release (054) it does not work. I'll have to figure out what's changed and make a new patch :disappointed: Could you consider to store bone files and high scores files inside the user's home folder in future versions? At least optional. That would be very nice :smile: Besides, there are other operating systems with similar restrictions, such as GuixSD. Anyway, thanks in advance and have a nice day! :wink:

ryfactor commented 5 years ago

Hi @freepotion! Is this in any way related to #221 and #309 #310 Lots of changes were made since the last release. @AquariusPower might have some better idea of what is going on. If there is anything you would like to patch into IVAN to make this problem go away permanently, then please feel free to create the PR :)

AquariusPower commented 5 years ago

@freepotion

hi!

I looked for this everywhere in the sources and cmake files: LOCAL_STATE_DIR

here:

./CMakeLists.txt:28: -DLOCAL_STATE_DIR="${CMAKE_INSTALL_FULL_LOCALSTATEDIR}/ivan")

but configuring that would not provide dynamic user home.

so I think this is better:

festring game::GetStateDir()
{
#ifdef UNIX
#ifdef MAC_APP
  return GetHomeDir();
#else
#ifdef FORCE_HOME_AS_STATE_DIR
  return GetHomeDir();
#else
  return LOCAL_STATE_DIR "/";
#endif
#endif
#endif

#if defined(WIN32) || defined(__DJGPP__)
  return GetHomeDir();
#endif
}

and add this to INSTALL instructions: To force score and bones' files at user $HOME path, you can add this flag '-DFORCE_HOME_AS_STATE_DIR', like this: CMAKE_CXX_FLAGS="-DFORCE_HOME_AS_STATE_DIR -DWIZARD" (you may chose what flags to add independently of each other).

then add something like this during compilation when calling cmake -DCMAKE_CXX_FLAGS="-DFORCE_HOME_AS_STATE_DIR"

I think it will work, if dont, say so, a PR would be nice :)

PS.: $HOME/.ivan is for personal configuration files/saves etc, while bones and scores could be shared between many users, but I see no problem on patching instead like:

#ifdef FORCE_HOME_AS_STATE_DIR
  return GetHomeDir()+"/.ivan/";

it could even be an override not specific to UNIX therefore put on top of game::GetStateDir() being a more generic solution if any thing change later allowing other than HOME to MacOSX and Windows (may be android?).

ghost commented 5 years ago

@fejoa @AquariusPower Thanks a lot, I have to try! :alien: By the way, it looks like -DWIZARD flag no longer exists. May I ask you one more thing? I wanted to add support for other platforms (platform.unix instead platforms.linux). Thus, Nix will be able to build the package on other platforms, primarily on Mac. So if I want FORCE_HOME_AS_STATE_DIR option to work for all unix platforms, I have to add a definition for it after add_definitions(-DUNIX) in CMakeLists.txt? And to swap #ifdef FORCE_HOME_AS_STATE_DIR and #ifdef MAC_APP in your version of festring game::GetStateDir()?

AquariusPower commented 5 years ago

By the way, it looks like -DWIZARD flag no longer exists.

did you have not the expected result while using -DWIZARD ? run this on src root to see how often it is still used :) grep "#ifdef WIZARD" * -rnI

I wanted to add support for other platforms (platform.unix instead platforms.linux). Thus, Nix will be able to build the package on other platforms, primarily on Mac.

may be you could try the pre-compiled/released mac one first? if I am not wrong, it has the required libs bundled, but.. I never used Mac.

So if I want FORCE_HOME_AS_STATE_DIR option to work for all unix platforms, I have to add a definition for it after add_definitions(-DUNIX) in CMakeLists.txt? And to swap #ifdef FORCE_HOME_AS_STATE_DIR and #ifdef MAC_APP in your version of festring game::GetStateDir()?

just do this to be a generic overrider, put the new code on the top of the function:

festring game::GetStateDir()
{
#ifdef FORCE_HOME_AS_STATE_DIR
  return GetHomeDir()+"/.ivan/";
#endif

// so the code that is already there committed  will just continue existing but below here

The command you prepare to compile Ivan using cmake, you can just set FORCE_HOME_AS_STATE_DIR as specified in the new INSTALL instructions.

After it completes you prepare the Nix package (that's how I guess is it's packages preparation flow at least).

What I mean is, you will compile it as anyone would, may be even install it (here I can use checkinstall that tracks the installation and creates a .deb package), and after that you create the Nix package.

But not at CMakeLists.txt, neither force UNIX there as the PR (to commit here) would break windows compilation (despite I am not cmake expert and so cant give other options/possibilities related to implementing on it).

If you prefer to just implement it, it will be easier to we pin point whatever needs fixing/changing directly on the code :)

ghost commented 5 years ago

@AquariusPower

did you have not the expected result while using -DWIZARD ? run this on src root to see how often it is still used :) grep "#ifdef WIZARD" * -rnI

My bad, it's okay :expressionless:

may be you could try the pre-compiled/released mac one first? if I am not wrong, it has the required libs bundled, but.. I never used Mac.

Actually, neither am I. I just wanted the package to be available for this platform. But at the moment I decided to leave it as is, only for Linux :smiling_imp:

After it completes you prepare the Nix package (that's how I guess is it's packages preparation flow at least). What I mean is, you will compile it as anyone would, may be even install it (here I can use checkinstall that tracks the installation and creates a .deb package), and after that you create the Nix package.

Actually, the package is stored here. This nix expression defines the package. Plus definition here is to tell Nix about the package. Then build farm based on the continuous integration system Hydra build different binary packages. :sunglasses:

But not at CMakeLists.txt, neither force UNIX there as the PR (to commit here) would break windows compilation (despite I am not cmake expert and so cant give other options/possibilities related to implementing on it).

I've created pull request, which fully corresponds to my PR to nixpkgs upstream. Although I added the option to the CMakesList.txt, it will be only used for unix platforms, I took care of it. (there were some problems with passing multiple CMAKE_CXX_FLAGSflags). This option is disabled by default, so nothing will change without the corresponding flag.

Unfortunately, the music still doesn't work in-game, and the reason is unknown (the sound generally works) :expressionless: I mean, this has been happening since the package was created (version 052). Here's some output: ALSA lib pcm.c:8495:(snd_pcm_recover) underrun occurred

AquariusPower commented 5 years ago

the music still doesn't work in-game

I use Qsynth (fluidsynth) and had to download a package and setup this on it: /usr/share/sounds/sf2/FluidR3_GM.sf2

If I start Qsynth after the game, the music wont play. If I mess too much with Qsynth, the music will stop playing. If I just open and close the ivan cfg in-game, it will fix the music volume if not correct.

ghost commented 5 years ago

Thanks a lot! I'll try! :smiley: I think this issue can be closed. I hope you don't mind if I open a new one in the case of the new questions. :alien: