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

the-dark-mod: init at 2.12 #356578

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Conversation

pluiedev
Copy link
Contributor

@pluiedev pluiedev commented Nov 16, 2024

One day I have to ask myself why I have such a penchant for spending days on packaging retro video games

I'm not super confident that the game assets are 100% reproducible due to how the installer works — try building it yourself and see if it works

Closes #300213

Depends on #361114

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@pluiedev pluiedev force-pushed the push-ysuuzkmulrnk branch 9 times, most recently from a0789d1 to 9b849f4 Compare November 18, 2024 08:41
Copy link
Contributor

@ambroisie ambroisie left a comment

Choose a reason for hiding this comment

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

Should probably use lib.sourceTypes.binaryNativeCode for its meta.sourceProvenance.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd just make this a sub-file from the-dark-mod-unwrapped and make it passthru.assets.

I think that's the more common pattern, but I speak mostly from an experience of packages needing to i.e: build a webui.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For games? Not necessarily, since often times assets have to be requireFile'd from an installer and so are usually made separate

Comment on lines 13 to 37
# We use the SVN repo as it contains built static binaries that are not
# present in the Git mirror. Currently Nixpkgs only supports a limited set
# of packages that can be statically linked (notably excluding ffmpeg, pulseaudio
# and pipewire), so we couldn't build everything from scratch even if we want to :(
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand this: can't we just make them buildInputs and/or link them into the expected place in the repo?

Copy link
Contributor Author

@pluiedev pluiedev Nov 18, 2024

Choose a reason for hiding this comment

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

No — it specifically expects statically-linked libraries (.a) which Nixpkgs has a very limited support for right now: PulseAudio, Pipewire and libav are all marked broken when evaluated under pkgsStatic. I did manage to get some of them working including patching OpenAL, and some of them already have prebuilt statically-linked libraries, but trying to coerce FFmpeg into building statically is a rabbithole I really don't want to get down right now.

I'm open to devendoring them when we have support for static linking for all of these dependencies.

Comment on lines 36 to 38
gameDir=''${XDG_DATA_HOME:-$HOME/.local/share}/darkmod
mkdir -p "$gameDir"
lndir -silent ${the-dark-mod-assets} "$gameDir"
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the assets are updated (or a file gets removed): do stale links linger?

Copy link
Contributor Author

@pluiedev pluiedev Nov 18, 2024

Choose a reason for hiding this comment

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

Any updated files will be re-linked but old files won't be purged. I think what we have to do is remove all symlinks that belonged to the last version of assets first, like what I've done for nzportable

export ALSOFT_CONF=$gameDir/alsoft.ini

${lib.getExe the-dark-mod-unwrapped} \
+set fs_basepath "$gameDir" \
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is for the assets? If so, why can't it just directly point to the nix store and instead need linking into XDG_DATA_HOME?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It really wants to write things directly next to itself, or to the game data folder. Internal comments indicate that fs_gamepath is supposed to be read-only but that's just not the case after very extensive testing

(I've genuinely wasted ~3-4 hours trying to coax it into working. It just doesn't.)

Copy link
Contributor

Choose a reason for hiding this comment

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

Would upstream be receptive to a bug report (definitely not a blocker for merging the package)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Nov 19, 2024
Copy link
Member

@keenanweaver keenanweaver left a comment

Choose a reason for hiding this comment

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

image

Missions download & play as intended. Thanks for packaging TDM!

@pluiedev
Copy link
Contributor Author

pluiedev commented Dec 2, 2024

Note to committers: this PR would likely have to depend on a PR I'll make that unifies the pattern of "program wants writable working directory that also contains all the assets" pattern that I've seen in at least three games I've packaged now. It's simply too common and annoying to deal with to justify having a bespoke solution for each package...

@pluiedev pluiedev marked this pull request as draft December 2, 2024 14:46
@pluiedev pluiedev mentioned this pull request Dec 2, 2024
13 tasks
@pluiedev pluiedev force-pushed the push-ysuuzkmulrnk branch 2 times, most recently from b341e66 to ca2dd4e Compare December 2, 2024 17:54
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-darwin: 1 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Dec 2, 2024
@github-actions github-actions bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Dec 4, 2024
@pluiedev
Copy link
Contributor Author

pluiedev commented Dec 4, 2024

I don't exactly know why but now TDM complains about missing savegames despite me nuking the game directory multiple times and it doesn't seem to keep track of all the savegames I've made... No idea if it would happen elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: The Dark Mod
3 participants