onscripter: init at 20230825 by midchildan · Pull Request #451514 · NixOS/nixpkgs

midchildan · 2025-10-13T00:43:03Z

Add package for ONScripter
Base the existing package for the fork, ONScripter EN, on the new package
Fix Darwin build for ONScripter EN

Things done

Add a 👍 reaction to pull requests you find important.

Sigmanificient · 2025-10-14T05:38:16Z

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 451514
Commit: 45e902296e7f6be4c8cf9dfbd9137ed0d789bddb (subsequent changes)
Merge: aa4848a44ca48852b017bb0b9410f22e993d6ad7

Logs: https://github.com/Sigmanificient/nixpkgs-review-gha/actions/runs/18480843180

`x86_64-linux`

✅ 2 packages built:

onscripter
onscripter-en

`aarch64-linux`

✅ 2 packages built:

onscripter
onscripter-en

`x86_64-darwin` (sandbox = true)

✅ 2 packages built:

onscripter
onscripter-en

`aarch64-darwin` (sandbox = true)

✅ 2 packages built:

onscripter
onscripter-en

nixos-discourse · 2025-10-15T18:44:45Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/5947

emilazy · 2025-10-15T19:23:00Z

pkgs/by-name/on/onscripter-en/package.nix

+let
+  inherit (onscripter) stdenv;
+in
+onscripter.overrideAttrs (


We should avoid introducing new uses of overrideAttrs. This would ideally be a flag in the onscripter package, but considering this is a version from 2011, has no maintainer, and the upstream source is apparently gone, perhaps it would be better to simply drop this package?

It seems like there are active forks, though:

https://github.com/Galladite27/ONScripter-EN

https://github.com/insani-org/onscripter-insani

I've now removed ONScripter EN because 1. there should be English support in ONScripter upstream, 2. I'm not a user of ONScripter EN so it's better for actual users to test and maintain the package if needed.

emilazy · 2025-10-15T19:23:15Z

pkgs/by-name/on/onscripter/package.nix

+  src = fetchFromGitHub {
+    owner = "ogapee";
+    repo = "onscripter";
+    rev = finalAttrs.version;
+    hash = "sha256-XaZTtOkV+2dHmcgZ4GbyiMxRBYdwT5eHxx+r05eAmBw=";
+  };


Is this the official upstream? It seems like maintenance may be abandoned?

Yes, this is the official upstream. It's a clone of a game engine from the 2000s, with over 20 years of development history. So it's stable and has less churn these days.

I've been able to complete a whole game with this build, so functionality-wise, things should be okay.

emilazy

Thanks, this looks good to me with a few minor issues (which I should have gone over more thoroughly in the previous review; sorry about that).

emilazy · 2025-10-16T03:01:52Z

pkgs/top-level/all-packages.nix

+  onscripter = callPackage ../by-name/on/onscripter/package.nix {
+    # Check unpatched Makefile.Linux for the Lua version
+    lua = lua5_1;
+  };


I think per #444420 we would prefer this to be an explicit lua5_1 argument in onscripter now rather than adding an entry to all-packages.nix.

emilazy · 2025-10-16T03:02:39Z

pkgs/by-name/on/onscripter/package.nix

+  ++ (if stdenv.isDarwin then [ freetype ] else [ fontconfig ]);
+
+  # The build script for BSDs also fall under Makefile.Linux
+  makefile = if stdenv.isDarwin then "Makefile.MacOSX" else "Makefile.Linux";


We are trying to deprecate stdenv.isFoo since it is ambiguous in the presence of cross‐compilation, so:

Suggested change

++ (if stdenv.isDarwin then [ freetype ] else [ fontconfig ]);

# The build script for BSDs also fall under Makefile.Linux

makefile = if stdenv.isDarwin then "Makefile.MacOSX" else "Makefile.Linux";

++ (if stdenv.hostPlatform.isDarwin then [ freetype ] else [ fontconfig ]);

# The build script for BSDs also fall under Makefile.Linux

makefile = if stdenv.hostPlatform.isDarwin then "Makefile.MacOSX" else "Makefile.Linux";

emilazy · 2025-10-16T03:03:24Z

pkgs/by-name/on/onscripter/package.nix

+    # 1. Remove direct linker flags for macOS SDKs.
+    # 2. Build with Lua
+    #
+    # ONScripter appears to depend on macOS SDKs only indirectly through SDL, so flags provided by
+    # the sdl-config command should suffice. The existing flags also referenced the deprecated
+    # QuickTime framework, resulting in a build failure.


The -framework part of this patch should no longer be necessary, since the entire SDK is available by default these days. (Though from the sounds of it, it shouldn’t do any harm either.)

The patch removes -framework flags instead of adding them. It was actually breaking the build because -framework QuickTime no longer works with newer SDKs.

emilazy · 2025-10-16T03:06:58Z

pkgs/by-name/on/onscripter/package.nix

+  nativeBuildInputs = [
+    freetype
+    SDL
+    smpeg
+  ];


I am not totally sure these are correct in the presence of cross‐compilation, as I believe the sdl-config would give the flags for the build rather than host platform. My understanding is that you want the sdl-config from the host platform SDL instead – when building cross, the executable will be for the build platform.

I am not sure exactly how this should be expressed; export PATH=${lib.getDev SDL}/bin:$PATH would do it, I believe, but is inelegant. The ideal thing may be to patch the build to use pkg-config instead, since we’re already touching those lines, and drop these from nativeBuildInputs.

I could be wrong here, though; cc @NixOS/sdl perhaps?

I replaced it with the following:

nativeBuildInputs = [ # Take cross compilation into account (runCommand "onscripter-host-deps" {} '' mkdir -p $out/bin ln -s \ ${lib.getDev freetype}/bin/freetype-config \ ${lib.getDev SDL}/bin/sdl-config \ ${lib.getDev smpeg}/bin/smpeg-config \ $out/bin '') ];

emilazy · 2025-10-16T03:10:59Z

Oh, this also needs a throw for onscripter-en in aliases.nix.

Also fix Darwin build.

midchildan · 2025-10-16T17:18:53Z

@emilazy Thanks. Could you take another look?

emilazy

Looks good to me, thanks.

emilazy · 2025-10-20T19:26:26Z

pkgs/by-name/on/onscripter/package.nix

+      runCommand "onscripter-host-deps" { } ''
+        mkdir -p $out/bin
+        ln -s ${lib.escapeShellArgs commands} $out/bin
+      ''


I think this should technically be buildPackages.runCommand, but it shouldn’t make a difference in practice.

nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. labels Oct 13, 2025

midchildan force-pushed the feat/add-onscripter branch from 5974991 to 45e9022 Compare October 13, 2025 01:34

emilazy reviewed Oct 15, 2025

View reviewed changes

nixpkgs-ci bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Oct 15, 2025

midchildan force-pushed the feat/add-onscripter branch from 4140238 to 1a2bea9 Compare October 16, 2025 02:18

nixpkgs-ci bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 16, 2025

emilazy reviewed Oct 16, 2025

View reviewed changes

midchildan force-pushed the feat/add-onscripter branch from 1a2bea9 to 68ac240 Compare October 16, 2025 16:51

midchildan added 3 commits October 17, 2025 02:09

onscripter: init at 20230825

c49d51b

onscripter-en: base package on onscripter

a9b7e17

Also fix Darwin build.

onscripter-en: remove

b225803

midchildan force-pushed the feat/add-onscripter branch from 68ac240 to b225803 Compare October 16, 2025 17:11

emilazy approved these changes Oct 20, 2025

View reviewed changes

emilazy added this pull request to the merge queue Oct 20, 2025

Merged via the queue into NixOS:master with commit eaa4d38 Oct 20, 2025
25 checks passed

nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 20, 2025

midchildan deleted the feat/add-onscripter branch October 22, 2025 19:48

Uh oh!

Conversation

midchildan commented Oct 13, 2025

Things done

Uh oh!

Sigmanificient commented Oct 14, 2025

nixpkgs-review result

x86_64-linux

aarch64-linux

x86_64-darwin (sandbox = true)

aarch64-darwin (sandbox = true)

Uh oh!

nixos-discourse commented Oct 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilazy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

midchildan Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emilazy commented Oct 16, 2025

Uh oh!

midchildan commented Oct 16, 2025

Uh oh!

emilazy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`

`x86_64-darwin` (sandbox = true)

`aarch64-darwin` (sandbox = true)

midchildan Oct 16, 2025 •

edited

Loading