lotharsm / devilutionx-snapcraft

Snap package for DevilutionX
3 stars 1 forks source link

Unclear where to put diablo files #2

Open zyga opened 4 years ago

zyga commented 4 years ago

Hey

I just installed your snap. I have the data from the GOG copy of the game extracted and available. Can you please indicate where the data should be placed?

I'm a snapd developer so I can also help you make this experience nicer.

lotharsm commented 4 years ago

You only need the diabdat.mpq file. I assume you already were able to extract it?

In this case, please simply copy the diabdat.mpq file to the directory ~/snap/devilutionx/current/.local/share/diasurgical/devilutionx as indicated on https://snapcraft.io/devilutionx in the "Usage" section :)

Regarding making the experience nicer: The main probleme here is that the path were devilutionx looks for the diabdat.mpq file is currently hard coded, it has to be either the .local/share directory or the directory where the executable itself is located - which is unfortunately not possible in this case.

zyga commented 4 years ago

Ah.

Is that configurable? The $SNAP_USER_DATA directory is copied automatically on each refresh. It would be more efficient to use $SNAP_USER_COMMON (aka ~/snap/$SNAP_NAME/common) as that is a perfect fit for this kind of file (large and compatible across revisions).

Thanks, trying now.

zyga commented 4 years ago

Could we simply have a patch in the code to understand the snap packaging or make it configurable in some other way? Ideally if $SNAP_USER_COMMON is defined it would be enough to make the game work.

As a part of another big development in snapd, I'm working on making the presentation of user-facing snap data nicer. I would be great if one could simply have a ~/Snap/devilutionx/game-data or similar (in the end).

lotharsm commented 4 years ago

Is that configurable? The $SNAP_USER_DATA directory is copied automatically on each refresh. It would be more efficient to use $SNAP_USER_COMMON (aka ~/snap/$SNAP_NAME/common) as that is a perfect fit for this kind of file (large and compatible across revisions).

I can try to use the layout and organize features in the snapcraft.yaml file to map the diabdat.mpq file to the location you proposed, I'm not entirely sure if the executable will then reference it correctly - looks like I have something to play around now :)

zyga commented 4 years ago

Organize is only useful at build time. Layouts might be useful but cannot (yet) be relative to $SNAPUSER* variables.

zyga commented 4 years ago

