Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 185211 - Review Request: prboom - GPL doom game engine
Summary: Review Request: prboom - GPL doom game engine
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Hans de Goede
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 185212
TreeView+ depends on / blocked
 
Reported: 2006-03-11 20:45 UTC by Wart
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-03-14 20:38:28 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch fixing 64 bit warnings (10.40 KB, patch)
2006-03-14 17:11 UTC, Hans de Goede
no flags Details | Diff

Description Wart 2006-03-11 20:45:46 UTC
Spec Name or Url: http://www.kobold.org/~wart/fedora/prboom.spec
SRPM Name or Url: http://www.kobold.org/~wart/fedora/prboom-2.3.1-1.src.rpm
Description: 
An open-source DOOM game engine.

This has a Require: on 'doom-data', which is provided by the freedoom package.  It also a Provides: doom-engine so that we can have multiple doom engines installed and the game files can require that any of them are present.

Comment 1 Wart 2006-03-11 20:50:28 UTC
Adding Hans to cc: per his request.

Comment 2 Hans de Goede 2006-03-12 12:24:19 UTC
Argh, I accidently hit backspace without the textinput having focus and firefox
interpreted this as go back one page and now my review is gone. GRRRRR.

Anyways trying again:

MUST
====
* Source tarball matches upstream
* Package (and .spec) named properly
* License file included
* Spec file readable, in Am. English
* Compiles and builds on FC-5 x86_64, but with warning see MUST-FIX
* No locale files
* No excessive BR: or Requires:
* Not relocatable
* buildroot cleaned up in %install and %clean
* No duplicate files
* permissions ok
* Macro usage consistent
* Contains code, not content
* No headers, api docs, and .so library
* No .la archives
* No desktop file needed


MUST-FIX
========
* Building gives a number of "integer <-> pointer of different size cast"
  warnings these are _BAD_ on x86_64. I'll attach a patch fixing these.
  One of these is a real bu, the others were ok.
* %{_datadir}/prboom is not owned by the package
* why the %dir %{_datadir}/games/doom ?


About provide / req doom-game/-engine and Desktop files
=======================================================
I've been thinking about this and initially I came up with the following:
-generic doom engines and doom data provide doom-engine resp doom-data
-generic doom engines are set up using the alternatives system and can be called
 as just doom through alternatives
-doom-data packages include the .desktop file and icons, the .desktop
 file invokes "doom" with cmdlines arguments to use their wad. This way if 
 people install multiple versions of the data (free-doom, heretic-shareware, 
 doom-shareware) they get menu entries for each.

Special cases:
-certain doom-data packages will not work with all doom-engines. This is
 true for free-doom which requires prdoom. These will require the correct
 engine instead of the generic doom-engine. And these provide specific 
 engine-data. So free-doom would provide prboom-data
-In order for the special doom-data-packages to be sufficient for the 
 requirements of their needed engines, engines which have doom-data-packages
 which only work with them will require engine-data instead of doom-data
 thus prboom will require prboom-data
-Generic doom-data packages such as doom-shareware will thus not only need to
 provide the generic doom-data but also specific engine-data for engines which
 have a specific require, so that they will meet this require too.
-The .desktop file in an engine specific doom-data-package will not use the
 doom "alternative" but instead call the needed engine directly.


But this is /becomes a mess so I suggest instead:
-doom-engines provide dataname-engine for all datasets they support
-doom-engines require enginename-data
-doom-data provide enginename-data for all engines they are known to work
 with
-doom-data requires dataname-engine
-doom-data packages include the .desktop file and icons, the .desktop
 file invokes "dataname" wich is a wrappercript provided by the engine
 with the correct cmdline arguments to use the relevant wad. If it is possible
 that there are multiple providers of dataname-engine then the alternatives
 system will be used.
 
 This way (desktop-file in data package) if people install multiple versions of
 the data (free-doom, heretic-shareware,  doom-shareware) they get menu entries
 for each.
-datafiles packages are named as the game and will show up in comps for easy
 user selection (the reqs will drag in an engine). so the free-doom data 
 package will be called just free-doom, doom-shareware-package will be called 
 doom19-shareware, etc.

I hope this makes sense and you think its a good idea :)

