[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#438857: r4140 - in packages/trunk/hex-a-hop/debian: . patches



On Mon, Sep 10, 2007 at 08:58:50AM +0200, Gerfried Fuchs wrote:
> * Jens Seidel <jensseidel@users.sf.net> [2007-09-09 21:58:59 CEST]:
> > On Sun, Sep 09, 2007 at 07:52:00PM +0200, Miriam Ruiz wrote:
> > > --- Jens Seidel <jseidel-guest@alioth.debian.org> escribió:
> > > > Log:
> > > > Game status file is now always written with little endian data.
> > > > So it's exchangeable across different systems.
>
>  Hmm, might not be considered critical, but:
> 
> > PS: I consider all the little<-->big endian changes as a big mess.
> 
>  And adding code explicitly messing around with the endianness for the
> sake of being able to copy data files around isn't adding to the mess?

Right, it isn't! First of all I'm not so familiar with the code. It is
well possible that some code fragments dealing with save games are not
used at all. (E.g. support for old saved games of version 1--3 should no
longer be used (at least in Debian), current file format version is 4).
Now I just adapted (hopefully) *all* read/save operations equally
without large analysis of the intruction flow.

It would be harder to only partly deal with endianess issues. Some code
is related to reading level data and images which are little endian
encoded by the author. But the save game status is written on each host
separately and that's why by default in the system endianess. In this
case I always have to know the endianess of the data for read operations.
 
Another issue: hex-a-hop contains also a level editor. I never started
it, it needs to be activated during compilation with a #define. I'm not
sure but assume that such level descriptions can be written to file as
well. This should happen in little endian format for compatibility.

Also: hex-a-hop uses in load/save operations file pointers fn to
fread/fwrite and shares (often) the same code.

Previously I used:

fn(&data, sizeof(data), 1, file);
if (!save) {
  SWAP32(data);
}

Now I use:

SWAP32(data);
fn(&data, sizeof(data), 1, file);
SWAP32(data);

I think this is also simpler and works bor read and write operations :-)

> I wonder about the usecase of people doing that...

It allows also easier test cases for bug reports.
I think I explained it well. Now you can sponser the package :-)

PS: My changes are minimal. A complete rewrite of load/save operations
would simplifiy a lot but even in this case it is important to
demonstrate errors in the current code as I did.

Jens



Reply to: