Subject: Re: [bug#35127] [PATCH] Add meritous.

From: Nicolas Goaziou <[email-masked]>
Date: Thu, 04 Apr 2019 11:24:54 +0200

Hello,

Thank you for the review.

Ricardo Wurmus <[email-masked]> writes:

>> +         (add-after 'unpack 'fix-sdl-path
>> +           ;; XXX: For some reason, `sdl-config' reports stand-alone SDL
>> +           ;; directory, not SDL-union provided as an input to the package.
>> +           ;; We force the latter with "--prefix=" option.
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             (substitute* "Makefile"
>> +               (("sdl-config" command)
>> +                (string-append command " --prefix=" (assoc-ref inputs "sdl"))))
>> +             #t))
>
> Hmm, weird.  Do you know why this happens?  Is this something we can fix
> in sdl-union?

The Makefile uses "$(shell sdl-config --libs)" (see
<https://gitlab.com/meritous/meritous/blob/master/Makefile>). I tried to
replace the sdl-config executable with its full path, i.e.,
(string-append (assoc-ref inputs "sdl") "/bin/sdl-config"), to no avail.

Is there a problem in sdl-union? Is the `shell' call the issue? I don't
know.

>> +         (add-after 'unpack 'fix
                                 ^^^
                     I replaced that with 'fix-crash

>> +           ;; XXX: Songs are removed due to licensing issues.  However, the
>> +           ;; game tries to load them, and, since it cannot find them,
>> +           ;; crashes.  Users cannot add them back, the store being read-only,
>> +           ;; so we turn off background music altogether.
>
> Oh :(
>
> Where are the songs removed?  Should they be removed in a snippet?

The songs are not in the repository. See
<https://gitlab.com/meritous/meritous/blob/master/dat/m/README-music.txt>
Yet, the game tries to find them, and, AFAICT, crashes when it fails.
They could be replaced with free music, but I don't think it has been
done in any distribution.

Also, this is just for background music. The game still provides ambient
sounds.

For clarity, I reworded the comment a bit:

     ;; XXX: Songs are not present in the repository, due to licensing
     ;; issues.  Yet, the game tries to load them, and, since it cannot
     ;; find them, crashes.  Users cannot add them back, the store being
     ;; read-only, so we turn off background music altogether.

>> +           (lambda _
>> +             (substitute* "src/audio.c"
>> +               (("PlayBackgroundMusic\\(new_track\\);" all)
>> +                (string-append "// " all))))))))
>
> Please end with #T.

Oops. Done.

>> +    (native-inputs
>> +     `(("intltool" ,intltool)))
>> +    (inputs
>> +     `(("sdl" ,(sdl-union (list sdl sdl-image sdl-mixer)))
>> +       ("zlib" ,zlib)))
>> +    ;; Original home page <http://asceai.net/meritous/> has stalled.
>
> I don’t think “stalled” is the right term.  I think you can remove this
> comment.  The gitlab page is fine as a home page.

OK.

>> +    (description "Far below the surface of the planet is a secret.  A place of
>> +limitless power.
>
> How about “a secret place of limitless power”?  (“is a secret” sounds
> odd to me.)
>
>> +Meritous is a action-adventure game with simple controls but a challenge to
>> +find a balance of power verses recovery time during real-time
>> battles.
>
> Should be “versus”.

OK. I will fix those before pushing.

WDYT?

Regards,

-- 
Nicolas Goaziou