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

Jump to bottom

devilutionx: init at unstable-2019-07-28 #65567

Merged
merged 1 commit into from Aug 6, 2019
Merged

devilutionx: init at unstable-2019-07-28 #65567

merged 1 commit into from Aug 6, 2019

Conversation

karolchmist
Copy link
Member

@karolchmist karolchmist commented Jul 29, 2019 •
edited

Motivation for this change

This PR adds DevilutionX, a build of Diablo game from 1996 for modern operating systems.

To make it run, a file with original data from the game (diabdat.mpq) must be placed in ~/.local/share/diasurgical/devilution. In future releases of DevilutionX, this is planned to be configurable. We could make it a parameter as well when possible.

This PR builds a recent master commit, because since the last release (0.4.0) there have been a lot of improvements for 64 bit build. I've managed to build and run the game on a 64 bit NixOs and play it quite well.

As soon a next version is released, I plan to keep DevilutionX track the official releases.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

aanderse
aanderse requested changes Jul 30, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

Thanks for contributing this package! I took a quick scan over your .nix file and left a few suggestions I hope you find useful. I didn't dig into the actual project itself or try to build it, so if I've made any mistakes or missed anything I apologize.

pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/building-devilutionx-as-beginner-cmake-doesnt-find-libsodium/3387/6

@karolchmist
Copy link
Member Author

Thank you @aanderse for you code review, very helpful. I've pushed fixes for the issues that you have found, and commented the addition of the NIX_CFLAGS_COMPILE flag.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/building-devilutionx-as-beginner-cmake-doesnt-find-libsodium/3387/7

aanderse
aanderse requested changes Aug 5, 2019
View reviewed changes
Copy link
Member

@aanderse aanderse left a comment

Choose a reason for hiding this comment

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

I have added a few minor comments. I'll try building and running and then we're good to merge. Thanks again!

pkgs/games/devilutionx/default.nix Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Outdated Show resolved Hide resolved
pkgs/games/devilutionx/default.nix Show resolved Hide resolved
@karolchmist
Copy link
Member Author

Ok, it passes for Darwin I guess? Is eval really building it? And for Windows, will it be built once merged?

@aanderse
Copy link
Member

aanderse commented Aug 6, 2019 •
edited

@GrahamcOfBorg build devilutionx

@aanderse aanderse changed the title devilutionx: init at 0.5.0-SNAPSHOT devilutionx: init at unstable-2019-07-28 Aug 6, 2019
@aanderse
Copy link
Member

aanderse commented Aug 6, 2019

@karolchmist Squash all commits into a single commit with init at unstable-2019-07-28 and you have yourself a merge 🎉 Thanks again!

@karolchmist
Copy link
Member Author

Done! Thank you @aanderse , you were very helpful :)

@aanderse aanderse merged commit 1857824 into NixOS:master Aug 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aanderse aanderse aanderse approved these changes

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@karolchmist @nixos-discourse @aanderse