Comment 3 Wart 2006-03-13 08:03:22 UTC
(In reply to comment #2)
[...]

> MUST-FIX
> ========
> * Building gives a number of "integer <-> pointer of different size cast"
>   warnings these are _BAD_ on x86_64. I'll attach a patch fixing these.
>   One of these is a real bu, the others were ok.

Thanks for catching this.  It would be nice if there were a flag for gcc that
would treat these "integer <-> pointer" warnings as errors, esp. on x86_64.

> * %{_datadir}/prboom is not owned by the package
> * why the %dir %{_datadir}/games/doom ?

%{_datadir}/games/doom was supposed to be %{_datadir}/prboom.  My bad.  I've
uploaded a new .spec with this fix.

> About provide / req doom-game/-engine and Desktop files
> =======================================================
> I've been thinking about this and initially I came up with the following:
[...]
> But this is /becomes a mess so I suggest instead:
> -doom-engines provide dataname-engine for all datasets they support

I'm not too keen on this because it means that every time a new iwad becomes
available, the game engine package must be updated to signal that it can run it.
 The game engine package should not have to know about all of the possible
dataname packages that it can run.

> -doom-engines require enginename-data
> -doom-data provide enginename-data for all engines they are known to work
>  with
> -doom-data requires dataname-engine
> -doom-data packages include the .desktop file and icons, the .desktop
>  file invokes "dataname" wich is a wrappercript provided by the engine
>  with the correct cmdline arguments to use the relevant wad. If it is
possible>  that there are multiple providers of dataname-engine then the
alternatives
>  system will be used.

I'm starting to think that using the alternatives system for game engines might
be a little bit of overkill and introduce some unneeded complexity.  If the game
data was designed to work with a specific game engine, then the .desktop file
should reflect that.  If the game data can work with an alternate game engine,
then that's great, but the user will have to do so from the command line.

>  This way (desktop-file in data package) if people install multiple versions of
>  the data (free-doom, heretic-shareware,  doom-shareware) they get menu entries
>  for each.
> -datafiles packages are named as the game and will show up in comps for easy
>  user selection (the reqs will drag in an engine). so the free-doom data
>  package will be called just free-doom, doom-shareware-package will be called
>  doom19-shareware, etc.

+1

> I hope this makes sense and you think its a good idea :)

It seems that all of this complexity is being added (and I admit that I started
this messy discussion) only to support some level of indirection in the .desktop
file.  There are really only two simple requirements that I feel we must satisfy:

1) Each iwad has a usable .desktop entry so it can be run from the menu

2) Additional game engines can be installed to run the games

If we don't try to mix these requirements (don't allow alternate game engines in
the .desktop files) then it becomes much simpler and can be satisfied by a
subset of your suggestion.

Each game data package Requires: <enginname>, even if it is known to work with
others.  That game engine is hardcoded in the .desktop file and will be used
when the game is launched from the menu.  This ensures that users can 'yum
install <gamedata>' and end up with the data, game engine, and a usable .desktop
icon.

Game data Provides: <enginename>-data for each engine that it is known to work
with.  Each game engine Requires: <enginename>-data so that when the engine is
installed, it will pull in the data that can be played with the game.  However,
if the game data package Requires: a different enginename, then you might end up
with a situation where "yum install engine2" results in the installation of
engine2, game-data1, and engine1.  It's a little odd this way, but it does
ensure that both 'yum install enginename' and 'yum install dataname' still end
up with working .desktop icons.

Alternately, we can try to relax the "game engine requires game data" packaging
requirement for game engines like prboom that are designed to work with multiple
game data packages.  There are really two classes of game engines.  The first
class of game engine is very self-contained, such as 'tong'.  In this class of
game engine, the game data is just 'skin', such as images and sound.  The
purpose of splitting the game data from the game engine in this case is to
minimize the download size of package updates, not to provide alternate skins
for the game. While alternate skins could be made available, it is unlikely to
occur.  Such game engines should require game data in order to be played.

The second class of game engine are more 'game interpreters' like prboom, for
which the game data packages provide a new game experience with new game logic.
 For example, 'freedoom' is more than a cosmetic change from the shareware doom
game.  Such engines should not have to require game data files, since the data
files contain a large portion of the game logic.  In this case, the game data is
more than just skin, and it is expected that alternate game data packages will
be available.  They should only require that such data files exist and are
available through the packaging system.

