Bug 398581

Summary:	Review Request: 8Kingdoms - 8 Kingdoms is a 3D turn-based fantasy strategic game
Product:	[Fedora] Fedora	Reporter:	Hans de Goede <hdegoede>
Component:	Package Review	Assignee:	Wart <wart>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora-package-review, notting
Target Milestone:	---	Flags:	wart: fedora-review+ kevin: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2007-12-11 13:17:23 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:

Description Hans de Goede 2007-11-25 18:39:51 UTC

Spec URL: http://people.atrpms.net/~hdegoede/8Kingdoms.spec
SRPM URL: http://people.atrpms.net/~hdegoede/8Kingdoms-1.1.0-1.fc9.src.rpm
Description:
8 Kingdoms is a 3D turn-based fantasy strategic game in which
players become kings, build their empires and conquer enemy
kingdoms.

Theme of the game 8 Kingdoms is inspirated by the world of fantasy.
Players play on a fully 3D hex map. They construct buildings,
recruit units including infantry, mounted units, mages, catapults
and finally they attack enemy or help allies. Units gain experiences
during the battle, each unit can get some abilities upgraded to be
stronger. Data are stored in XML and freely accessible - from
language versions to units' attributes, moreover map editor with
random map generator is included for comfortable map editing.

---

Note rpmlint says:
8Kingdoms.x86_64: W: file-not-utf8 /usr/share/doc/8Kingdoms-1.1.0/index.html
8Kingdoms.x86_64: W: file-not-utf8 /usr/share/doc/8Kingdoms-1.1.0/user_guide_en.
<and many more>

This is correct, all these html files are ISO-8859-2, however changing there encoding is a bad idea, as there html headers contain "charset=ISO-8859-2", so if their encoding is changed they will get displayed wrongly.

Comment 1 Wart 2007-11-25 23:33:36 UTC

A couple of issues after a first look:

'BuildRequires: expat' should instead be 'expat-devel' to build on F-8.

The application fails to start with an untranslated error message:

$ 8Kingdoms 
common/rm/rminit.cpp (170): Nelze inicializovat SDL_Mixer

Comment 2 Hans de Goede 2007-11-26 09:44:27 UTC

I guess that means cannot initialize SDL_mixer.

Is this on F-8? Are you using a default F-8 install, or have you upgraded from
F-7, if you have upgraded from F-7 have you installed and enabled pulseaudio? If
you are running F-8 have you disabled pulseaudio?

In short are you using pulseaudio? SDL_mixer on F-8 got a last minute hack for
pulseaudio which AFAIK makes it fail when not using pulseaudio.

To be precise it installs:
/etc/profile.d/SDL_pulseaudio_hack.sh

Which contains:
# Temporary hack until SDL directly supports pulseaudio
export SDL_AUDIODRIVER=esd

So doing "unset SDL_AUDIODRIVER" before starting 8Kingdoms should fix this if
you are not using pulseaudio.

Comment 3 Wart 2007-11-26 16:57:34 UTC

This was a fresh F-8 install.  I guess I'm not using pulseaudio, because the
'unset SDL_AUDIODRIVER' workaround fixes it for me.  8Kingdoms now starts,
complains about pulseaudio missing, but still plays the music:

$ 8Kingdoms 
*** PULSEAUDIO: Unable to connect: Connection refused

Thanks for the tip.

Comment 4 Wart 2007-12-02 01:19:07 UTC

GOOD
====
* rpmlint output contains UTF-8 warnings as noted above.  This is ok
  since the HTML files contain the proper encoding header.
* Source matches upstream, with the exception of the missing extgl.h
  as noted in the spec file 
* GPL+ license ok, since only a COPYING files is included and source
  files don't appear to have any copyright statements in them.
* License file included
* spec file name ok
* spec file legible and in Am. English
* No locale handling needed
* No shared libaries
* Package is not relocatable
* Owns all directories that it creates
* No duplicate %files 
* buildroot cleaned where appropriate
* file permissions ok
* No headers, static libraries, pkgconfig files, or -devel subpackage
* .desktop file installed correctly.

MUSTFIX
=======
* Does not compile on F8-x86_64.  Must replace 'BuildRequires: expat'
  with 'BuildRequires: expat-devel'

Comment 5 Hans de Goede 2007-12-02 15:31:06 UTC

(In reply to comment #4)
> MUSTFIX
> =======
> * Does not compile on F8-x86_64.  Must replace 'BuildRequires: expat'
>   with 'BuildRequires: expat-devel'
> 

Fixed:
Spec URL: http://people.atrpms.net/~hdegoede/8Kingdoms.spec
SRPM URL: http://people.atrpms.net/~hdegoede/8Kingdoms-1.1.0-2.fc9.src.rpm

Comment 6 Wart 2007-12-04 03:29:54 UTC

Update looks good.  I like the -locking patch.  It makes the game a bit more
usable than before.

All MUSTFIX items addressed.

APPROVED.

Comment 7 Hans de Goede 2007-12-04 10:08:17 UTC

Thanks for the review!

New Package CVS Request
=======================
Package Name:      8Kingdoms
Short Description: 8 Kingdoms is a 3D turn-based fantasy strategic game
Owners:            jwrdegoede
Branches:          F-7 F-8 devel
InitialCC:         <empty>
Cvsextras Commits: Yes

Comment 8 Kevin Fenzi 2007-12-04 20:02:35 UTC

cvs done.

Comment 9 Hans de Goede 2007-12-11 13:17:23 UTC

I finally found the time to import and build this, closing.