Opened 15 years ago

Closed 15 years ago

Last modified 11 months ago

#8373 closed patch

ITE: MAC demo support

Reported by: SF/h00ligan Owned by: sev-
Priority: normal Component: Engine: SAGA
Keywords: Cc:
Game: Inherit the Earth

Description

This patches include:
- BE<>LE wrapper for MemoryReadStream -
MemoryReadStreamEnd
- SAGA Engine implementation of that wrapper ( ITE
MAC Demo is trying to load)
- some code clean up in SAGA

todo:
- fully implement ITE Mac Demo support
- after all changes YS_DL_LIST must gone away

Ticket imported from: #1081904. Ticket imported from: patches/478.

Attachments (3)

SAGA.beta1.patches.tar.bz2 (9.6 KB ) - added by SF/h00ligan 15 years ago.
SAGA beta1 patches
SAGA.beta2.patch.bz2 (19.6 KB ) - added by SF/h00ligan 15 years ago.
SAGA.beta2.patch
SAGA.beta3.patch.bz2 (21.2 KB ) - added by SF/h00ligan 15 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 by fingolfin, 15 years ago

There is no file attached, maybe you forgot to activate the
"Check to Upload and Attach a File" checkbox.

by SF/h00ligan, 15 years ago

Attachment: SAGA.beta1.patches.tar.bz2 added

SAGA beta1 patches

comment:2 by SF/h00ligan, 15 years ago

This patches include:
- BE<>LE wrapper for MemoryReadStream -
MemoryReadStreamEnd
- SAGA Engine implementation of that wrapper ( ITE
MAC Demo is trying to load)
- some code clean up in SAGA

todo:
- fully implement ITE Mac Demo support
- after all changes YS_DL_LIST must gone away

comment:3 by sev-, 15 years ago

o Please, follow our code formatting conventions:
http://www.scummvm.org/documentation.php?view=conventions

o It would be more logical to rename MemoryReadStreamEnd to
MemoryReadStreamEndian

o EVENTLIST -> EventList. Capitalized file types is
reinherit leftover, in ScummVM codebase we name them
similiar to classes

o Endianness flag in game.c would be more logical to
implement as a game feature, i.e. add GF_BIG_ENDIAN_DATA and
use little endian as a default.

o With previous item implemented _vm->_endianness could go
away, and _vm->_features & GF_BIG_ENDIAN_DATA is used
everywhere.

o Speaking of different resource numbers in Mac.
* 1535 and 1536 are missing, so all higher IDs are
shifted by 2
* 1805 and 1806 are additional Mac-specific resources.
So you could just add simple convertor, which will check
resource number and decrease it by two for IDs > 1537 and
give warning if its object 1535 or 1536 on Mac data.

o As of new target in game detector, it should be extended,
as currently it uses file-based detection scheme, and both
Mac and Win demos use same files layout. But we may take a
look into it little later.

comment:4 by sev-, 15 years ago

Owner: set to sev-

comment:5 by sev-, 15 years ago

Fingolfin: could you check his changes to common? (despite
that name change, it seems ok to me)

Ajax:
o ZLIB_WINAPI sneaked into your patch (see
common/scummsys.h). This should be posted separately

o Forgot to mention. It would be more elegant (IMHO) to write:
MemoryReadStreamEnd(const byte *buf, uint32 len, bool
bigEndian = false).
This way Endianness enum will go away and you will write
_bigEndian?MemoryReadStream::readUint16BE() :
MemoryReadStream::readUint16LE();
everywhere.

comment:6 by sev-, 15 years ago

Summary: SAGA Engine: MAC demo supportITE: MAC demo support

comment:7 by fingolfin, 15 years ago

* The changes to common/list.h have nothing to do with this patch
and should be submitted as a seperate patch tracker item.

* Same for moving the writeSint16BE etc. method to the header
file, making them inline -- I'll happily review and discuss that
change on an explicit tracker item, but I don't want it to be
slipped in through the backdoor as part of this patch

* The design of the MemoryReadStreamEnd class makes no sense for
several reasons. First off, you overload inline methods, which
can easily lead to very strange and hard to debug errors. Also,
the design contract is broken since those functions change their
behaviour in a fundamental way (readUint16LE does not anymore
read memory in LE mode in all cases).
The proper solution would be to add a "readUint16()" method which
then either calls readUint16LE or readUint16BE, depending on the
_endianess flag

* MemoryReadStreamEnd is, as Eugene already said, not a good
name.

* At this point I see no reason why MemoryReadStreamEnd should be
in common, it's specific to saga only. If we ever find we need
this in other modules, we can still move it to common.

To sum this up: I see no reason at all why this patch should
touch common at all!

comment:8 by SF/h00ligan, 15 years ago

progression of patch:
- class that handles BE<>LE management renamed to
MemoryReadStreamEndian and moved to saga/stream.h
- MemoryReadStreamEndian gives interface for reading
ordinary data types without postfix ( e.g. readUint16 )
and does proper reading relying on _bigEndian flag
- YS_DL_LIST was completely substituted by Common::
List
- this patch needs common/list patch [1083548] !!!

todo:
- RLE workaround
- MAC resource id conversion

comment:9 by SF/h00ligan, 15 years ago

progression of patch:
- class that handles BE<>LE management renamed to
MemoryReadStreamEndian and moved to saga/stream.h
- MemoryReadStreamEndian gives interface for reading
ordinary data types without postfix ( e.g. readUint16 )
and does proper reading relying on _bigEndian flag
- YS_DL_LIST was completely substituted by Common::
List
- this patch needs common/list patch [1083548] !!!

todo:
- RLE workaround
- MAC resource id conversion

by SF/h00ligan, 15 years ago

Attachment: SAGA.beta2.patch.bz2 added

SAGA.beta2.patch

comment:10 by SF/h00ligan, 15 years ago

- some code styling

comment:11 by sev-, 15 years ago

Looks nice to me, but have to wait until #1083548 will be
accepted.

by SF/h00ligan, 15 years ago

Attachment: SAGA.beta3.patch.bz2 added

comment:12 by SF/h00ligan, 15 years ago

- patch is all-sufficient (contains own SortedList)
- MAC Music supported
- MAC Animation RLE decompression fixed

comment:13 by sev-, 15 years ago

- Add legal header to new files. See any saga/* file for
example.
- It does not compile under gcc:

In file included from saga/text.h:29,
from saga/saga.h:38,
from saga/actionmap.cpp:25:
saga/list.h:17: error: syntax error before `&' token
saga/list.h:21: error: syntax error before `(' token
saga/list.h:21: error: syntax error before `&' token
saga/list.h:21: error: ISO C++ forbids declaration of
`pushBack' with no type

etc

comment:14 by fingolfin, 15 years ago

To make this compilable with a proper ISO-C++ compiler, you need to
add this to SortedList:

typedef typename Common::List<T>::iterator iterator;
typedef typename Common::List<T>::const_iterator const_iterator;

comment:15 by sev-, 15 years ago

Cleaned up formatting.

Fingolfin's portability suggestion applied.

See music.cpp, we have special function for that.
Side note: music plays too fast on Mac version, bitrate
should be wrong.

Commited

Welcome to the team, Andrew.

comment:16 by sev-, 15 years ago

Status: newclosed

comment:17 by digitall, 11 months ago

Component: Engine: SAGA
Game: Inherit the Earth
Note: See TracTickets for help on using tickets.