#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)
Change History (10)
by , 19 years ago
Attachment: | samnmax.s20 added |
---|
comment:1 by , 19 years ago
Fingolfin, I believe you wrote the Big Scary Comment(TM). Do you have any insights on this one?
comment:2 by , 19 years ago
Owner: | set to |
---|
by , 19 years ago
Attachment: | samnmax_1220168.diff added |
---|
comment:3 by , 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 , 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 , 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 , 19 years ago
Owner: | changed from | to
---|---|
Resolution: | → fixed |
Status: | new → closed |
comment:7 by , 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...
Savegame at the Ball of Twine museum