soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436

sternenseemann · 2021-01-04T23:55:17Z

Motivation for this change

Adds the open sourced development version of soldat, a 2d side
scrolling shooter. The soldat-unstable attribute name was chosen since
this is a development version of the unfinished soldat 1.8 version which
most notably has incompatible netcode to the windows-only soldat 1.7
version which you can download on steam for example, so naming this
soldat-unstable hopefully avoids confusion as to why it is not possible
to connect to soldat 1.7 servers.

We wrap the soldat and soldatserver executables to have fixed paths set
for fs_basepath (the game archive) and fs_userpath (configuration, state
files). The latter is set to $XDG_CONFIG_HOME/soldat/soldat{,server} by
default to stop the executables from polluting the working directory.
Both settings can be overriden however by giving the respective options
on the command line of the wrapper.

GameNetworkingSockets is a dependency of soldat.

Things done

pkgs/development/libraries/gamenetworkingsockets/default.nix

pkgs/games/soldat-unstable/default.nix

Profpatsch · 2021-01-05T13:52:48Z

pkgs/games/soldat-unstable/default.nix

+    # make sure soldat{,server} find their game archive,
+    # let them write their state and configuration files
+    # to $XDG_CONFIG_HOME/soldat/soldat{,server} unless
+    # the user specifies otherwise.
+    for p in $out/bin/soldatserver $out/bin/soldat; do
+      configDir="\''${XDG_CONFIG_HOME:-\$HOME/.config}/soldat/$(basename "$p")"
+
+      wrapProgram "$p" \
+        --run "mkdir -p \"$configDir\"" \
+        --add-flags "-fs_portable 0" \
+        --add-flags "-fs_userpath \"$configDir\"" \
+        --add-flags "-fs_basepath \"${base}/share/soldat\""
+    done
+  '';


What’s the default paths soldat uses?

Maybe we can upstream XDG_CONFIG_HOME instead

soldat and soldatserver expect to be run from the client or server subdirectories of the development repository respectively. By default they assume fs_basepath and fs_userpath are the same directory as their executable which obviously doesn't work since that would be in /nix/store.

I contemplated using ./ as a default for fs_userpath, but it's also not a nice solution and relatively annoying.

Upstreaming XDG_CONFIG_HOME could be an idea, unfortunately haven't heard a lot on the other patch yet as well. I guess I'll try make a patch which uses XDG_CONFIG_HOME on Unix and %AppData% on Windows…

SuperSandro2000 · 2021-01-05T22:37:17Z

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108436 run on x86_64-linux 1

2 packages built:

gamenetworkingsockets
soldat-unstable

SuperSandro2000 · 2021-01-05T23:44:01Z

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108436 run on x86_64-darwin 1

1 package failed to build and are new build failure:

gamenetworkingsockets: log https://hastebin.com/raw/feqelinexe

sternenseemann · 2021-01-05T23:45:18Z

gamenetworkingsockets: log https://hastebin.com/raw/feqelinexe

log not found

SuperSandro2000 · 2021-01-07T10:37:57Z

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108436 run on x86_64-linux 1

2 packages built:

gamenetworkingsockets
soldat-unstable

Not sure why it now builds...
Also about the log missing: I fixed a few bugs in it the last days. Should become more stable over time.

Profpatsch · 2021-01-10T10:53:52Z

Shall we merge?

sternenseemann · 2021-01-10T11:05:13Z

Fixing GameNetworkingSockets and soldat for darwin would be nice, but I can' really gauge what the problem is without a build log.

SuperSandro2000 · 2021-01-11T07:14:42Z

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108436 run on x86_64-darwin 1

1 package failed to build and are new build failure:

gamenetworkingsockets: log https://termbin.com/o8jc

sternenseemann · 2021-01-11T09:44:23Z

Okay, this is a genuine issue with GameNetworkingSockets, SOCK_NONBLOCK doesn't exist on darwin. I've marked it as broken for now and will try to get a patch into upstream and the come back to fix the package.

