Bug 185211
Summary: | Review Request: prboom - GPL doom game engine | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Wart <wart> | ||||
Component: | Package Review | Assignee: | Hans de Goede <hdegoede> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Package Reviews List <fedora-package-review> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | hdegoede | ||||
Target Milestone: | --- | ||||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2006-03-14 20:38:28 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: | |||||||
Bug Depends On: | |||||||
Bug Blocks: | 163779, 185212 | ||||||
Attachments: |
|
Description
Wart
2006-03-11 20:45:46 UTC
Adding Hans to cc: per his request. 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 :) (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. :) >> 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. (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. > 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.
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. (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. 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. 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 Looks good, but you forgot to add my patch fixing all the 64 bit warnings, once that is done I'll approve it. (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? Created attachment 126110 [details]
patch fixing 64 bit warnings
Woops look like I never attached it, that would explain its absence, sorry.
New package with the 64-bit patch: http://www.kobold.org/~wart/fedora/prboom-2.3.1-4.src.rpm http://www.kobold.org/~wart/fedora/prboom.spec One word: APPROVED Imported and built. Thanks! |