Bug 313461 - Review Request: astromenace - Hardcore 3D space shooter with spaceship upgrade possibilities
Review Request: astromenace - Hardcore 3D space shooter with spaceship upgrad...
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
All Linux
medium Severity medium
: ---
: ---
Assigned To: Hans de Goede
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2007-09-30 22:39 EDT by Jon Ciesla
Modified: 2007-11-30 17:12 EST (History)
5 users (show)

See Also:
Fixed In Version: 1.2-3.fc7
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-10-10 15:34:03 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
hdegoede: fedora‑review+
kevin: fedora‑cvs+

Attachments (Terms of Use)
Original Main.cpp (38.22 KB, application/octet-stream)
2007-10-05 12:17 EDT, Jon Ciesla
no flags Details
openastromenace fork Main.cpp (37.46 KB, application/octet-stream)
2007-10-05 12:18 EDT, Jon Ciesla
no flags Details
Patch bringing in the changes from the fork into the orig main.cpp (782 bytes, application/octet-stream)
2007-10-05 13:43 EDT, Hans de Goede
no flags Details

  None (edit)
Description Jon Ciesla 2007-09-30 22:39:55 EDT
Spec URL: http://zanoni.jcomserv.net/fedora/AstroMenace/AstroMenace.spec
SRPM URL: http://zanoni.jcomserv.net/fedora/AstroMenace/AstroMenace-1.2.0-1.fc7.src.rpm
Description: Space is a vast area, an unbounded territory where it seems there is a
room for everybody, but reversal of fortune put things differently. The
hordes of hostile creatures crawled out from the dark corners of the   
universe, craving to conquer your homeland. Their force is compelling, 
their legions are interminable. However, humans didn't give up without 
a final showdown and put their best pilot to fight back. These malicious
invaders chose the wrong galaxy to conquer and you are to prove it! 
Go ahead and make alien aggressors regret their insolence.
Comment 1 Marc Bradshaw 2007-09-30 23:24:14 EDT
IMO, you should consider placing the data files in a seperate -data package to
help reduce large update downloads.
Comment 2 Hans de Goede 2007-10-01 03:39:59 EDT
(In reply to comment #1)
> IMO, you should consider placing the data files in a seperate -data package to
> help reduce large update downloads.

Since the data files are already in a seperate tarbal upstream, I will even go
as far as to say that you *must* put them in a seperate srpm.

Then if we do bug / packaging fixes to the code the user doesn't need to
redownload all the data again.

Also I think you should package the other languages too (make all languages part
of the data srpm please).
Comment 4 Leszek Matok 2007-10-02 13:27:30 EDT
What you're packaging is not exactly AstroMenace. You're packaging a fork based
of an open portion of AstroMenace and that fork is called OpenAstroManace.
Either package real AM (newer version is available already, without
GAME_VERSION_ID/BUILD change, but with many code cleanups/changes), or change
package name appropriately.

I'm aware that at the moment both games don't differ (most changes in AM look to
fix build warnings), but time will tell if the fork is successful.

BTW, wasn't there a tendency to change package names to lower case
(SysVinit->sysvinit and some others)? Even if not, both upstreams use low-case
file names.
Comment 5 Jon Ciesla 2007-10-02 13:33:32 EDT
Based on the timing of the announcement on f-g-l and the dates on the sf.net
files, I assumed the sf.net project was owned by Viewizard.  If this is not the
case, I can easily change sources to Viewizard's upstream.  The only reason I
went with sf.net was because I figured that way if something happened to
Viewizard, I could keep the same URLs.  I'd actually be happier using the
Viewizard upstream, if for no other reason than to reward them for good
behaviour in GPLing AstroMenace.

I can also change to all lc.  Opinions from the cc: gallery? :)
Comment 6 Hans de Goede 2007-10-02 13:38:35 EDT
I agree that using viewwizard as upstream is better then using then
openastromenace sf.net project, using all lc also gets a +1 from me.
Comment 7 Jon Ciesla 2007-10-02 14:06:30 EDT
Ok, I'll get on that.  FWIW, I don't see anything more recent than 1.2 on the
Viewizrd site, and that's what oamenace is at.
Comment 9 Jon Ciesla 2007-10-03 09:08:43 EDT
Of course, now it doesn't work:

[limb@fawkes ~]$ astromenace --noAA --dir=/usr/share/astromenace/
AstroMenace 1.2 70914