sternenseemann · 2021-01-11T11:35:12Z

The build failure is already resolved on GameNetworkingSockets master, so it's just a matter of time to have it for darwin as well.

SuperSandro2000 · 2021-01-11T21:39:51Z

Result of nixpkgs-review pr 108436 run on x86_64-darwin 1

1 package marked as broken and skipped:

gamenetworkingsockets

SuperSandro2000 · 2021-01-12T00:28:59Z

This is a semi-automatic executed nixpkgs-review which does not build all packages (e.g. lumo, tensorflow or pytorch)
If you find some bugs or got suggestions for further things to search or run please reach out to SuperSandro2000 on IRC.

Result of nixpkgs-review pr 108436 run on x86_64-linux 1

2 packages built:

gamenetworkingsockets
soldat-unstable

SuperSandro2000 · 2021-01-12T00:44:02Z

pkgs/development/libraries/gamenetworkingsockets/default.nix

+  cmakeFlags = [ "-G Ninja" ];
+
+  # tmp home for go
+  preBuild = "export HOME=\"$TMPDIR\"";


Suggested change

preBuild = "export HOME=\"$TMPDIR\"";

preBuild = ''

export HOME=$TMPDIR

'';

Profpatsch · 2021-01-12T09:31:38Z

@SuperSandro2000 Automating review is nice and all, but please scroll through the backlog and tell me it’s not starting to get spammy.

e.g. your bot (‽) added the same comment as earlier, even though @sternenseemann already said that the nit is irrelevant.

We shouldn’t force stylistic nitpicks onto maintainers that know what they are doing.

SuperSandro2000 · 2021-01-12T13:21:10Z

e.g. your bot (‽) added the same comment as earlier, even though @sternenseemann already said that the nit is irrelevant.

I am not a bot. I have several humans that can prove that I made out of flesh. Jokes aside.

I read through the diff and thought the second time that this is still not quite right.

Generally variables which contain commands use '' because they can easily be multiline even if they only contain one command. Additionally some extra escaping is required here because of that. I think that adds the false intention that $TMP maybe shouldn't be evaluated at that point and only at runtime. I try to not nit about older packages to much when they just change one line or do a simple version update but for new packages or packages with bigger changes we should focus on making them clean and use hooks and variables when available. Especially for new packages we should really think how to make them clean, short and simple to easily maintain them in the future, make updates easy and don't hinder treewide changes with unusual syntaxes or escapes.

We shouldn’t force stylistic nitpicks onto maintainers that know what they are doing.

We already enforce a pretty great list of stylistics and probably some of them are nitpicks but so far I don't think I have seen one that is literally useless.

Profpatsch · 2021-01-12T13:44:12Z

Cool, merging.

sternenseemann · 2021-01-12T13:48:51Z

