Opened 16 years ago

Closed 16 years ago

Last modified 2 years 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 16 years ago.
SAGA beta1 patches
SAGA.beta2.patch.bz2 (19.6 KB ) - added by SF/h00ligan 16 years ago.
SAGA.beta2.patch
SAGA.beta3.patch.bz2 (21.2 KB ) - added by SF/h00ligan 16 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 by fingolfin, 16 years ago

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

by SF/h00ligan, 16 years ago

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

SAGA beta1 patches

comment:2 by SF/h00ligan, 16 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-, 16 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-, 16 years ago

Owner: set to sev-

comment:5 by sev-, 16 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-, 16 years ago

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

comment:7 by fingolfin, 16 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, 16 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, 16 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, 16 years ago

Attachment: SAGA.beta2.patch.bz2 added

SAGA.beta2.patch

comment:10 by SF/h00ligan, 16 years ago

- some code styling

comment:11 by sev-, 16 years ago

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

by SF/h00ligan, 16 years ago

Attachment: SAGA.beta3.patch.bz2 added

comment:12 by SF/h00ligan, 16 years ago

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

comment:13 by sev-, 16 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, 16 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-, 16 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-, 16 years ago

Status: newclosed

comment:17 by digitall, 2 years ago

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