[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#778546: RFS: miceamaze/4.2-1 -- video game with mice in a maze



Hello,

Thank you for answering and reviewing the package so fast.

I have (I hope) adressed the blocking issue and some of
your recommendations also.

The new files are there: http://www.miceamaze.org/debian/

See below for my answers:

2015-02-17 6:12 GMT+01:00 Paul Wise <pabs@debian.org>:
Control: tags -1 + moreinfo

On Mon, Feb 16, 2015 at 10:38 PM, Raphaël Champeimont wrote:

>   miceamaze - video game with mice in a maze

There is one issue that need to be fixed before I will upload the package:

The debian/copyright file needs to be updated for the new copyright
year. It also needs updating for the new files that are under
different licenses or have different copyright holders (data/music/
and data/mazes/maze13.txt). Please note that a full copy of the CC-BY
3.0 license needs to be added to debian/copyright, so offline users
can read the license.

She agrees to release the maze to the public domain.
I also release the ones I have made to the public domain,
so I have added an entry to say all mazes are in the public domain.

OK I added entries for the music files with CC-BY.
 

Other thoughts:

On a system with two screens (laptop + external screen), full-screen
mode crosses the two screens which makes the menu hard to read and
cuts part of it off.

I'm not sure I can fix this, because all I do is ask for SDL to setup a
full-screen OpenGL display, but don't think it is possible to specify
the behavior on multi-screen.
 

In chromium-bsu I was able to get nice anti-aliased text by passing
GLC_TEXTURE to glcRenderStyle, but for some reason that only produces
white squares in miceamaze.

Perhaps it is because I use display lists in which I put some text rendering,
and it may not work well with the texture-based rendering (just a guess).
 

You might want to mention the Standards-Version in debian/changelog.
If you have gone through the upgrading checklist and there aren't any
changes that apply to miceamaze, you can say just "Bump
Standards-Version, no changes needed".

http://www.debian.org/doc/debian-policy/upgrading-checklist

Yes. Actually I had checked this list and noticed nothing applied for miceamaze.
I have added the changelog entry.
 


You might want to mention the reason for the Priority change in
debian/changelog.

Did I do that? The only thing I changed is "experimental" instead of "unstable".
Is it what you are talking about?
 

I would have indented the second debian/changelog item like this:

  * New upstram release 4.2 (Closes: #766820)
    - Removed creation timestamp in PNG generation (Closes: #778491)

ok fixed
 

ttf-dejavu-core is a transitional dummy package, you could depend on
fonts-dejavu-core | ttf-dejavu-core instead.

fixed
 

You may want to work on porting the code to SDL2. chromium-bsu is an
example of a game that supports SDL and SDL2 (in upstream git only).

Last time I checked, SDL2 was not shipped with most linux distributions
(in stable releases) so I wanted to wait. Hopefully with jessie release, SDL2
will be available, so I might do the swith eventually (I think I will permanently
switch and drop SDL 1.2 compatibility completely when I do that).
 

The debian/patches directory is empty and could be removed.

ok
 

There are two spelling errors:

$ codespell --quiet-level=3
./src/Functions.h:149: occured  ==> occurred
./src/AIVertex.h:33: colum  ==> column

thanks. corrected
 

The include-what-you-use tool (from the iwyu Debian package) suggests
a lot of files that are missing headers for the variables and
functions that they use. I ran this command:

$ find -type f \( -iname '*.c' -o -iname '*.cc' -o -iname '*.cxx' -o
-iname '*.cpp' -o -iname '*.h' -o -iname '*.hh' -o -iname '*.hxx' -o
-iname '*.hpp' \) -exec include-what-you-use {} \;

I'm surprised because gcc never complained about missing includes.
I will look into that later (this is not fixed in this release).
 

I would suggest renaming the upstream README.txt file to INSTALL.txt.

ok done
 

The upstream LICENSE.txt still contains details about the
Bitstream/DejaVu license even though that was removed from the source
tarball.

True. I removed this part.
 

The upstream LICENSE.txt is missing copyright/license info for
maze13.txt, which appears to have been contributed by someone else?

Ok I added information about that.
 

This is a better URL for the Ogg file from ccmixter, I found it in the
metadata of the Ogg file:

http://ccmixter.org/files/George_Ellinas/14073-

Thanks. I added the URL.
 


The upstream ChangeLog.txt is missing information about versions from
2.1 to 4.2.

Yes I tend to forget about this file and only update the website.
But this is now fixed.
 

You've changed the mouse image and introduced 2 modified copies of it.
This could be problematic if you want to tweak the mouse image or use
different modifications in the future. Personally I would have done it
like this: mouse.png containing the normal mouse image, helmet.png
containing the mouse helmet and sickness.png containing the sickness
pattern. Then at build time you would use imagemagick to overlay the
helmet on the mouse image to create drill_mouse.png and similarly to
create sick_mouse.png. Using SVG for the mouse image would give you
more options for modifying the mouse too.

I agress this might have been another option, but actually I did not make
this change myself and the other developped preferred to do like this.
 

The Ogg files aren't very modifiable, for example you could not easily
change the drums to different drum or to a sample of someone hitting
two rocks together.

That's true but I cannot provide anything better because I just downloaded
it like this and did not change anything.
 

lintian complaints:

P: miceamaze source: debian-watch-may-check-gpg-signature
I: miceamaze: arch-dep-package-has-big-usr-share 12285kB 98%

For the first one, refer to these URLs:

https://lintian.debian.org/tags/debian-watch-may-check-gpg-signature.html
https://wiki.debian.org/debian/watch#Cryptographic_signature_verification
https://help.riseup.net/security/message-security/openpgp/best-practices

For the second one, refer to these URLs:

https://lintian.debian.org/tags/arch-dep-package-has-big-usr-share.html

So if I want to fix that, I should build two packages:
miceamaze with the binary file and miceamaze-data with the rest?
 


As you are upstream for a game, you may want to look at these:

https://wiki.debian.org/UpstreamGuide
http://www.freedesktop.org/wiki/Games/Upstream/

--
bye,
pabs

https://wiki.debian.org/PaulWise

bye,
Raphael 


Reply to: