Re: [PATCH] Add MARS shooter.

[Ricardo Wurmus]

Eric Bavier <address@hidden> writes:

>> I could not get sound to work, unfortunately, because /dev/dsp does not
>> exist on my machine.  It’s not a problem with the package, but maybe
>> someone could give me a hint about what kernel module to load to get
>> this device.
>
> Same on my machine.  I learned about the ossdp software, which may
> help.  http://sourceforge.net/projects/osspd/

Yes, this would help, but I think it’s unnecessary.  OpenAL was built
with ALSA and Pulseaudio backends, yet neither seems to work:

~~~~~~~~~
address@hidden $  ALSOFT_LOGLEVEL=3 
/gnu/store/hgx7fddl7ss45jhv4bns2837nd4x1h0f-mars-0.7.5-cc855d04409/bin/mars 
AL lib: (II) alc_initconfig: Supported backends: pulse, alsa, oss, null, wave
...
AL lib: (II) GetConfigValue: Key general:rt-prio not found
AL lib: (II) GetConfigValue: Key general:resampler not found
AL lib: (II) GetConfigValue: Key general:trap-al-error not found
AL lib: (II) GetConfigValue: Key general:trap-alc-error not found
AL lib: (II) GetConfigValue: Key reverb:boost not found
AL lib: (II) GetConfigValue: Key reverb:emulate-eax not found
AL lib: (II) GetConfigValue: Key general:drivers not found
AL lib: (WW) alc_initconfig: Failed to initialize backend "pulse"
AL lib: (WW) alc_initconfig: Failed to initialize backend "alsa"
AL lib: (II) GetConfigValue: Key oss:device not found
AL lib: (II) GetConfigValue: Key oss:capture not found
AL lib: (II) alc_initconfig: Initialized backend "oss"
AL lib: (II) alc_initconfig: Added "oss" for playback
AL lib: (II) alc_initconfig: Added "oss" for capture
...
AL lib: (EE) oss_open_playback: Could not open /dev/dsp: No such file or 
directory
Failed to open the audio device
...
~~~~~~~~~

I’ll try to get some more information about the failure to initialise
the “alsa” backend, but it seems to me that this is a problem with our
“openal” package.

>> +(define-public mars
>> +  ;; The latest release on SourceForge relies on an unreleased version of 
>> SFML
>> +  ;; with a different API, so we take the latest version from the official
>> +  ;; repository on Github.
>> +  (let ((commit "c855d04409"))
>> +    (package
>> +      (name "mars")
>> +      (version (string-append "0.7.5-c" commit ))
>
> I realize we have no guidelines in the manual concerning the version
> field for git checkouts, but I wonder whether we should, as it comes up
> a bit.  Several existing packages use (string-append "1.2.3." commit),
> where "1.2.3" is the version of the corresponding source.  One other
> package uses the (string-append "1.2.3-c" commit) method, and another
> uses (string-append "1.2.3-" commit.  I personally prefer the "-"
> notation, since it distinguishes the commit hash from the version
> number (does it confuse any internal logic that assumes a package
> version number is the last component of the store path following a
> dash?).  In this case, the "-c" seems confusing because the commit hash
> itself begins with a 'c'.

True.  I have no real preference.  If an update to a more recent commit
makes the introduction of an ordering character necessary it could still
be introduced to force a particular ordering.

>> +      (source (origin
>> +                (method git-fetch)
>> +                (uri (git-reference
>> +                      (url "https://github.com/thelaui/M.A.R.S..git";)
>> +                      (commit commit)))
>
> guix lint should warn now about the 'file-name' of the origin.

Oh, right.  I’ll fix it.

>> +                (sha256
>> +                 (base32
>> +                  "1r4c5gap1z2zsv4yjd34qriqkxaq4lb4rykapyzkkdf4g36lc3nh"))
>> +                (patches (list (search-patch "mars-sfml-2.3.patch")
>
> As mentioned on IRC, I had trouble applying this patch because some DOS
> line-endings went missing somewhere.  We just need to make sure they
> are preserved by git.

The patch looks fine when I inspect the commit.  It still has the DOS
line-endings, so I don’t think this will be a problem when I push it.

>> +                               (search-patch "mars-install.patch")))))
>> +      (build-system cmake-build-system)
>> +      (arguments
>> +       `(#:tests? #f        ; There are no tests
>> +         #:phases
>> +         (modify-phases %standard-phases
>> +           (add-after 'unpack 'fix-install-path
>> +            (lambda _
>> +              (substitute* "src/CMakeLists.txt"
>> +                (("\\$\\{CMAKE_INSTALL_PREFIX\\}/games")
>> +                 "${CMAKE_INSTALL_PREFIX}/bin"))))
>
> As in the following phase, could you return #t after the 'substitute*'?

Yes, I forgot this when I broke this up into two phases.

>> +           (add-after 'unpack 'fix-data-path
>> +            (lambda* (#:key outputs #:allow-other-keys)
>> +              (substitute* "src/System/settings.cpp"
>> +                (("C_dataPath = \"./data/\";")
>> +                 (string-append "C_dataPath = \""
>> +                                (assoc-ref outputs "out")
>> +                                "/share/games/marsshooter/\";")))
>> +              #t)))))
>> +      (inputs
>> +       `(("mesa" ,mesa)
>> +         ("fribidi" ,fribidi)
>> +         ("taglib" ,taglib)
>> +         ("sfml" ,sfml)))
>
> Our sfml package has been failing to build on hydra for a few weeks
> because of receiving unexpected hashes from the source downloads.  It
> appears the authors may have changed the zip file in place when they
> released the latest version, and you simply had the previous zip in
> your store from before.  Hydra seems to not be serving the correct zip
> file either.

I updated our sfml package about an hour ago.  It now downloads the
sources from the Github tags/releases, which are (almost) immutable.

~~ Ricardo