[bug#37714] Add renpy package

[Nicolas Goaziou <mail@nicolasgoaziou.fr>]

Hello,

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> This patch adds the Ren'py Visual Novel engine split into four
> outputs:

Great! It was on my TODO list, too.

> - "out": contains the engine itself. Since Guix differs strongly from
> the platforms Ren'py usually runs on, we ship our own Ren'py main file.
> - "launcher": The Ren'py launcher, that people would normally expect
> when invoking renpy -- it does not seem very usable in Guix, though.

What do you mean?

> * gnu/packages/game-development: (python2-pygame-for-renpy): New procedure.
> (python2-renpy): New variable.

New variable for both in enough, I think.

> +(define (python2-pygame-for-renpy version hash)
> +  (package
> +   (inherit python2-pygame)
> +   (name "python2-pygame-for-renpy")
> +   (version version)
> +    (source
> +     (origin
> +      (method git-fetch)
> +      (uri (git-reference
> +            (url "https://github.com/renpy/pygame_sdl2.git")
> +            (commit (string-append "renpy-" version))))

Upstream provides a tarball for that:

  https://www.renpy.org/dl/7.3.5/pygame_sdl2-2.1.0-for-renpy-7.3.5.tar.gz

I think it is preferable using it than pointing to Github.

> +      (sha256
> +       (base32
> +        hash))

Nitpick: At least bash32 and hash could go on the same line.

> +    (home-page "http://www.renpy.org/")
> +    (synopsis "Reimplementation of the Pygame API using SDL2")

The description field is missing. Could you add one?

> +(define-public python2-renpy

"python2" prefix is for libraries. I think this one can be called
"renpy".

> +  (package
> +   (name "python2-renpy")
> +   (version "7.3.4.596")

I noticed 7.3.5 is out. Could you update it?

> +    (source
> +     (origin
> +      (method git-fetch)
> +      (uri (git-reference
> +            (url "https://github.com/renpy/renpy.git")
> +            (commit version)))

As above, I suggest to use tarball from main site:

  https://www.renpy.org/dl/7.3.5/renpy-7.3.5-source.tar.bz2

> +         (replace 'build
> +           (lambda args
> +             (apply
> +              (lambda* (build-root #:key inputs outputs #:allow-other-keys)
> +                (chdir "module")
> +                (apply (assoc-ref %standard-phases 'build) args)
> +                (chdir build-root)
> +                (delete-file "renpy/__init__.pyc")
> +                (invoke "python" "-m" "compileall" "renpy"))
> +              (getcwd) args)
> +             #t))

This is a bit confusing because you're not really replacing `build'
phase, but wrapping stuff around it. I think it may be better to add
a `before-build' and an `after-build' phases, adding comments, if
possible, to explain why they are needed.

> +         (replace 'install
> +           (lambda args
> +             (apply
> +              (lambda* (build-root #:key inputs outputs #:allow-other-keys)
> +                (let* ((pygame (assoc-ref inputs "python2-pygame"))
> +                       (out (assoc-ref outputs "out"))
> +                       (site (string-append "/lib/python"
> +                                            ,(version-major+minor
> +                                              (package-version python-2))
> +                                            "/site-packages")))
> +                  (chdir "module")
> +                  (apply (assoc-ref %standard-phases 'install) args)
> +                  (chdir build-root)
> +                  (copy-recursively "renpy"
> +                                    (string-append out site "/renpy"))))
> +              (getcwd) args)
> +             #t))

See above. It might be more readable to use one phase before, and one
after.

> +    (inputs
> +     `(("ffmpeg" ,ffmpeg)
> +       ("freetype" ,freetype)
> +       ("glew" ,glew)
> +       ("libpng" ,libpng)
> +       ("sdl-union"
> +        ,(sdl-union (list sdl2 sdl2-image sdl2-mixer sdl2-ttf)))
> +       ("python" ,python)))

So it needs both Python 2 and Python 3?

Could you order alphabetically the inputs?

> +    (outputs
> +     (list "out" "launcher" "tutorial" "the-question"))
> +    (propagated-inputs
> +     `(("python2-pygame"
> +        ,(python2-pygame-for-renpy
> +          version
> +          "1gwbqkyv7763813x7nmzb3nz4r8673d33ggsp6lgp1d640rimlpb"))))

Could you see if there's a way to not propagate this input? What happens
if it is a simple input?

> +    (native-inputs
> +     `(("python2-cython" ,python2-cython)))
> +    (home-page "http://www.renpy.org/")
> +    (synopsis "The Ren'Py Visual Novel Engine -- libary files")
> +    (description "Ren'Py is a visual novel engine -- used by thousands of
> +creators from around the world -- that helps you use words, images, and sounds
> +to tell interactive stories that run on computers and mobile devices. These can
> +be both visual novels and life simulation games. The easy to learn script
> +language allows anyone to efficiently write large visual novels, while its
> +Python scripting is enough for complex simulation games.")

You need to add two spaces at the end of sentences.

> +    (license license:x11)))

I this should be license:expat.

Could you send an updated patch?

Thank you for the work.


Regards,

-- 
Nicolas Goaziou