I apologize if this sounds like rambling.  I'll try to clarify anything if I've
muddied it up too much.  :)


Comment 4 Hans de Goede 2006-03-13 16:20:38 UTC
>> About provide / req doom-game/-engine and Desktop files
>> =======================================================
>> I've been thinking about this and initially I came up with the following:
[...]
>> But this is /becomes a mess so I suggest instead:
>> -doom-engines provide dataname-engine for all datasets they support
>
>I'm not too keen on this because it means that every time a new iwad becomes
>available, the game engine package must be updated to signal that it can run it.
> The game engine package should not have to know about all of the possible
dataname packages that it can run.

I get your point, but what are the chances of new iwads comming out? We need to
only take into account iwads which _might_ be packaged. So for prboom that would
be free-doom and doom-shareware. For vavoom it would be doom-shareware and
heretic-shareware. If people have registered versions they will need to install
them themselves, we can't provide packages for thus we don't need provides
doomxx-registered-engine. and besided the shareware versions and the free
versions I don't know of any other iwads, but that could be me.

Hmm, after doing a search I've found many interesting conversions, but these are
all pwads and most don't have a clear license. Still lets assume some have an ok
license, how do these fit in?

About your other comments, I tend to agree with your simpeler solution (which
would make my comment above void) but I'm not sure yet, I need to think a bit
more about this one.