One other silly idea is to have a symbolic link next to the game data, that points to /var/snap/$SNAP_NAME/diabdat.mpq and ask the user to put the files there (there's also the advantage of having only one copy per system)

zyga commented 4 years ago

One other simple idea is to set XDG_DATA_HOME to something else. It looks like .local/share/diasurgica is defined this way

lotharsm commented 4 years ago

One other silly idea is to have a symbolic link next to the game data, that points to /var/snap/$SNAP_NAME and ask the user to put the files there (there's also the advantage of having only one copy per system)

Indeed, that's a nice idea. In my opinion, it has the downside of the data being located outside of the users homedir on a location that requires root privileges, so you can't simply drag and drop the diabdat.mpq from the original game CD. But that's mainly my personal preference.

zyga commented 4 years ago

Yeah, I think the XDG idea is easier for users. You can set it via environment section:

environment:
  XDG_DATA_HOME: $SNAP_USER_COMMON
lotharsm commented 4 years ago

Yeah, I think the XDG idea is easier for users. You can set it via environment section:

environment:
  XDG_DATA_HOME: $SNAP_USER_COMMON/

Wow, great. I'll try this.

zyga commented 4 years ago
Zrzut ekranu 2020-05-20 o 21 47 41
zyga commented 4 years ago

If you can patch the upstream sources, the error message in absence of the right file could also instruct them where to put it.

lotharsm commented 4 years ago

Yes, the string is hard coded into one of the files, so I can basically patch this "on the fly" at build time with a simple sed. I wasn't sure if patching the code is a nice thing to do :)

zyga commented 4 years ago

Or by applying a .patch but yeah. It would be great if you could have a patch that detects snap runtime (presence of SNAP_USER_COMMON) and upstream both the patch and the snapcraft.yaml itself.

lotharsm commented 4 years ago

Yes, submitting it upstream is the ultimate goal. However, I currently have not enough C++ experience to check if the snap runtime/environment variable is present in the code without applying a shell wrapper.

I'm currently trying the XDG_DATA_HOME method and try to at least patch the error message "on the fly" to hard code it to the new location.

zyga commented 4 years ago

It should be super easy in C/C++

if (getenv("SNAP_USER_COMMON") != NULL) {
  /* snap mode */
} else {
  /* vanilla mode */
}
lotharsm commented 4 years ago

That's neat! Once I get everything up and running and after gathering a little bit more feedback, I'll try to upstream everything. Working on the XDG_DATA_HOME method now.

lotharsm commented 4 years ago
    environment:
      XDG_DATA_HOME: $SNAP_USER_COMMON

seems not to work - the variable is ignored, and the game is still looking for diabdat.mpq at the old location.

lotharsm commented 4 years ago

Wait, I found something in the code: "--data-dir", "Specify the folder of diabdat.mpq"

zyga commented 4 years ago

Then change the command: to pass --data-dir $SNAP_USER_COMMON please. That should do it.

lotharsm commented 4 years ago

It works, it now accepts /snap/devilutionx/common as location for the diabdat.mpq file \o/

zyga commented 4 years ago

Superb! Push it

You can also make it automatically move the data. The startup program can be a script that does the mv for the user. It's totally optional as this snap is still in edge and has few users.

lotharsm commented 4 years ago

I just pushed it (see https://github.com/lotharsm/devilutionx-snapcraft/commit/4794b0b013dffeccc759415bb3a64bcc24e522c3), rebuilding is currently in progress :) Yes, writing a small shell wrapper that moves the file seems like a good idea, but as you pointed out, it's maybe not that important at this point in time - I better invest my time to patch the error message and do some more extensive playing testing.

zyga commented 4 years ago

Haha, don't forget to edit the snapcraft store page and make some nice screenshots!

lotharsm commented 4 years ago

Bildschirmfoto von 2020-05-20 23-16-34

Proof of concept. The way I'm applying the patch during the build is most likely violating each and every snapcrafters coding style. :P

zyga commented 4 years ago

Snapcraft is practical. If it works it works. Thanks for making the snap!

pizzadude commented 4 years ago

@lotharsm Thanks for making this snap! Works perfectly in Fedora 32 KDE.

AJenbo commented 4 years ago

@lotharsm hi, DevilutionX maintainer here.

The path isn't actually hardcoded, it's determined at runtime by SDL, and as you have found out it can be overwritten via a parameter :)

Instead of having this a separate project, it would be nice if you could submit the config in a PR for the project itself. Also, don't worry about C++ we are happy to work together and make adjustments where needed for various systems.

lotharsm commented 4 years ago

HI @AJenbo, thanks a lot for your feedback! Right, I learned about the parameter magic a little bit too late :)

Merging this back to the project was the plan from the beginning - as soon as I'm able to build a proper "stable" release (currently, as soon as I check out a git tag, compilation breaks due to some path mismatches - I have to investigate this. After that, I'll surely open a PR.

It's also great to hear that you are open to changes like modifying the error message indicating a missing DIABDAT.MPQ - indeed it would be great if I don't have to patch the sources that way anymore :)

ghost commented 3 years ago

Sorry to post but I am on Manjaro KDE (Arch) 20.2 but putting diabdat.mpq in location mentioned still isn't working for me says cannot find file. I am using GOG version. Also if I have Hellfire where do I put that ? My GOG version comes with Hellfire...

Anybody got it working ?

AJenbo commented 3 years ago

Hellfire support isn't finished yet and isn't available via the snap build, instead, you need to build it your self: https://github.com/diasurgical/devilutionX#building-from-source (see CMake build options for how to enable Hellfire support)

ghost commented 3 years ago

Ok then how do I make Diablo itself work I get file not found error even though I put mpq exactly where am supposed to xD

Shortcut for me says : env BAMF_DESKTOP_FILE_HINT=/var/lib/snapd/desktop/applications/devilutionx_devilutionx.desktop /var/lib/snapd/snap/bin/devilutionx

lotharsm commented 3 years ago

You need to copy the diabdat.mpq file (all lowecase) in the ~/snap/devilutionx/common/ directory.

ghost commented 3 years ago

You need to copy the diabdat.mpq file (all lowecase) in the ~/snap/devilutionx/common/ directory.

I have done that that's what am saying still no go...

lotharsm commented 3 years ago

I have done that that's what am saying still no go...

Sorry, can't replicate. Maybe your diabdat.mpq got corrupted?

ghost commented 3 years ago

I have done that that's what am saying still no go... Sorry, can't replicate. Maybe your diabdat.mpq got corrupted?

Nope it's from a brand new install. I just cut n paste to correct location keep getting that no file found message...

https://i.imgur.com/ipohuxp.png

Shortcut for me says : env BAMF_DESKTOP_FILE_HINT=/var/lib/snapd/desktop/applications/devilutionx_devilutionx.desktop /var/lib/snapd/snap/bin/devilutionx

AJenbo commented 3 years ago

What is the checksum from running md5sum ~/snap/devilutionx/common/diabdat.mpq

Do you have the same issue when using a regular release?