Opened 19 years ago

Closed 19 years ago

Last modified 19 years ago

#2064 closed defect (fixed)

SAM: Actor layering glitch

Reported by: eriktorbjorn Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
Version: Keywords:
Cc: Game: Sam and Max

Description

Sam & Max, English CD version Latest ScummVM CVS snapshot

(This probably happens in other places as well, but this is the best example I've seen so far.)

When talking to the guy at the Ball of Twine Museum, if Max walks over to the spot where Sam is standing he will sometimes be drawn in front of Sam and sometimes behind, causing an annoying "flickering" effect. It would look better is Max was always drawn in front.

In this scene, I believe Sam is actor number 2 and Max is actor number 3. They're both in the same layer and on the same Y coordinate, giving them the same value in the sort order. Unless I'm mistaken they're always added to the unsorted list in the same order, so I guess the current sorting algorithm is not a stable one, i.e. it does not preserve the order of equal keys.

It's quite easy - and very, very tempting - to make the sorting stable. On the other hand, there's that big, scary comment to contend with...

Since Max's movements are random, it's not certain that he will stay in the same spot long enough for you to observe the phenomenon. If he moves away too quickly, just reload the savegame and try again, or wait for him to walk back to that spot.

Ticket imported from: #1220168. Ticket imported from: bugs/2064.

Attachments (2)

samnmax.s20 (44.1 KB ) - added by eriktorbjorn 19 years ago.
Savegame at the Ball of Twine museum
samnmax_1220168.diff (1.7 KB ) - added by cyxx 19 years ago.

Download all attachments as: .zip

Change History (10)

by eriktorbjorn, 19 years ago

Attachment: samnmax.s20 added

Savegame at the Ball of Twine museum

comment:1 by eriktorbjorn, 19 years ago

Fingolfin, I believe you wrote the Big Scary Comment(TM). Do you have any insights on this one?

comment:2 by eriktorbjorn, 19 years ago

Owner: set to fingolfin

by cyxx, 19 years ago

Attachment: samnmax_1220168.diff added

comment:3 by cyxx, 19 years ago

Looking at the disasm of the original interpreter, the processActors() is different there. In particular, it was modified to handle "properly" equal elements ; so in samnmax, the sorting seems stable as you were thinking Erik.

I did a quick asm to C translation of this function, see the attached patch. With it, the flickering is gone and it doesn't "wake up" bug #758167...

comment:4 by eriktorbjorn, 19 years ago

This line looks unnecessarily complicated to me:

if ((j > i && _sortedActors[j] < _sortedActors[i]) || (j < i && _sortedActors[j] > _sortedActors[i])) {

Wouldn't this do the same thing?

if (_sortedActors[j]->_number < _sortedActors[i]->_number) {

Which is short enough that it could be combined with the outer if-statement without impairing readability, I think.

Out of curiosity, do you know if the stable sorting algorithm was only in Sam & Max, and reverted to the unstable one for later games?

comment:5 by cyxx, 19 years ago

As of the simplification, your suggestion, Erik, looks good to me.

About later games, I only checked FT and COMI, and the algorithm used is the same as in scummvm now. DOTT also uses it.

comment:6 by eriktorbjorn, 19 years ago

Owner: changed from fingolfin to eriktorbjorn
Resolution: fixed
Status: newclosed

comment:7 by eriktorbjorn, 19 years ago

I see no reason not use your suggested fix, then. Thanks! I've made a few more changes, but they shouldn't affect the result.

I'm a bit surprised that the actor layer isn't used at all, but Sam & Max work in mysterious ways sometimes...

comment:8 by Kirben, 19 years ago

actor layer is only used in SCUMM 7/8 games and HE90+ games.

Note: See TracTickets for help on using tickets.