Bug 185721

I just realized the .desktop file is missing.  I'll add it shortly.

.desktop file included:

http://www.kobold.org/~wart/fedora/yadex-1.7.0-2.src.rpm
http://www.kobold.org/~wart/fedora/yadex.spec

Question: Why the Requires: freedoom?

Problem on FC-5:
src/wadlist.cc: In member function 'void Wad_list::del()':
src/wadlist.cc:178: error: no match for 'operator=' in
'((Wad_list*)this)->Wad__list::priv->Wad_list_priv::iter = 0'
/usr/lib/gcc/x86_64-redhat-linux/4.1.0/../../../../include/c++/4.1.0/bits/stl_list.h:112:
note: candidates are: std::_List_iterator<boost::shared_ptr<Wad_file> >&)

I'll see if I can fix this, also some 64 bit related warnings to check out
(printf argument size).

Created attachment 126273 [details]
Patch fixing gcc41 compile

I might find more problems, I justed wanted to share this one asap, so that we
don't do double work.

(In reply to comment #3)
> Question: Why the Requires: freedoom?

Yadex won't run without an iwad, so I modified the default configuration to use
the freedoom iwad.  Without an iwad yadex will complain and quit, and I
guarantee that will result in a bug report the first time someone tries to use
it.  :)  Best to avoid the issue and Requires: an iwad.

(In reply to comment #3)
> Problem on FC-5:
> src/wadlist.cc: In member function 'void Wad_list::del()':
> src/wadlist.cc:178: error: no match for 'operator=' in
> '((Wad_list*)this)->Wad__list::priv->Wad_list_priv::iter = 0'
>
/usr/lib/gcc/x86_64-redhat-linux/4.1.0/../../../../include/c++/4.1.0/bits/stl_list.h:112:
> note: candidates are: std::_List_iterator<boost::shared_ptr<Wad_file> >&)

I've got to be more careful about testing the build on multiple platforms before
submitting these things.

I'll add your patch and do some testing on FC-5 i386 before updating the package.

I thought it would be a good idea to start the full review while waiting for an
updated version with my patch included. This review is done with my patch
included, otherwise I wouldn't ger very far.


MUST
====
* rpmlint output clean
* Package named correctly
* GPL license OK.
* spec file legible, in Am. English
* Source matches upstream
* Successfully compiles and builds on at least one platform (FC-5 x86_64)
* no locale data, shared libraries, or static libraries
* No excessive Requires: or BR:
* Summary and description ok
* macro use consistent
* package owns the directories that it creates.
* Not relocatable
* %doc does not affect runtime

MUSTFIX
=======
* Does not compile on FC-5 without the attached patch

Patch included.  The sprite viewer doesn't seem to work on FC4-x86_64 or
FC5-i386.  Type "v" at the yadex command prompt to view sprites and you'll get a
new window that hangs.

http://www.kobold.org/~wart/fedora/yadex-1.7.0-3.src.rpm
http://www.kobold.org/~wart/fedora/yadex.spec

Hmm,

I though I could reproduce this, but after a recompile with some debugging
printf's added its gone, and now even a clean compile doesn't do it anymore,
maybe a compilerbug which got fixed in one of the last updates?

I'll investigate on my end once I've finished reconfiguring my desktop with
FC-5.  Don't expect any updates for about a week.

Any progress on this?

User error.  I was trying to use the mouse in the "view texture" mode, when it
seems that it only responds to up/down arrow.  The sprite and texture viewers
don't seem to properly refresh the display on Expose events, but it's not a
critical problem.

Right now I'm trying to figure out why it fails to build in mock but not on my
desktop.

It was a local mock configuration error.  Everything should be good now.

Ok,

I'll try to todo a full review soon then. Unfortunatly I _really_ should start
doing some stuff for my work right now. I know I just submitted a package for
review myself, thats because a Dutch language pack was recently released for an
educational game, so no my little one can play it once packaged. Thus I couldn't
help myself :)

I'll also take a look at the bsd-games setgid stuff when I find the time.

I just saw I already started a full review, so its easy to finish it here we go
again:

MUST
====
* rpmlint output OK:
E: yadex configure-without-libdir-spec
W: yadex patch-not-applied Patch1:
http://glbsp.sourceforge.net/yadex/Yadex_170_Hexen.diff
Which are both ok, for the patch warning see the comment in the spec file, the
other is no problem since there are no files installed to %{_libdir} and there
is a good reason to not use %configure (see comment in spec)
* Package named correctly
* GPL license OK.
* spec file legible, in Am. English
* Source matches upstream
* Successfully compiles and builds on at least one platform (devel x86_64 & i386)
* no locale data, shared libraries, or static libraries
* No excessive Requires: or BR:
* Summary and description ok
* macro use consistent
* package owns the directories that it creates.
* Not relocatable
* %doc does not affect runtime

Approved!

Imported and built.  Thanks!