http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 David Timms <dtimms(a)iinet.net.au&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dtimms(a)iinet.net.au Blocks|2 |3 AssignedTo|rpmfusion-package- |dtimms(a)iinet.net.au |review(a)rpmfusion.org | --- Comment #1 from David Timms <dtimms(a)iinet.net.au&gt; 2009-01-05 13:17:16 --- Review for snowballz: ===== OK no problemo NA not applicable ?? can't say/don't understand. ! needs work. ===== [OK] rpmlint .src.rpm : 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [! ] rpmlint /home/davidt/rpmbuild/RPMS/noarch/snowballz-1.0-0.1.beta1.20081222hg.fc10.noarch.rpm : snowballz.noarch: E: zero-length /usr/share/games/snowballz/data/maps/crystalcrises/dynamic_objects.txt snowballz.noarch: E: zero-length /usr/share/games/snowballz/data/maps/world1/dynamic_objects.txt snowballz.noarch: E: zero-length /usr/share/games/snowballz/data/maps/td1/dynamic_objects.txt snowballz.noarch: E: zero-length /usr/share/games/snowballz/data/maps/crystalcrises/script_areas.txt 1 packages and 0 specfiles checked; 4 errors, 0 warnings. Are these zero length files required for snowballz to work ? [OK] included source is pulled from hg so direct md5sum compare isn't possible. Since package includes a snowballz-snapshot script, this was used to retrieve 1.0beta1 from the mercurial repo, which created a source archive with today's date. While these two archives differ slightly in size, I think that is to be expected because the file dates within the archive are stored - and this probably leads to different compression levels. The freshly retrieved archive was extracted, and the included source rpm archive also extracted. diff -ur between the two extracted trees shows no differences. [OK] package name is ok, it's the only user in a web search. [OK] spec named %name.spec as required [OK] source tarball contains no prebuilt binaries/libraries. [OK] files placed into FHS locations - meets fedora python standard [OK] changelog in standard format [OK] correctly omits Packager, vendor, copyright, prereq , includes license tag. [OK] summary is <80 chars, no ending period [OK] source0 is a direct archive made with the snapshot script that retrieves it from mercurial repo. [OK] buildroot set at second most preferred location: [OK] %install correctly erases buildroot before build [OK] mock build succeeds. [OK] rpmdiff between default build and mock build shows only time {T} differences for folders and the pre-compiled .py[co] files [OK] description is column limited to <80 chars. [OK] man 6 is included. [OK] charset is ascii [OK] debuginfo not expected in noarch application. [OK] no static libraries, rpath, self copies of already packaged libaries etc, since pure python project. [OK] no config, initscripts, [OK] variable style is used consistently [NA] not multilingual [OK] timestamps are kept [NA] make is python based, so smp_flags not required. [OK] build requires python. [OK] no conditional dependencies [OK] is program code and game data (graphics, sounds) [NA] no provides to consider [OK] dirs/files owned by the package [OK] not a web app, shouldn't conflict with other packages. [NA] python sitelib applies only to python libraries [OK] eggs are built from source into binary rpm. - dont know much about this, but the folder is created with various info files inside it, so looks OK. [OK] no files in %{_bindir} and %{_sbindir}. [OK] %install setup.py install --skip-build. as guided for python packages. [OK] %files: - is it normal to stick games in ../games/%{name} ? (Well I see fortune and torcs are, and frozen-bubble isn't so I guess I answered that.) - the icon, manual, and desktop entry are specifically noted, while the ../games/snowballz/ folder is defined, so this looks good to me. [NA] there are no unit tests, that I can see. --- issues: [! ] Summary: A fun RTS game featuring snowball fights with penguins - It might be worth expanding RTS into what it means (for those that don't know), [!] desktop files has been created. It uses a "beta" trailer in the title, which is useful to inform players that there could be problems in the game. The groups meet freedesktop standard, as required. - GenericName=RTS Game Maybe: Graphical Real Time Strategy Game - Comment=Command your army of penguins Maybe: A game where you command an army of penguins [! ] license "GPLv2+ and MIT": the only licensing I can see is in: snowui/__init__.py but it isn't a well known license. Why did you select these as the license ? [??] in setup, ttf fonts are removed. Would it be better to remove them (TSCu_Comic.ttf) in the snapshot creation process, so that they don't make it into the .src.rpm ? This might make it more difficult once there are release source archives. Is the reason to remove it because it's license is unclear ? A comment to describe why would help readers. Do you need to (somehow) replace the font with font already in Fedora ? [??] no scriptlets why would you build the wrapper script during setup, rather than separately storing the script in cvs ? Would it be better to be able to keep the creation time from cvs ? [! ] waiting on Requires: pyglet to be able to install built package and test functionality. ==== Needs work, and or brief spec file comments for some choices youhave made, in my opinion, and waiting on other reviews. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. You are the assignee for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #3 from Orcan Ogetbil <oget.fedora(a)gmail.com&gt; 2009-01-10 16:03:26 --- (In reply to comment #1) [...] Afaict yes. Removing them makes the game misbehave. I wonder why this didn't show up when I ran rpmlint the first time. I think it's OK. This is the first game I ever packaged though. I'm open to suggestions. done Isn't it obvious that it is a "game"? Also, we already used the word "game" in the Generic Name section. What's your opinion? It is MIT, Modern Style with sublicense (see https://fedoraproject.org/wiki/Licensing/MIT) I should remove the GPLv2+ since I removed that font (which is GPLv2+). Done. Inclusion of fonts in non-font packages is forbidden in Fedora now. I worked on packaging this font separately. After some consultations to the fonts guys in Fedora and Debian, we figured that this font is incomplete and has a wrong table, and hence we shouldn't package it in Fedora as is. I worked on it further. I corrected the table and added more characters. Here's the project I've been working on: http://serafettin.sourceforge.net/ I will package this for Fedora at some point. The font is not used in the program. I believe it is because the game is in the production stage. It may be used later when the stable version comes up. Then we can replace it with some other font from Fedora. I'm removing the font during the snapshot creation process now. I guess you're right. Changed. Let me know about the Comment in the .desktop file. Here are the latest files: SPEC: http://oget.fedorapeople.org/review/snowballz.spec SRPM: http://oget.fedorapeople.org/review/snowballz-1.0-0.2.beta1.20090110hg.fc... Thanks again, for the review. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #5 from Kevin Kofler <kevin.kofler(a)chello.at&gt; 2009-01-10 18:51:53 --- /usr/games is obsolete in Fedora, I think the games still using it should be fixed. In practice, GNOME only shows Comment, KDE only shows GenericName, so information should be repeated between those, as users will only see one or the other. GenericName should be just a short noun phrase ("Real-time Strategy Game" is about the longest which makes sense), Comment can be longer because GNOME shows it as a tooltip. And yes, I think KDE should be ultimately fixed to display Comment as a tooltip and GNOME to display the GenericName. I'll see what I can do on the KDE side. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #7 from Orcan Ogetbil <oget.fedora(a)gmail.com&gt; 2009-01-11 18:58:52 --- Updated the files: SPEC: http://oget.fedorapeople.org/review/snowballz.spec SRPM: http://oget.fedorapeople.org/review/snowballz-1.0-0.3.beta1.20090110hg.fc... Changelog: 1.0-0.3.beta1.20090110hg - More changes in the .desktop file (Comment and GenericName) - Game files are installed in %%{datadir}/%%{name} instead of %%{datadir}/games/%%{name} - Remove the egg - Wrapper script supplied internally once again David, I had to put the wrapper script back into the SPEC file. I remembered why I did it this way. The wrapper script contains the line cd %{_datadir}/%{name} If I was to supply this script externally, then I have to hard-code the %{_datadir} into it, which is not nice. Therefore the script is produced during package build. If anyone has a better solution, please let me know. Andrea, %{python_sitelib} and %{python_sitearch} are for shared python modules only. This is a python application, not a shared module. There are bunch of python applications installed directly in %{_datadir}, for instance: rpmlint, system-config-*, PackageKit, yumex, etc. By the way, after some consultation in #fedora-devel, I learned that the eggs are only needed for python modules that are installed in site-packages. Hence I also removed the python egg that was created during the build. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #9 from Orcan Ogetbil <oget.fedora(a)gmail.com&gt; 2009-01-12 15:08:14 --- OK I did it that way: SPEC: http://oget.fedorapeople.org/review/snowballz.spec SRPM: http://oget.fedorapeople.org/review/snowballz-1.0-0.4.beta1.20090110hg.fc... -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #10 from David Timms <dtimms(a)iinet.net.au&gt; 2009-02-02 22:08:35 --- (In reply to comment #9) Hi, I hadn't noticed that pyglet and python-rabbyt are available in the testing repo, with those installed: [OK] rpmbuild succeeds [OK] rpmlint: 4x warnings about zero length files - you have already explained that they seem to be required for normal operation [! ] package works as described / expected - when run from the desktop icon, nothing visible is shown. - when run from a user account: ===== snowballz Traceback (most recent call last): File "snowballz.py", line 2, in <module> from lib.scene_handler import SceneHandler File "/usr/share/snowballz/lib/scene_handler.py", line 4, in <module> from pyglet.window import Window File "/usr/lib/python2.5/site-packages/pyglet/window/__init__.py", line 1682, in <module> gl._create_shadow_window() File "/usr/lib/python2.5/site-packages/pyglet/gl/__init__.py", line 491, in _create_shadow_window _shadow_window = Window(width=1, height=1, visible=False) File "/usr/lib/python2.5/site-packages/pyglet/window/xlib/__init__.py", line 474, in __init__ super(XlibWindow, self).__init__(*args, **kwargs) File "/usr/lib/python2.5/site-packages/pyglet/window/__init__.py", line 641, in __init__ raise NoSuchConfigException('No standard config is available.') pyglet.window.NoSuchConfigException: No standard config is available. ===== Is that what you have found ? Is it that the user is expected to build some configs for pyglet ? Or this snapshot has issues ? -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #12 from David Timms <dtimms(a)iinet.net.au&gt; 2009-02-02 22:40:46 --- (In reply to comment #11) rpm -qa pyglet mesa-libGLU-devel mesa-libGL-devel snowballz python-rabbyt \*nvi\*|sort akmod-nvidia-173xx-173.14.16-1.fc10.i686 kmod-nvidia-173xx-2.6.27.12-170.2.5.fc10.i686-173.14.16-1.fc10.i686 kmod-nvidia-173xx-2.6.27.9-159.fc10.i686-173.14.15-1.fc10.7.i686 mesa-libGL-devel-7.2-0.15.fc10.i386 mesa-libGLU-devel-7.2-0.15.fc10.i386 pyglet-1.1.2-5.fc10.noarch python-rabbyt-0.8.2-8.fc10.i386 snowballz-1.0-0.4.beta1.20090110hg.fc10.noarch xorg-x11-drv-nvidia-173xx-173.14.16-1.fc10.i386 xorg-x11-drv-nvidia-173xx-libs-173.14.16-1.fc10.i386 nvidia fx5600 uname -a Linux davidtdesktop 2.6.27.12-170.2.5.fc10.i686 #1 SMP Wed Jan 21 02:09:37 EST 2009 i686 athlon i386 GNU/Linux That wants to remove a heap of devel packages: http://fedora.pastebin.com/m15bdaecd so I did a rpm --nodeps instead: rpm -e mesa-libGL-devel mesa-libGLU-devel --nodeps After that, snowballz does startup. Guess I'll have to read the manual on how to actually play :) -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #14 from Orcan Ogetbil <oget.fedora(a)gmail.com&gt; 2009-02-03 02:11:00 --- Created an attachment (id=89) --> (http://bugzilla.rpmfusion.org/attachment.cgi?id=89) pyglet lib.py try-out -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #16 from Orcan Ogetbil <oget.fedora(a)gmail.com&gt; 2009-02-03 18:24:39 --- Thank you. I tagged and built pyglet with a new patch. What do you think of the game? It looks promising to me, but it is certainly not very playable. Also, I don't see any updates on the project (not even in their hg) in the last 5 months. It looks like it might be abandoned. I'm not sure if I should revert back to the 0.9.5 version. That one looks more playable. I also get segfaults sometimes with this 1.0 version. Did you experience segfaults? -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #18 from David Timms <dtimms(a)iinet.net.au&gt; 2009-02-03 22:28:10 --- (In reply to comment #16) Agreed, it is obvious that a lot of work was required for the developers to get this far. However, with some insight from a person I knew who get into mod development for a commercial game, it takes serious amount of hours to build stuff like this from scratch; that person eventually decided to work on the game full time, so that she could make the progress she wanted. I would agree, I definitely never really made a dent in the opposing teams army. In the manual, various operating keys are mentioned, but I wasn't able to get any of them to work (eg page up/down, ,.> tab etc). I am not normally into these type of games in any case. The upstream site does mention the call for developers to help with certain tasks. I think it would be good to have at least an F1 help, or maybe an extended package description that provides this info. Is the packaging useful for devels wanting to work on the upstream project ? - the binary rpm works in it's current state, so is useful to make it easy for people to decide whether they might be interested to work on the project. - the .src.rpm could be described as an easy way for developers to get started on snowballz, without the extra learning curve of hg. - we need to make sure that it is clear in the packaging that the above is the case; I we don't want to disappoint potential users, rather pull at their "how can I help" strings ... No segfaults seen by me with my limited testing. Perhaps their is a special part of the game where the issue occurs, or with just one of the maps ? Possible the package version should include "alpha" or "developer demo", so that it is clear that it is really a call to arms. Regarding the package itself, previous issues regarding: - names in the desktop file - wrapper script - license clarification - install path - use of eggs have been taken care of. As far as I can see, it is just waiting on the updated pyglet to become available in the repo, and perhaps more explicitly mentioning the development state of the game, perhaps in the .desktop comment, and in the package summary. I'm guessing the other semi-reviewers were happy with the rest of the packaging, since I am ready to approve it ;-) -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 --- Comment #20 from David Timms <dtimms(a)iinet.net.au&gt; 2009-02-09 21:46:58 --- (In reply to comment #19) Since that is 404, comments refer to: http://oget.fedorapeople.org/review/snowballz-1.0-0.5.beta1.20090110hg.fc... No, that should be obvious enough if someone is using PK install. [OK] runs as advertised: I see that pyglet is now in updates, and a newer pyglet is in updates-testing. With the updates one, snowballz won't start, as I saw a few comments ago (with GL -devel packages installed). With the newer pyglet snowballz runs OK. Again, no crashes in my brief testing. [OK] matches upstream source: since the source archive date changed since my earlier checkout, the archive was re-extracted, and diff -ur'd: diff -ur snowballz/ snowballz-1.0-0.5.beta1.20090110hg.fc10.src/snowballz\ \(2\)/ Only in snowballz/data: TSCu_Comic.ttf - as Fedora font requirements require fonts to be in external packages, the font was removed. ===== This package, snowballz-1.0-0.5.beta1.20090110hg.fc10.src.rpm is APPROVED by dtimms. -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.

http://bugzilla.rpmfusion.org/show_bug.cgi?id=269 Thorsten Leemhuis <fedora(a)leemhuis.info&gt; changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks|33 | --- Comment #22 from Thorsten Leemhuis <fedora(a)leemhuis.info&gt; 2009-02-13 18:10:30 --- (In reply to comment #21) done -- Configure bugmail: http://bugzilla.rpmfusion.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug.