Bug 313461

Summary: Review Request: astromenace - Hardcore 3D space shooter with spaceship upgrade possibilities
Product: [Fedora] Fedora Reporter: Gwyn Ciesla <gwync>
Component: Package ReviewAssignee: Hans de Goede <hdegoede>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora, fedora-package-review, hdegoede, lam, notting
Target Milestone: ---Flags: hdegoede: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.2-3.fc7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2007-10-10 19:34:03 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Original Main.cpp
none
openastromenace fork Main.cpp
none
Patch bringing in the changes from the fork into the orig main.cpp none

Description Gwyn Ciesla 2007-10-01 02:39:55 UTC
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-10-01 03:24:14 UTC
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 07:39:59 UTC
(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 17:27:30 UTC
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 Gwyn Ciesla 2007-10-02 17:33:32 UTC
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 17:38:35 UTC
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 Gwyn Ciesla 2007-10-02 18:06:30 UTC
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 Gwyn Ciesla 2007-10-03 13:08:43 UTC
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 08:14:14 UTC
Jon,

Any luck on getting the official Astromenace to run, iow any luck on getting
past the error from comment #9? Need help?


Comment 11 Gwyn Ciesla 2007-10-05 12:20:16 UTC
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 15:47:26 UTC
Probably some kind of encoding problem, if you can attach them both to this bug
I'll take a look.


Comment 13 Gwyn Ciesla 2007-10-05 16:17:26 UTC
Created attachment 217681 [details]
Original Main.cpp

Comment 14 Gwyn Ciesla 2007-10-05 16:18:07 UTC
Created attachment 217691 [details]
openastromenace fork Main.cpp

Comment 15 Gwyn Ciesla 2007-10-05 16:19:19 UTC
There you are.  I could probably just use the forked version of this file only,
but I'd rather patch.

Comment 16 Gwyn Ciesla 2007-10-05 16:29:25 UTC
Just opened them elsewhere.  That looks like Cyrillic.

Comment 17 Hans de Goede 2007-10-05 17:43:44 UTC
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
+0200
+++ Astromenace-version/src/Main.fork.cpp	2007-10-05 19:32:55.000000000
+0200

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 Gwyn Ciesla 2007-10-05 18:05:04 UTC
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 13:31:38 UTC
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:
 Name=Astromenace

-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:
 http://fedoraproject.org/wiki/SIGs/Games/GamesLive

-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 Gwyn Ciesla 2007-10-08 18:34:04 UTC
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
InitialCC: 
Cvsextras Commits: yes

Comment 22 Kevin Fenzi 2007-10-08 18:54:13 UTC
cvs done.

Comment 23 Fedora Update System 2007-10-10 19:34:01 UTC
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.