Status
The engine now runs with occasonal minor graphical glitches. But after 5th screen cursor bug is triggered which crashes the game. The code is not endian-safe, JoostP is looking into this as time permits.
Problematic places
- map_loadDataFromAvo() in map.cpp. These are plain calls to memcpy() so all read data is not endian safe and moreover it could read wrong size if sizeof(structure) is used in length calcualtion. All structures should be unwind, i,e, read element by element and FROM_LE_XX wraps applied.
- data_readData(). Same as above.
- int32 *gob_curGobStateVarPtr and all gob_*Ptr variables. They all map directly into data, and have to be fixed.
- All structs directly mapped to memory that are not trivially replaced (such as 'scen_animations'). Currently use (or should use - this is still WIP) READ_LE_* functions to read most member variables which are known to be in LE format and only read from - NOT written to. Later, when the data is hopefully converted to native format during loading, these READ_LE_* calls should obviously go.
Showstoppers/Overall bugs
- After fifth screen as well as in demo cursor code breaks. .TOT files specify that there is no .IM1 file but still offset which is passed to inter_loadCursor() is negative. Abscence of .IM1 makes game_imFileData zero which leads to crash in attemt to get data from that. Gobreverse has this bug too
- Screen is not wiped out properly between fade in/fade outs
- Fade out leaves yellow background
- Currently only VGA floppy is supported so we should add MD5-based detection before it will go to CVS
- There is a glitch when you use 'put' action on edge of collision area. See screenshots on the right for a good example. It doesn't happen all the time, i.e. goblin should do some walking to reach that point so glitch will appear.
- Attempt to Put 3rd apple from second screen (VQVQFDE) into gap (note Put, not Use) leads to instant crash.
More on Endianness and Alignment
Assignee: Carthag. I am going to work on this, this Sunday. I will initlally confine myself to game.cpp, and then start migrating the changes to wherever the files are accessed.
It crashes now in game.cpp function game_loadExtTable() at data_readData().
This is all bad. Whole game tries to read structures directly without parsing them. Problems which it arises:
- sizeof(struct) is bigger than sum of sizes of its elements on most systems due to alignment. This is why it crashes now
- Endianness is not honoured
What we should do here is get rid of both data_openData and data_readData() and write function with this prototype (give it proper name):
MemoryReadStream data_getData(const char *filename, const char *chunkname);
It will read whole named chunk and return a stream of it. So code using it will look as follow:
chunk = new data_getData("intro.stk", "intro.tot");
foo.a = chunk.readUint32LE();
chunk.read(foo.b, 20);
delete chunk;
MemoryReadStream will track end of chunk by itself, so no additional code is needed.
All places should be revisited and rewritten as every one will cause problems.
This will let us get rid of file_open() too which is an implementation of an ugly idea.
Update
sev: As a temporary solution I fixed that with such code:
data_readData(game_extHandle, (char *)&game_extTable->items[i].offset, 4); game_extTable->items[i].offset = FROM_LE_32(game_extTable->items[i].offset); data_readData(game_extHandle, (char *)&game_extTable->items[i].size, 2); game_extTable->items[i].size = FROM_LE_16(game_extTable->items[i].size);
which is ugly, difficult to read but feasible until proper object-oriented implementation will be done
Main and long-term goals
- Make thing work
- Convert it to C++ classes. This should be done on per-file basis and keep in mind that we should make it to resemble original code al little as possible.
- Endianness fixes
File-specific tasks
gob.cpp
- Write proper game detection. (MD5-based?)
map.cpp
- See FIXME comment. All calls to map_loadDataFromAvo() should be made endianness and alignment-safe.
sound.cpp
- Has yet to be written from sound.asm
text.cpp
- This file should be removed and is not currently required for compilation but we need to inspect which variables does it set and to which values and convert them to our Config Manager. We should support all video outputs and at least PC speaker and Adlib.
util.cpp
- Get rid of rand() and use RandomSource
video.cpp
Assignee: JoostP is working on these
- Implement
pDrawSprite,pFillRect, pDrawLine,pPutPixel,pDrawletter, pDrawPackedSprite functions from the VGA driver (lvga.gdr) - Classify
- Add support for other rendering modes (EGA/CGA/Hercules) - the EGA version of Gobliiins works with gobreverse, so vid_spriteUncompressor needs to be adjusted and/or the packedSprite drawing routine from the driver need to be implemented.