Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#1408 closed defect (fixed)

COMI: Guybrush appears above tomb

Reported by: SF/lechimp Owned by: cyxx
Priority: high Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 3

Description

ScummVM 0.5.7 cvs tarball 23/1/2004, using MacOS X,
gcc 3.3. English version.

After triggering the 'enter Stan's crypt a lot of times' easter
egg, you see the 'The Dig' scene, but afterwards Guybrush
appears in the cemetery on top of the tomb. Walking
makes him disappear.

A savegame is attached - just walk into the crypt.

Ticket imported from: #883415. Ticket imported from: bugs/1408.

Attachments (1)

comi.s09 (61.7 KB) - added by SF/lechimp 15 years ago.
Beforing entering Mutual of Stan

Download all attachments as: .zip

Change History (23)

Changed 15 years ago by SF/lechimp

Attachment: comi.s09 added

Beforing entering Mutual of Stan

comment:1 Changed 15 years ago by SF/lechimp

Hmmm.... this also happens after picking up the engagement
band and leaving the crypt....

comment:2 Changed 15 years ago by SF/lechimp

Priority: normallow

comment:3 Changed 15 years ago by SF/lechimp

Hmmm... this also happens after picking up the engagement
band and leaving the crypt...

comment:4 Changed 15 years ago by SF/ender

Raising priority and setting release critical. This bug makes it
quite difficult to move once it occurs (in the ring situation at
least, the easter egg isn't so important). This makes it very
annoying :)

comment:5 Changed 15 years ago by SF/ender

Priority: lowblocker
Summary: COMI : Guybrush appears above tombRC COMI: Guybrush appears above tomb

comment:6 Changed 15 years ago by eriktorbjorn

I don't know about the ring, but I've taken a closer look at
the easter egg.

From what I can see, it's script-383 that displays the
cutscene. Here's how it ends:

[0233] (85) endOverride()
[0234] (82) endCutscene()
[0235] (A1) putActorAtXY(11,0,0,0)
[024A] (A1) putActorAtXY(15,0,0,0)
[025F] (A1) putActorAtXY(10,0,0,0)
[0274] (A2) putActorAtObject(1,1066)
[027F] (8D) actorFollowCamera(1)
[0285] (AC) actorOps.setCurActor(1)
[028C] (AC) actorOps.setActorDirection(180)
[0293] (7B) stopObjectCode()
END

The culprit here appears to be putActorAtObject(1,1066).
Since the whereIsObject() fails, it places Guybrush on the
default coordinates, x = 160 and y = 120.

Now, before anyone says "that has to be wrong for v8 games",
it should be pointed out that these coordinates appeared in
revision 1.89 of script_v6.cpp with the comment "correct to
match dissassembly". Before that, x was set to _realWidth / 2.

Unless I completely misremember, the change was made to fix
the bug that was described on our CMI issues page as
"Guybrush seems to walk down from the sky when entering the
Canibal Village for the first time. This is because
roomobj-53-873 calls a different script on the first entry,
which runs a putActorAtObject opcode before loading the room".

Of course, that doesn't necessarily mean that the current
code is correct - obviously there's at least one bug - but
if we change it we should at least be aware of the possible
consequences.

comment:7 Changed 15 years ago by fingolfin

Indeed. COMI does hard code 160/120 there.

Maybe it's a bug that it can't find object 1066 ? Maybe a walkbox issue?
Questions over questions =)

comment:8 Changed 15 years ago by eriktorbjorn

It uses putActorAtObject() to move the actor to a new room,
so either it expects to be able to find the object even
though it isn't in the current room (as far as I can tell
that's what fails right now, but there's a lot I don't
understand about objects in the SCUMM engine), or it expects
the default position to be useful in every case.

As far as I remember, the idea of ensuring that the actor
moved to the new room before looking for the object was
nixed the last time around.

Maybe it's supposed to call adjustActorPos() somewhere along
the line? That will at least put Guybrush at a sensible
spot. In this particular case.

comment:9 Changed 15 years ago by fingolfin

That all reminds me a little bit about that bug in the Dig which we work
around in scumm/scumm.cpp (around line 2100). Of course the situation
is different there, but in both cases, we have an issue in a V7+ game
when entering a new room and placing the actor at an object there.

Of course, they my not have anything to do with each other, it's just an
association that popped to my mind :-)

comment:10 Changed 15 years ago by fingolfin

Summary: RC COMI: Guybrush appears above tombCOMI: Guybrush appears above tomb

comment:11 Changed 15 years ago by Kirben

I checked COMI disasm. and it doesn't use adjustActorPos()
in any additional places. I checked the functions that call
adjustActorPos() too and they aren't used in any additional
places either.

comment:12 Changed 15 years ago by fingolfin

Actually, did you check whether the bug still occurs with latest CVS? You
recently made a change to the room loading code which allowed me to
remove that The Dig specific hack I mentioned...

comment:13 Changed 15 years ago by Kirben

Unfortunately the bug still occurs when I check with the
saved game. I was hoping that change might have helped too.

comment:14 Changed 15 years ago by fingolfin

For your information:
Obj 1065: 'cemetery-dog-house-door'
Obj 1066: 'cemetery-stans-crypt-door'
Obj 1067: 'cemetery-family-crypt-door'

So 1066 would indeed only make sense after the room gets changed...

I think we already did so, but still: it wouldn't hurt if somebody double
checked the disasm of putActorAtObject

BTW, this bug isn't that terrible, since you can always double click on an
exit to another room - that still works, and will reset your size etc.

comment:15 Changed 15 years ago by fingolfin

Priority: blockerhigh

comment:16 Changed 15 years ago by Kirben

I have check COMI disasm, our o6_putActorAtObject() is
correct and whereIsObject() seems fine too.

comment:17 Changed 14 years ago by SF/mherberg

This also occurs with the 0.7.0 release.

comment:18 Changed 14 years ago by lordhoto

yeah and here it happend once after leaving the familiy tomb
after getting the part of the ring.

comment:19 Changed 14 years ago by cyxx

Ok, I think I found what was wrong.

First, I ran the original interpreter though a debugger to
check that the whereIsObject call in o_putActorAtObject was
returning WIO_NOT_FOUND like in our implementation ; this
*is* the case.

So, to 'fix' Guybrush position, the original interpreter
calls adjustActorPos which calls adjustXYToBeInBox. The
problem is in adjustXYToBeInBox where the initial value of
bestDist is too low for computing correctly the distance to
the room boxes (calls to getClosestPtOnBox are returning
values > 0xFFFF, hence the problem). Increasing the bestDist
initial value (I tested with 0xFFFFFF) seems to fix that bug...

comment:20 Changed 14 years ago by Kirben

Status: newclosed

comment:21 Changed 14 years ago by Kirben

Owner: set to cyxx
Resolution: fixed

comment:22 Changed 14 years ago by Kirben

SCUMM 7 and 8 used a bestDist value of 0x7FFFFFFF,
added change to ScummVM CVS.

Note: See TracTickets for help on using tickets.