search

djanatyn / ssbm-nix

Nix expressions for Super Smash Bros. Melee players.
31 stars 17 forks source link

[WIP] Tests, slippi patch, shields, readme, others #2

Closed 6AA4FD closed 3 years ago

6AA4FD commented 3 years ago

This is my next feature branch, I am working on

I could use your input on

I also learned something from the slippi discord today, they sometimes build the '$version' appimage from HEAD, something to be aware of for future updates, even if the .appimage on the tag is reported working, the source at the tag may not be.

6AA4FD commented 3 years ago

@djanatyn don't review quite yet, i have a build running that will get the new user.json method working and update the readme.

6AA4FD commented 3 years ago

@djanatyn Alright, slippi is now working fully (except on wayland, the gecko code submenu is a little bit hard to use). Mind the instructions about -u ~/.config/slippi-netplay or using the desktop item, you won't be able to login otherwise. If you want to just merge the slippi changes I can make a seperate branch or isolate a commit for you to cherry pick.

djanatyn commented 3 years ago

Take as much time as you'd like with this feature branch. A few comments:

the 2.2.5 stable version is broken for linux

I pushed a new version to master that I've been using to play: https://github.com/djanatyn/ssbm-nix/commit/4c21ac47f3864b9231d0b821044e9c11cde479fa

commit 4c21ac47f3864b9231d0b821044e9c11cde479fa (HEAD -> master, origin/master)
Author: Jonathan Strickland <djanatyn@gmail.com>
Date:   Thu Jan 21 14:59:12 2021 -0500

    slippi: 2.2.3 -> 2.2.5

