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

(deprecated) stardrop: init at 1.2.1 #338661

Closed
wants to merge 8 commits into from
Closed

Conversation

jh-devv
Copy link

@jh-devv jh-devv commented Aug 31, 2024

Description of changes

Stardrop is an open-source, cross-platform mod manager for the game Stardew Valley.
https://floogen.gitbook.io/stardrop/

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.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.

@jh-devv jh-devv marked this pull request as draft August 31, 2024 17:57
pkgs/by-name/st/stardrop/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/st/stardrop/package.nix Outdated Show resolved Hide resolved
pkgs/top-level/all-packages.nix Outdated Show resolved Hide resolved
pkgs/by-name/st/stardrop/package.nix Outdated Show resolved Hide resolved
@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

I have not yet pushed the changes but I have added an FHS environment to the package. This was recommended on the Matrix channel since it launches a game.

@SigmaSquadron
Copy link
Contributor

SigmaSquadron commented Sep 1, 2024

I have not yet pushed the changes but I have added an FHS environment to the package. This was recommended on the Matrix channel since it launches a game.

I saw the FHS you posted on Matrix. My only nit with it is that you should avoid using the pname variable everywhere, and hardcode executable names instead. Overriding the pname shouldn't alter the final derivation.

@jh-devv jh-devv marked this pull request as ready for review September 1, 2024 15:03
Copy link
Contributor

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

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

Does this really not work without an FHS env? We're building the thing from source here, surely we can patch as needed

@eclairevoyant
Copy link
Contributor

Also, please rebase and squash everything into the first commit

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

Also, please rebase and squash everything into the first commit

Yikes, didn't mean to do that, sorry!

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

Does this really not work without an FHS env? We're building the thing from source here, surely we can patch as needed

Haven't looked into the code but I suspect heavily it does not.

@jh-devv jh-devv force-pushed the stardrop branch 2 times, most recently from 8935251 to bae055b Compare September 1, 2024 16:06
@jh-devv jh-devv force-pushed the stardrop branch 2 times, most recently from 765ef26 to 8a9cf40 Compare September 1, 2024 19:35
@eclairevoyant
Copy link
Contributor

I suspect heavily it does not.

Please try. We really want to avoid FHS envs.

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

I suspect heavily it does not.

Please try. We really want to avoid FHS envs.

Oh, well, I'll take a look at the code of the application and the linking of the binary! 👍

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

Without FHS, the application works, you can't run the game though. I suspect this is because the game is linked against FHS paths.

❯ readelf -a Stardew\ Valley | grep "/" 

  OS/ABI:                            UNIX - GNU
      [Requesting program interpreter: /lib64/ld-linux-x86-64.so.2]
  Tag        Type                         Name/Value
 0x000000000000000f (RPATH)              Library rpath: [$ORIGIN/netcoredeps]

You can really patch the game, without the updates braking and stuff like that. Because of this, it cannot run in an non FHS environment without patching the game, which is not even in the slightest sense practical.

@eclairevoyant
Copy link
Contributor

The FHS env is only going to apply to the application being built here, how is the game relevant?

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

The FHS env is only going to apply to the application being built here, how is the game relevant?

Because, it is an mod manager and a launcher for the game. You can click a button to launch the game.

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

You could launch it from steam (since, it just manages the mod files) and SMAPI (the mod loader) is already patched in. But then you lose functionality like better logs, priority mods etc.

@eclairevoyant
Copy link
Contributor

and a launcher for the game.

This was not mentioned at all in the original description. Now this whole rigmarole makes sense.

@jh-devv
Copy link
Author

jh-devv commented Sep 1, 2024

and a launcher for the game.

This was not mentioned at all in the original description. Now this whole rigmarole makes sense.

Oh, yeah, sorry.... but yeah... You don't have to use it as a launcher, but you can. I for example, don't.

@poperigby
Copy link
Contributor

Anything else need to be done to get this merged?

@jh-devv jh-devv closed this Oct 15, 2024
@jh-devv jh-devv deleted the stardrop branch October 15, 2024 22:12
@jh-devv jh-devv restored the stardrop branch October 15, 2024 22:12
@jh-devv jh-devv reopened this Oct 15, 2024
@jh-devv
Copy link
Author

jh-devv commented Oct 15, 2024

Silly me :3 Accidentally deleted the branch lol, it's back up now!

@jh-devv jh-devv closed this by deleting the head repository Oct 22, 2024
@jh-devv
Copy link
Author

jh-devv commented Oct 23, 2024

Some problems with the fork, i'll get it back this evening!

@jh-devv jh-devv changed the title stardrop: init at 1.2.1 (deprecated) stardrop: init at 1.2.1 Oct 23, 2024
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.

4 participants