Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#2064 closed defect (fixed)

SAM: Actor layering glitch

Reported by: eriktorbjorn Owned by: eriktorbjorn
Priority: normal Component: Engine: SCUMM
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 14 years ago.
Savegame at the Ball of Twine museum
samnmax_1220168.diff (1.7 KB ) - added by cyxx 14 years ago.

Download all attachments as: .zip

Change History (10)

by eriktorbjorn, 14 years ago

Attachment: samnmax.s20 added

Savegame at the Ball of Twine museum

comment:1 by eriktorbjorn, 14 years ago

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

comment:2 by eriktorbjorn, 14 years ago

Owner: set to fingolfin

by cyxx, 14 years ago

Attachment: samnmax_1220168.diff added

comment:3 by cyxx, 14 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, 14 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, 14 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, 14 years ago

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

comment:7 by eriktorbjorn, 14 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, 14 years ago

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

Note: See TracTickets for help on using tickets.