Opened 18 years ago

Closed 18 years ago

Last modified 5 years ago

#8499 closed patch

GobEngine code wrapped in classes

Reported by: DrMcCoy Owned by: sev-
Priority: normal Component: Engine: Gob
Version: Keywords:
Cc: Game:

Description

I already posted that into the forums, but sev found some flaws and suggested that I submit it here... It's just the whole code for the Gobliiins engine wrapped into classes, similar to the SAGA one...

Known flaws:

- Wrong coding style, I think I now caught every wrong '{'... - sev mentioned it breaks the engine for him with a "dataFileOpen: Data file slots are full", the only thing I could think of right now was that the Global::dataFiles variables don't get initialized with 0/NULL correctly, I added that into the Global-constructor... Since that problem didn't occur on my end, I'm not sure if that was the problem, though...

Since the patch's to big according to sourceforge (max. 256KB, it's > 512KB), here's a link: http://drmccoy.xtrem3.de/zeugs/scummvm_gob_classes.diff

Ticket imported from: #1395615. Ticket imported from: patches/604.

Attachments (2)

scummvm_gob_classes.diff.bz2 (77.1 KB ) - added by fingolfin 18 years ago.
Patch v1
scummvm_gob_classes.diff.2.bz2 (78.1 KB ) - added by DrMcCoy 18 years ago.
Patch v2

Download all attachments as: .zip

Change History (11)

comment:1 by fingolfin, 18 years ago

Patch is only 80kb after compression, attaching it here :-).

by fingolfin, 18 years ago

Patch v1

comment:2 by DrMcCoy, 18 years ago

*headdesk*

Okay, I'm stupid, I completely forgot about compressing it... *g*

comment:3 by sev-, 18 years ago

just bzip the patch and attach it here.

I ran this new patch. Well, game starts, but it hangs with a solid pink screen. So apparently there is more stuff to initialize.

It hungs in

#11 0x287dad72 in Gob::PalAnim::fade (this=0x824e000, palDesc=0x824c9c4, fade=0, allColors=0) at gob/palanim.cpp:191 #12 0x287c2d7c in Gob::Draw::interPalLoad (this=0x822a800) at gob/draw.cpp:735 #13 0x287d510c in Gob::Inter::funcBlock (this=0x8241c00, retFlag=2) at gob/inter.cpp:1401 #14 0x287d561f in Gob::Inter::callSub (this=0x8241c00, retFlag=2) at gob/inter.cpp:1653 #15 0x287c7eaa in Gob::Game::playTot (this=0x8230c00, skipPlay=3072) at gob/game.cpp:1842

comment:4 by sev-, 18 years ago

Owner: set to sev-

comment:5 by sev-, 18 years ago

Maybe it would be best to use inline struct initializers (just like SAGA does, and initialize every class member, just for safety. It's really annoying feature of MSVC to zero all newly created objects automatically.

comment:6 by DrMcCoy, 18 years ago

Global::pPrimarySurfDesc wasn't initialized, that could be the problem... Just to be sure, every variable is now initialized to something deterministic... Not every struct has an initializer, though, it breaks the #pragma START_PACK_STRUCTS/END_PACK_STRUCTS :/

Another thing: The original code uses 0 instead of NULL nearly everywhere, to be consistent, I did so as well... But since the rest of scummvm doesn't, should I change it?

by DrMcCoy, 18 years ago

Patch v2

comment:7 by sev-, 18 years ago

Great stuff! I commited it now with slight modifications. We have little different modification of class declarations. I.e. members encapsulation type should be idented to 0. Hence all members were reindented.

Another thing is that according to our standards class variables should be prefixed with underscore, so I started to do that. Though not to make it too different from your patch I stopped this after couple files.

Otherwise commited as is. Please, come to IRC and we will chat what could be done next and how.

comment:8 by sev-, 18 years ago

Status: newclosed

comment:9 by digitall, 5 years ago

Component: Engine: Gob
Note: See TracTickets for help on using tickets.