It makes a difference of one `\n in the string and I'd argue in this case it reduces noise and readability since a relatively unimportant compatibility fix is not occupying to much space.

Also I think I'll be fine maintaining this whereas endless rebasing is just plain annoying.

SuperSandro2000 · 2021-01-13T00:36:17Z

@Profpatsch If you wouldn't be the first person labeling my manual work as a bot I would be pretty offended by that.

@sternenseemann You could have just accepted that change but instead we are arguing about something which is not worth to argue about. I am not writing comments on things because I feel like I want to annoy but I think they would improve the overall quality by more than a \n.

This was referenced Jan 4, 2021

soldat: init at unstable-2020-11-26 openlab-aux/vuizvui#37

Closed

gamenetworkingsockets: init at 1.2.0 #107100

Closed

ofborg bot added 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 labels Jan 5, 2021

SuperSandro2000 requested changes Jan 5, 2021

View reviewed changes

pkgs/development/libraries/gamenetworkingsockets/default.nix Outdated Show resolved Hide resolved

pkgs/games/soldat-unstable/default.nix Outdated Show resolved Hide resolved

pkgs/games/soldat-unstable/default.nix Outdated Show resolved Hide resolved

sternenseemann force-pushed the soldat branch 2 times, most recently from ce3d1ff to e514dec Compare January 5, 2021 10:21

Profpatsch approved these changes Jan 5, 2021

View reviewed changes

sternenseemann force-pushed the soldat branch from e514dec to 4b2b51f Compare January 11, 2021 09:43

sternenseemann force-pushed the soldat branch from 4b2b51f to b0bc84d Compare January 11, 2021 11:33

sternenseemann added 2 commits January 11, 2021 12:33

gamenetworkingsockets: init at 1.2.0

a609850

soldat-unstable: init at 2020-11-26

6440b8d

sternenseemann force-pushed the soldat branch from b0bc84d to 6440b8d Compare January 11, 2021 11:33

SuperSandro2000 reviewed Jan 12, 2021

View reviewed changes

Profpatsch merged commit 2a88330 into NixOS:master Jan 12, 2021

sternenseemann deleted the soldat branch January 12, 2021 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436

soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436

sternenseemann commented Jan 4, 2021 •

edited

Profpatsch Jan 5, 2021

Profpatsch Jan 5, 2021

sternenseemann Jan 5, 2021 •

edited

SuperSandro2000 commented Jan 5, 2021

SuperSandro2000 commented Jan 5, 2021

sternenseemann commented Jan 5, 2021

SuperSandro2000 commented Jan 7, 2021 •

edited

Profpatsch commented Jan 10, 2021

sternenseemann commented Jan 10, 2021

SuperSandro2000 commented Jan 11, 2021

sternenseemann commented Jan 11, 2021

sternenseemann commented Jan 11, 2021

SuperSandro2000 commented Jan 11, 2021

SuperSandro2000 commented Jan 12, 2021

SuperSandro2000 Jan 12, 2021

Profpatsch commented Jan 12, 2021

SuperSandro2000 commented Jan 12, 2021

Profpatsch commented Jan 12, 2021

sternenseemann commented Jan 12, 2021

SuperSandro2000 commented Jan 13, 2021 •

edited

-  preBuild = "export HOME=\"$TMPDIR\"";
+  preBuild = ''
+    export HOME=$TMPDIR
+  '';

soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436

soldat-unstable: init at unstable-2020-11-26; gamenetworkingsockets: init at 1.2.0 #108436

Conversation

sternenseemann commented Jan 4, 2021 • edited

Motivation for this change

Things done

Profpatsch Jan 5, 2021

Choose a reason for hiding this comment

Profpatsch Jan 5, 2021

Choose a reason for hiding this comment

sternenseemann Jan 5, 2021 • edited

Choose a reason for hiding this comment

SuperSandro2000 commented Jan 5, 2021

SuperSandro2000 commented Jan 5, 2021

sternenseemann commented Jan 5, 2021

SuperSandro2000 commented Jan 7, 2021 • edited

Profpatsch commented Jan 10, 2021

sternenseemann commented Jan 10, 2021

SuperSandro2000 commented Jan 11, 2021

sternenseemann commented Jan 11, 2021

sternenseemann commented Jan 11, 2021

SuperSandro2000 commented Jan 11, 2021

SuperSandro2000 commented Jan 12, 2021

SuperSandro2000 Jan 12, 2021

Choose a reason for hiding this comment

Profpatsch commented Jan 12, 2021

SuperSandro2000 commented Jan 12, 2021

Profpatsch commented Jan 12, 2021

sternenseemann commented Jan 12, 2021

SuperSandro2000 commented Jan 13, 2021 • edited

sternenseemann commented Jan 4, 2021 •

edited

sternenseemann Jan 5, 2021 •

edited

SuperSandro2000 commented Jan 7, 2021 •

edited

SuperSandro2000 commented Jan 13, 2021 •

edited