Comment 5 Wart 2006-03-13 18:30:33 UTC
(In reply to comment #4)
> >> About provide / req doom-game/-engine and Desktop files
> >> =======================================================
> >> I've been thinking about this and initially I came up with the following:
> [...]
> >> But this is /becomes a mess so I suggest instead:
> >> -doom-engines provide dataname-engine for all datasets they support
> >
> >I'm not too keen on this because it means that every time a new iwad becomes
> >available, the game engine package must be updated to signal that it can run it.
> > The game engine package should not have to know about all of the possible
> dataname packages that it can run.
> 
> I get your point, but what are the chances of new iwads comming out? We need to
> only take into account iwads which _might_ be packaged. So for prboom that would
> be free-doom and doom-shareware. For vavoom it would be doom-shareware and
> heretic-shareware. If people have registered versions they will need to install
> them themselves, we can't provide packages for thus we don't need provides
> doomxx-registered-engine. and besided the shareware versions and the free
> versions I don't know of any other iwads, but that could be me.

You've got a valid point.  There are a very limited number of iwads that we can
include in FE.

> Hmm, after doing a search I've found many interesting conversions, but these are
> all pwads and most don't have a clear license. Still lets assume some have an ok
> license, how do these fit in?

I was thinking about the pwad situation last night but didn't have enough
information to formulate a coherent plan.  While iwads are entire games (new
levels, sounds, sprites, music), pwads (also known as patch wads) replace only
some components of an iwad, though some of the better ones end up replacing
almost everything.  pwads require that an iwad be present in order to be played.
 I think the best way to handle pwads is to have a graphical launcher that lets
the user select from all of the installed pwads.  There were many of these
launchers for DOS back when doom was still new.  I found one for Linux, and
there may be more:  http://forums.newdoom.com/showthread.php?t=20863.  This
particular launcher lets you select the engine as well as the iwad and pwad.

> About your other comments, I tend to agree with your simpeler solution (which
> would make my comment above void) but I'm not sure yet, I need to think a bit
> more about this one.

I now think that a separate launcher is the way to go.  We can use an explicit
engine -> iwad and iwad -> engine requires (such as prboom requires freedoom,
freedoom requires prboom) with .desktop files to handle the simple case.  The
launcher is a separate package that lets the user select from multiple installed
engines, iwads, and pwads.

iwads and pwads would have to be installed in well-known locations so that the
launcher can pick them up automatically.  This may also require modifying the
launcher to look in these well-known locations automatically.

Comment 6 Nicolas Mailhot 2006-03-13 18:57:05 UTC
> If people have registered versions they will need to install
> them themselves, we can't provide packages for thus we don't need provides
> doomxx-registered-engine.

Actually if you really wanted to do it and stay on the good side of the law,
it's perfectly possible to provide users with nosrc.rpm

So it's not a technical/legal limit, it depends entirely on how far you wish to go.

Comment 7 Hans de Goede 2006-03-13 23:05:43 UTC
As promised I've done some more thinking, here are my conclusions which are much
in line with yours:
-doom iwad packages are leading, the get listed in comps, they have a desktop 
 file which directly calls the prefered engine for the iwad and the require the
 prefered engine.
-all iwads go into one dir, I suggest /usr/share/doom this way all engines
 can be modified to search here by default making them work from the cmdline
 without args too, and this dir can be used by launchers if / once we add those.
-doom-engines need to be usable when installed, thus they should require an 
 doom-iwad package, preferably one which has them as the prefered engine, but
 if no such package exists then any iwad-package will do.
-doom-engines will be "modified" so that they search /usr/share/doom by default,
 and  so that try to open the iwad they require. Prboom f.e. should try to open
 /usr/share/doom/free-doom.wad (not nescesarry the first one it tries to open).
 This is so that they will work when started from the console without any args.
-pwads which are really new games (conversions) are treated as iwads except that
 they should require the nescesarry iwad. (we will need to try these pwads
 against free-doom and if they work get their license clarified first)
-we will come up with something for pwads which are just levels later
 (and again we need tp get their license clarified first).

I hope you agree with this and then can modify prboom and free-doom to match
this, then I'll complete the review of prboom and start on free-doom.


Comment 8 Wart 2006-03-13 23:36:03 UTC
(In reply to comment #7)
[...]
> -pwads which are really new games (conversions) are treated as iwads except that
>  they should require the nescesarry iwad. (we will need to try these pwads
>  against free-doom and if they work get their license clarified first)
> -we will come up with something for pwads which are just levels later
>  (and again we need tp get their license clarified first).

IMO, the "conversion" pwads and minor patch pwads are treated the same (each
gets their own subdir in /usr/share/doom), except that the conversion pwads will
also have a .desktop file.  The minor pwads will have to be selected either on
the command line or through a launcher.

> I hope you agree with this and then can modify prboom and free-doom to match
> this, then I'll complete the review of prboom and start on free-doom.

I knew we would eventually converge on a decent solution.  :)

I'll work on updates to prboom and freedoom to reflect the results of this
discussion.

Comment 9 Wart 2006-03-13 23:50:51 UTC
I hit 'save' too quickly...

(In reply to comment #8)
> (In reply to comment #7)
> [...]
> > -pwads which are really new games (conversions) are treated as iwads except that
> >  they should require the nescesarry iwad. (we will need to try these pwads
> >  against free-doom and if they work get their license clarified first)
> > -we will come up with something for pwads which are just levels later
> >  (and again we need tp get their license clarified first).
> 
> IMO, the "conversion" pwads and minor patch pwads are treated the same (each
> gets their own subdir in /usr/share/doom), except that the conversion pwads will
> also have a .desktop file.  The minor pwads will have to be selected either on
> the command line or through a launcher.

I meant to delete the parenthetical about subdirectories.  No subdirectories are
necessary.

Comment 10 Wart 2006-03-14 00:05:53 UTC
Changes to the game data files directory, per the above discussion.  The
-gamedir patch has changed slightly as a result.

http://www.kobold.org/~wart/fedora/prboom-2.3.1-3.src.rpm
http://www.kobold.org/~wart/fedora/prboom.spec

Comment 11 Hans de Goede 2006-03-14 08:38:08 UTC
Looks good, but you forgot to add my patch fixing all the 64 bit warnings, once
that is done I'll approve it.


Comment 12 Wart 2006-03-14 15:22:52 UTC
(In reply to comment #11)
> Looks good, but you forgot to add my patch fixing all the 64 bit warnings, once
> that is done I'll approve it.

I haven't seen this patch yet.   Can you resubmit it?

Comment 13 Hans de Goede 2006-03-14 17:11:26 UTC
Created attachment 126110 [details]
patch fixing 64 bit warnings

Woops look like I never attached it, that would explain its absence, sorry.

Comment 15 Hans de Goede 2006-03-14 18:57:05 UTC
One word: APPROVED


Comment 16 Wart 2006-03-14 20:38:28 UTC
Imported and built.  Thanks!


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