diff --git a/slippi/default.nix b/slippi/default.nix
index 642b92f..0eede0f 100644
--- a/slippi/default.nix
+++ b/slippi/default.nix
@@ -5,14 +5,14 @@
 , mbedtls, curl, lzo, sfml, enet, xdg_utils, hidapi, webkit }:
 stdenv.mkDerivation rec {
   pname = "slippi-ishiiruka";
-  version = "2.2.3";
+  version = "2.2.5";
   name =
     "${pname}-${version}-${if playbackSlippi then "playback" else "netplay"}";
   src = fetchFromGitHub {
     owner = "project-slippi";
     repo = "Ishiiruka";
     rev = "v${version}";
-    sha256 = "0jj4s8ykl2qqwkg78gnl1n042y5f3hs003zdnxcp2r7cacsa0dsg";
+    sha256 = "02n2967rhbzcxb64392644c0g3x2q72ks4chdmawdanwij64a2z8";
   };

   outputs = [ "out" ];

Not sure if this is relevant, but I haven't seen any issues playing since I committed.

Uncle punch. I think we can temporarily simplify the design of the tests (because uncle-punch requires the melee iso, so it is always going to error out on a CI container) and readme by moving all of the uncle-punch work to a feature branch, until we figure out exactly what we need to do to get it working

Good call, agreed. I'll remove this work from the master branch for now. The existing work will be accessible in the commit history, so anyone can create an uncle-punch feature branch at any time.

Shields and actions - which actions and tests do you want? If you want to add a playback test, you should post a working .slp so I can write a test for it.

I needed to pick a good (and small!) clip. I uploaded one to IPFS: https://cloudflare-ipfs.com/ipfs/Qmf1eRVycbjvdChCGNrDCoGgWbYhpmT7nyMBa2gpx5ZyGX/rage_quit.slp

I don't know much about what this test would look like, but you can reference this file and URL for whatever you'd like.

.ini files - right now, it is basically impossible to configure dolphin at all, because its configuration files are located in the derivation. Do you know off-hand if it takes arguments

Yes, you can pass the configuration directory for dolphin with the -u flag:

❯ slippi-playback --help 2>&1 | rg '\--user'
  -u, --user=<str>                      User folder path

I use this to maintain two separate directories:

This is the same directory I store my user.json file as well. Setting your user config directory is required as far as I can tell (otherwise, it will try to use the read-only nix store directory).

Because of this, we might want to expose --user as an optional variable. nixpkgs provides some tools to create small wrapper scripts:

pkgs.symlinkJoin {
  name = "hello";
  paths = [ pkgs.hello ];
  buildInputs = [ pkgs.makeWrapper ];
  postBuild = ''
    wrapProgram $out/bin/hello \
      --add-flags "-t"
  '';
}

There's some more information on wrapProgram + makeWrapper in the nixpkgs manual. It would also be helpful to provide the user with a warning if this isn't set, perhaps at build time.

I experimented with rendering user.json using generators.toYAML. This works, but I don't think it's a good idea to add user.json to the nix store, since it is a secret.

djanatyn commented 3 years ago

@djanatyn Alright, slippi is now working fully (except on wayland, the gecko code submenu is a little bit hard to use). Mind the instructions about -u ~/.config/slippi-netplay or using the desktop item, you won't be able to login otherwise. If you want to just merge the slippi changes I can make a seperate branch or isolate a commit for you to cherry pick.

Oh, looks like my comments were a bit out of date! I read through all of the changes in the branch, everything looks great so far.

Adding shields - could use your input on what exactly you want

Good question, I haven't thought about this very much. At first, I think we'd want to provide visibility on the status of these two builds:

It would be great to also supply commands for running these builds (maybe using flake + commit hash).

I think these would also be fairly simple build invocations to tackle next:

It might be appropriate to create a PR for some of these packages upstream already.

For more motivation on what CI could represent, I use cachix. They are free for open source:

Community plan has 10 GB storage for open source projects.

Combining this with GitHub Actions would make our CI builds available to everyone using a binary cache. It's true that users can already play on Linux without building themselves using the AppImage, so this isn't too immediately valuable.

Like you mentioned, anything that relies on ssbm.iso is an issue for CI - we cannot add that ISO to the repository, or any references to download it.

While I don't mean to start implementing this immediately, I wonder if we could build this on a machine that already has the ISO?

An approach I've used in the past is to hand CI an SSH key that has access to connect to a service account:

tasks:
- build: |
    ssh srht-ci@vessel.voidheart.io \
      -p 8888 \
      -i ~/.ssh/c8e9cc18-e3c7-48bb-8d1e-0aa33c179bfe \
      -oStrictHostKeyChecking=no

I use ForceCommand and sudoer rules to configure this service account: https://github.com/djanatyn/nix-modules/blob/38f02e717d87893c3ffb78b00fda829e22f38d9d/modules/srht-ci/default.nix

This needs to be done safely (you don't want anyone with commit access to be able to run any command on your machine), but is an approach I'm comfortable exploring. I would probably look into running a Hydra instance if we took this approach - just an idea.

djanatyn commented 3 years ago

By the way, I just wanted to mention that I really appreciate your changes @6AA4FD! I see you've already started adding support for pplus, nice! README.md changes look good as well.

I left this repo a bit cluttered as I wasn't expecting anyone to take much notice. I've put some more thought into what value this repository could provide outside of nixpkgs and am drafting up a quick "roadmap" with some goals and phases.

I'm planning to create issues for some of the bigger tasks, providing a space to discuss their design and implementation. I'll keep the roadmap in source control (probably a file ROADMAP.md), so that it's possible to propose changes via pull request.

I don't have anything too ambitious in mind that's not already mentioned here, but there are some new tools that are worth investigating. As an example, mexTool provides a powerful API for packaging modifications to a melee ISO. I imagine that nix integration could provide some exciting opportunities.

6AA4FD commented 3 years ago

By the way, I just wanted to mention that I really appreciate your changes @6AA4FD... Thank you. It is much cleaner than my personal projects, trust me. Not a big deal. My general roadmap thoughts: probably leave pplus for later, as it is, fetching the inputs without a bunch of work going into caching will really slow down tests and everything. Uncle-punch and akaneia would both be suitable for building with nix, but they are impure and not very suited to flakes. I think we could instead provide flake templates or regular nix derivations for building them in some subfolder, that can call back to the main flake for the modding tools they need.

Tests are being a little bit stubborn, GitHub actions are a little bit stubborn (you can see the actions tab for my tests branch to see the errors) but I hope to have them working in the next few days. All of those builds in your list should still be working, so I will make tests for them once I have the basic workflow going.

BTW, the errors in the 2.2.5 build were either a wayland exclusive bug (missing GDK_BACKEND=x11) or with the new webview workflow, so if you already had user.json in your user folder, you would not have had a problem. My derivation also adds vulkan support, which I think is cool.

Good suggestion about wrapProgram by the way, I was using it but didn't know about --add-flags, hence the extra desktopItem step. I will update the derivation and the readme.

6AA4FD commented 3 years ago

Alright, I believe I have github actions and cachix working. We will see when the build finishes. I also added a feature in the flake module for a user to add the binary cache automatically, I would appreciate your input on whether we should provide the packages in the module as an overlay, or a flag to add the packages to the environment with the inputs from flake.lock (so to be sure they will be up-to-date with cachix)

edit: Yup, it's working. GH Actions builds, and I can pull from cachix fine. If you have a private means of communication you prefer, I can send you a cache auth token to add to your repo secrets (needed in order to write to cachix). Or vice versa, if you prefer.

6AA4FD commented 3 years ago

Bashing my head against the wall trying to get uncle-punch working, I found this thread. Note it says here 'Extracting files from a GameCube disc works, but not composing a GameCube disc.' If we want to build a gamecube image we will have to find a different tactic I think.

6AA4FD commented 3 years ago

RE: mexTool, it seems as if they don't distribute akaneia currently in their m-ex system, they provide an xdelta that works on the ISO. I would probably wait for them to publish the akaneia source code, to both packaging the tool.

djanatyn commented 3 years ago

I was able to pull from cachix as well! The latest flake looks good:

❯ nix flake show github:djanatyn/ssbm-nix/28cd0f819a267a1c1e193678dda2babaf5f9d56d
github:djanatyn/ssbm-nix/28cd0f819a267a1c1e193678dda2babaf5f9d56d
├───apps
│   └───x86_64-linux
│       ├───slippi-netplay: app
│       └───slippi-playback: app
├───nixosModule: unknown
├───overlay: Nixpkgs overlay
└───packages
    └───x86_64-linux
        ├───gcmtool: package 'gcmtool-unstable-2020-11-24'
        ├───gecko: package 'gecko-3.4.0'
        ├───powerpc-eabi-assembling: package 'powerpc-eabi-assembling'
        ├───projectplus-config: package 'projectplus-config'
        ├───projectplus-sdcard: package 'pplus-sdcard'
        ├───slippi-netplay: package 'slippi-ishiiruka-2.2.5-gitpatch-netplay'
        ├───slippi-netplay-chat-edition: package 'slippi-ishiiruka-chat'
        ├───slippi-playback: package 'slippi-ishiiruka-2.2.5-gitpatch-playback'
        └───wiimms-iso-tools: package 'wiimms-iso-tools'

I would appreciate your input on whether we should provide the packages in the module as an overlay, or a flag to add the packages to the environment with the inputs from flake.lock (so to be sure they will be up-to-date with cachix)

The way the packages are currently used in the overlay seems fine. Compatibility with cachix builds by default is good; I'm fuzzy on the interaction with flake.lock, especially if this repository is used as an input for another configuration. I'm not so sure about the differences here, so do whatever you feel is appropriate.

Bashing my head against the wall trying to get uncle-punch working, I found this thread. Note it says here 'Extracting files from a GameCube disc works, but not composing a GameCube disc.' If we want to build a gamecube image we will have to find a different tactic I think.

I also found myself very frustrated by this issue. I've noticed a lot of small differences in different implementations of GameCube ISO unpackers, like GCRebuilder. I found some other implementations. Writing code to work directly with a vanilla Melee ISO to manage skins and assets might be easier than getting wit working. Something like mexTool, but smaller in scope?

RE: mexTool, it seems as if they don't distribute akaneia currently in their m-ex system, they provide an xdelta that works on the ISO. I would probably wait for them to publish the akaneia source code, to both packaging the tool.

Yep! That's what I'm planning to do for now.

6AA4FD commented 3 years ago

I think I would probably wait to see how mexTool works before embarking on something like that. I am also taking a gander at building dat-texture-wizard, because 20XX is built with it.

edit: works in practice, i.e. what their build process is

djanatyn commented 3 years ago

Oh wow, I didn't realize DAT Texture Wizard was open source! I've been running it with wine 😅

djanatyn commented 3 years ago

I was looking at what's available on shields.io:

We could add this to the README.md as an indication that a cachix build is available.

I was hoping we could get a status for each individual package. It doesn't look like that's possible unless we maintain separate builds in .github/workflows/builds.yaml, which seems very unnecessary.

6AA4FD commented 3 years ago

I was looking at what's available on shields.io:

* [GitHub Workflow Status (`6AA4FD/ssbm-nix/builds`)](https://img.shields.io/github/workflow/status/6AA4FD/ssbm-nix/builds) ![badge](https://camo.githubusercontent.com/7fb571c1db3012680c08de50b87372f13db23b46c83d94c716503a2c1ee09445/68747470733a2f2f696d672e736869656c64732e696f2f6769746875622f776f726b666c6f772f7374617475732f3641413446442f7373626d2d6e69782f6275696c6473)

We could add this to the README.md as an indication that a cachix build is available.

I was hoping we could get a status for each individual package. It doesn't look like that's possible unless we maintain separate builds in .github/workflows/builds.yaml, which seems very unnecessary.

Sure, it's added. I also have a partially working build for dat-texture-wizard in there, maybe you want to give it a shot? I think there is a problem with the search path for python2.pkgs.cffi, but I am kind of sick of it for the moment, maybe you can take a look? Should be fine to push now too, check the tests. I sent you an email with the heading 'cachix auth token for ssbm-nix' or something like that, the body of the message is the value you should set CACHIX_AUTH_TOKEN to in your github repo secrets. You will also need to change the shield to your master branch in your README.

6AA4FD commented 3 years ago

@djanatyn pinging to remind you

djanatyn commented 3 years ago

I think there is a problem with the search path for python2.pkgs.cffi, but I am kind of sick of it for the moment, maybe you can take a look?

I'll check this out.

I sent you an email with the heading 'cachix auth token for ssbm-nix' or something like that, the body of the message is the value you should set CACHIX_AUTH_TOKEN to in your github repo secrets.

I've added the repository secrets:

❯ gh secret list -R djanatyn/ssbm-nix
CACHIX_AUTH_TOKEN  Updated 2021-02-11

You will also need to change the shield to your master branch in your README.

Will do.

Should be fine to push now too, check the tests.

Just finished reviewing, these changes are great. Thanks again for your patience with my responses.