#8373 closed patch
ITE: MAC demo support
| Reported by: | SF/h00ligan | Owned by: | sev- |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SAGA |
| Version: | 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)
Change History (20)
comment:1 by , 20 years ago
comment:2 by , 20 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 , 20 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 , 20 years ago
| Owner: | set to |
|---|
comment:5 by , 20 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 , 20 years ago
| Summary: | SAGA Engine: MAC demo support → ITE: MAC demo support |
|---|
comment:7 by , 20 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 , 20 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 , 20 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 , 20 years ago
| Attachment: | SAGA.beta3.patch.bz2 added |
|---|
comment:12 by , 20 years ago
- patch is all-sufficient (contains own SortedList) - MAC Music supported - MAC Animation RLE decompression fixed
comment:13 by , 20 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 , 20 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 , 20 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 , 20 years ago
| Status: | new → closed |
|---|
comment:17 by , 6 years ago
| Component: | → Engine: SAGA |
|---|---|
| Game: | → Inherit the Earth |
There is no file attached, maybe you forgot to activate the "Check to Upload and Attach a File" checkbox.