#1408 closed defect (fixed)
COMI: Guybrush appears above tomb
| Reported by: | SF/lechimp | Owned by: | cyxx |
|---|---|---|---|
| Priority: | high | Component: | Engine: SCUMM |
| Version: | 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)
Change History (23)
by , 21 years ago
comment:1 by , 21 years ago
Hmmm.... this also happens after picking up the engagement band and leaving the crypt....
comment:2 by , 21 years ago
| Priority: | normal → low |
|---|
comment:3 by , 21 years ago
Hmmm... this also happens after picking up the engagement band and leaving the crypt...
comment:4 by , 21 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 , 21 years ago
| Priority: | low → blocker |
|---|---|
| Summary: | COMI : Guybrush appears above tomb → RC COMI: Guybrush appears above tomb |
comment:6 by , 21 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 , 21 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 , 21 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 , 21 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 , 21 years ago
| Summary: | RC COMI: Guybrush appears above tomb → COMI: Guybrush appears above tomb |
|---|
comment:11 by , 20 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 , 20 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 , 20 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 , 20 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 , 20 years ago
| Priority: | blocker → high |
|---|
comment:16 by , 20 years ago
I have check COMI disasm, our o6_putActorAtObject() is correct and whereIsObject() seems fine too.
comment:18 by , 20 years ago
yeah and here it happend once after leaving the familiy tomb after getting the part of the ring.
comment:19 by , 20 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 , 20 years ago
| Status: | new → closed |
|---|
comment:21 by , 20 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
comment:22 by , 20 years ago
SCUMM 7 and 8 used a bestDist value of 0x7FFFFFFF, added change to ScummVM CVS.
Beforing entering Mutual of Stan