Opened 16 years ago

Closed 15 years ago

Last modified 15 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 16 years ago.
Beforing entering Mutual of Stan

Download all attachments as: .zip

Change History (23)

by SF/lechimp, 16 years ago

Attachment: comi.s09 added

Beforing entering Mutual of Stan

comment:1 by SF/lechimp, 16 years ago

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

comment:2 by SF/lechimp, 16 years ago

Priority: normallow

comment:3 by SF/lechimp, 16 years ago

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

comment:4 by SF/ender, 16 years ago

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 by SF/ender, 16 years ago

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

comment:6 by eriktorbjorn, 16 years ago

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 by fingolfin, 16 years ago

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 by eriktorbjorn, 16 years ago

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 by fingolfin, 16 years ago

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 by fingolfin, 16 years ago

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

comment:11 by Kirben, 15 years ago

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 by fingolfin, 15 years ago

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 by Kirben, 15 years ago

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

comment:14 by fingolfin, 15 years ago

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 by fingolfin, 15 years ago

Priority: blockerhigh

comment:16 by Kirben, 15 years ago

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

comment:17 by SF/mherberg, 15 years ago

This also occurs with the 0.7.0 release.

comment:18 by lordhoto, 15 years ago

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

comment:19 by cyxx, 15 years ago

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 by Kirben, 15 years ago

Status: newclosed

comment:21 by Kirben, 15 years ago

Owner: set to cyxx
Resolution: fixed

comment:22 by Kirben, 15 years ago

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

Note: See TracTickets for help on using tickets.