[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#40035] Add widelands game
[bug#40035] Add widelands game
Thu, 12 Mar 2020 15:31:20 +0100
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
Alberto EFG <address@hidden> writes:
> This is my first patch.
Thank you, and congratulations!
> +(define-public widelands
> + (let ((commit "d9513d413f2558f9ef6f033a7685bf9881fbdbb3")
> + (revision "1"))
I suggest to add a comment explaining why we rely on a commit, and, if
that makes sense, why this particular one, e.g., "No official release."
> + (package
> + (name "widelands")
> + (version (git-version "20" revision commit))
Where is this "20" coming from?
> + (source (origin
Nitpick: I suggest to move `origin' to a line on its own.
> + (arguments
> + `(#:tests? #f
Why are the tests disabled? We usually provide a comment when disabling
> + #:configure-flags
> + (let* ((out (assoc-ref %outputs "out"))
> + (share (string-append out "/share")))
> + (list "-DCMAKE_BUILD_TYPE=Release"
> + (string-append "-DCMAKE_INSTALL_PREFIX=" out "/bin")
> + (string-append "-DWL_INSTALL_BASEDIR=" share "/widelands")
> + (string-append "-DWL_INSTALL_DATADIR=" share "/widelands")
> + "-DOPTION_BUILD_WEBSITE_TOOLS=OFF"))))
> + (inputs
> + `(("sdl" ,(sdl-union (list sdl2
> + sdl2-image
> + sdl2-mixer
> + sdl2-ttf)))
Nitpick: all can go into a single line.
> + ("gettext" ,gettext-minimal)
This probably belongs to `native-inputs' not `inputs'.
> + ("icu4c" ,icu4c)
Indentation is off here.
> + ("libpng" ,libpng)
> + ("zlib" ,zlib)
> + ("boost" ,boost)
> + ("python" ,python)
This one may also be a native input. Could you check?
> + ("glew" ,glew)))
Could you re-order inputs alphabetically?
> + (synopsis "Real-time strategy game")
It is a bit terse. Debian uses the slightly more accurate:
"Fantasy real-time strategy game"
Maybe it is worth mentioning. Or better, something like:
"Fantasy real-time strategy game with singleplayer campaigns and multiplayer
It could make sense since in the description I suggest below, there is
no reference to campaigns nor multiplayer.
> + (description
> + "Widelands is a free, open source real-time strategy game with
You can remove "free" and "open source". All is free in Guix!
> + singleplayer campaigns and a multiplayer mode. The game was inspired
> + by Settlers II but has significantly more variety and depth to it. ")
Mind the spurious spaces at the end.
Again, Debian uses:
Widelands is a strategy game aiming for gameplay similar to Settlers II by
In this game, you start out on a small piece of land with nothing more than
a few of useful resources. Using those, you can build yourself an empire
with many thousands of inhabitants. On your way towards this goal, you will
have to build up an economic infrastructure, explore the lands around you
and face enemies who are trying to rule the world just like you do.
Would it be better to use it?
> + (home-page "https://www.widelands.org/")
Nitpick: home-page is usually located above synopsis. I don't know if
there's a strong rule about it, tho.
> + (license license:gpl2+))))
You are missing out some licenses (CC-based) from the assets in the
game. Could you add them too?
Could you send an updated patch?