VFS file was opened /usr/share/astromenace/gamedata.vfs
Can't find VFS file /usr/share/astromenace/gamelang.vfs
gamelang.vfs file not found or corrupted.

There is no gamelang.vfs, but there wasn't in oamenace either and it worked.
Comment 10 Hans de Goede 2007-10-05 04:14:14 EDT

Any luck on getting the official Astromenace to run, iow any luck on getting
past the error from comment #9? Need help?
Comment 11 Jon Ciesla 2007-10-05 08:20:16 EDT
I think if I patch Main.cpp to match Main.cpp from the fork, it'll work, because
I found where it's more permissive than the official version.  Problem is, I'm
having trouble generating a patch, as the official Main.cpp has tons of odd
characters.  Any thoughts?
Comment 12 Hans de Goede 2007-10-05 11:47:26 EDT
Probably some kind of encoding problem, if you can attach them both to this bug
I'll take a look.
Comment 13 Jon Ciesla 2007-10-05 12:17:26 EDT
Created attachment 217681 [details]
Original Main.cpp
Comment 14 Jon Ciesla 2007-10-05 12:18:07 EDT
Created attachment 217691 [details]
openastromenace fork Main.cpp
Comment 15 Jon Ciesla 2007-10-05 12:19:19 EDT
There you are.  I could probably just use the forked version of this file only,
but I'd rather patch.
Comment 16 Jon Ciesla 2007-10-05 12:29:25 EDT
Just opened them elsewhere.  That looks like Cyrillic.
Comment 17 Hans de Goede 2007-10-05 13:43:44 EDT
Created attachment 217761 [details]
Patch bringing in the changes from the fork into the orig main.cpp

I could generate a diff between the 2 just fine. Indeed they contain Cyrillic
(probably Russian, seeing how there is a Russian translation) comments, but
thats not the problem. The original one contains CRLF line endings for some
lines, and normal LF only line endings on other, where as the fork only ahs the
normal line endings. I've hand edited the diff to make it smaller, removing any
changes resulting from the line ending differences.

You may need to change these lines:
--- Main.orig.cpp	2007-10-05 19:30:17.000000000 +0200
+++ Main.fork.cpp	2007-10-05 19:32:55.000000000 +0200

To something like:
--- Astromenace-version/src/Main.orig.cpp	2007-10-05 19:30:17.000000000
+++ Astromenace-version/src/Main.fork.cpp	2007-10-05 19:32:55.000000000

So that you can use %patch -p1 as is normal. Note I gzipped the diff to make
sure bugzilla doesn't mess with the line endings.
Comment 18 Jon Ciesla 2007-10-05 14:05:04 EDT
I got it.  Turns out the problem was PuTTY.  Now that I'm using
Gnome-terminal/FreeNX, it's displaying correctly.

I'll try applying it and let you know what I find.  Thanks. :)
Comment 20 Hans de Goede 2007-10-08 09:31:38 EDT
Full review done (sources match upstream, license ok, everything else ok), a
couple of Should Fix's, but no blockers, so its approved!

Should Fix:
-Please always start the Name with a capital in the .desktop file, so:

-Please use opengl-games-utils wrapper to check for missing DRI
  Add: "Requires: opengl-games-utils"
  Add to %install:
  "ln -s opengl-game-wrapper.sh $RPM_BUILD_ROOT%{_bindir}/%{name}-wrapper"
  Add "%{_bindir}/%{name}-wrapper" to files
  Change the .desktop file Exec entry from "%{name}" to "%{name}-wrapper"

-Once build please add this to the games-live .ks file:

-Please change the default resolution to 640x480 (it defaulted to my desktop 
 resolution). With my desktop resolution its unplayable with my somewhat under
 powered 3d card (intel 965) on 640x480 it rocks on at 15 fps!
Comment 21 Jon Ciesla 2007-10-08 14:34:04 EDT
Thanks!  Shoulds addressed.

New Package CVS Request
Package Name: astromenace
Short Description: Hardcore 3D space shooter with spaceship upgrade possibilities
Owners: limb
Branches: FC-6 F-7
Cvsextras Commits: yes
Comment 22 Kevin Fenzi 2007-10-08 14:54:13 EDT
cvs done.
Comment 23 Fedora Update System 2007-10-10 15:34:01 EDT
astromenace-1.2-3.fc7 has been pushed to the Fedora 7 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.