Opened 17 years ago

Closed 17 years ago

Last modified 15 months ago

#8182 closed patch (outdated)

CMI: Actor scale experiment (don't apply)

Reported by: eriktorbjorn Owned by: fingolfin
Priority: normal Component: Engine: SCUMM
Keywords: Cc:
Game: Monkey Island 3

Description

This is an attempt to document what's known about the bug that, among other things, causes Dinghy Dog to shrink after he bites you. The patch is not meant to be applied, but points at the general area where the problem may be.

Dinghy Dog spends most of his time in walkbox 0, with ignoreBoxes set to 1. Ordinarily the latter would keep him from being scaled at all.

Somewhere along the way, SO_ACTOR_DEFAULT is invoked, which causes ignoreBoxes to be set to 0.

While ignoreBoxes is 0, SO_ACTOR_COSTUME is invoked. This calls setActorCostume(), which calls showActor(), which calls adjustActorPos(), which calls setBox(), which puts him in walkbox 7 and scales him to the appropriate size for this box.

Later he's put back in box 0 again, but at this point ignoreBoxes has been set to 1 again, so he won't be scaled back.

My first idea was to remove the ignoreBoxes check from setupActorScale, but this caused Murray to be drawn unscaled in the same room. This is clearly the wrong behaviour. What the patch does is simply to keep SO_ACTOR_DEFAULT from changing ignoreBoxes, but even if this turns out to be the correct solution, it's a hacky implementation.

Ticket imported from: #696033. Ticket imported from: patches/287.

Attachments (1)

cmi-actor-scale-hack.diff (751 bytes ) - added by eriktorbjorn 17 years ago.
Patch against a March 1 CVS snapshot

Download all attachments as: .zip

Change History (24)

by eriktorbjorn, 17 years ago

Attachment: cmi-actor-scale-hack.diff added

Patch against a March 1 CVS snapshot

comment:1 by fingolfin, 17 years ago

Maybe endy can look this up in the game code...

comment:2 by fingolfin, 17 years ago

Owner: set to SF/ender

comment:3 by fingolfin, 17 years ago

Owner: changed from SF/ender to aquadran

comment:4 by fingolfin, 17 years ago

We really need somebody with access to the disassmbly to verify this... maybe aquadran, since Endy is gone currently.

comment:5 by aquadran, 17 years ago

from IDB: commands_SO_ACTOR_DEFAULT: a->initActor(0); return;

comment:6 by eriktorbjorn, 17 years ago

Does initActor(0) set ignoreBoxes to 0 in the original interpreter? (It probably does, I just thought I should ask.)

comment:7 by aquadran, 17 years ago

actors_PrepareActor(int ActorID, int arg_4) { if (ActorID < 1) || ((Max_Actors - 1) < ActorID) { error("ActorsPrepareActor actor %d out of range (min %d, max %d\n", ActorID, 1, Max_Actors - 1); }

if (arg_4 == 1) { TActor.Costume = 0; TActor.Facing = 180; TActor.NextFacing = 180; TActor.Room = 0; TActor.X = 0; TActor.Y = 0; TActor.field_144 = 0 }

if (arg_4 == 2) { TActor.Facing = 180; TActor.NextFacing = 180; }

TActor.SpecialDraw = 0; TActor.Elevation = 0; TActor.Depth = 0; TActor.Width = 24; TActor.TalkColor = 15; TActor.TextOffsX? = 0; TActor.TextOffsY? = -80; TActor.ScaleY = 255; TActor.ScaleX = 255; TActor.NextFacing = TActor.Facing; StopActorMoving(ActorID); SetActorStepDist(ActorID, 8, 2);

TActor.IgnoreTurns = 0; TActor.Box = 0; // that seems to be ignoreBoxes = 0 TActor.ZClip = -1; TActor.AnimSpeed = 0; TActor.field_188 = 0; TActor.AnimInit = 1; TActor.AnimWalk = 2; TActor.AnimStand = 3; TActor.AnimTalk1 = 4; TActor.AnimTalk2 = 5; TActor.WalkScript = 0; TActor.TalkScript = 0; TActor.Volume = 127; TActor.Frequency = 256; TActor.Pan = 64;

i = 1; do { ax = objects_PutActObjClass(ActorID, i, objects_GetActObjClass(0, i)); } while (++i < 33); }

comment:8 by eriktorbjorn, 17 years ago

Hmm... A few differences between this one and ours:

* This one always sets talkFrequency = 256. Ours only do it in the mode == 1 case.

* This one sets TActor.NextFacing = TActor.Facing. Ours sets newDirection = 0.

* There are a few variables in this version that I don't know what they are.

For a moment I was hoping that "Box" corresponded to our "walkbox". Oh well, I guess the bug lies somewhere else...

comment:9 by aquadran, 17 years ago

names mapped to scummvm:

if (arg_4 == 1) { TActor.costume = 0; TActor.facing = 180; TActor.newDirection = 180; TActor.room = 0; TActor.x = 0; TActor.y = 0; TActor.field_144 = 0 // ??? }

if (arg_4 == 2) { TActor.facing = 180; TActor.newDirection = 180; }

TActor.shadow_mode = 0; TActor.elevation = 0; TActor.Depth = 0; // ??? TActor.width = 24; TActor.talkColor = 15; TActor.talkPosX = 0; TActor.talkPosY = -80; TActor.scaley = 255; TActor.scalex = 255; TActor.newDirection = TActor.facing; StopActorMoving(ActorID); SetActorStepDist(ActorID, 8, 2);

TActor.ignoreTurns = 0; TActor.Box = 0; // that seems to be ignoreBoxes TActor.forceClip = -1; TActor.animSpeed = 0; TActor.field_188 = 0; // ??? TActor.initFrame = 1; TActor.walkFrame = 2; TActor.standFrame = 3; TActor.talkFrame1 = 4; TActor.talkFrame2 = 5; TActor.walkScript = 0; TActor.talkScript = 0; TActor.Volume = 127; // not exist in scummvm TActor.talkFrequency = 256; TActor.Pan = 64; // not exist in scummvm

comment:10 by eriktorbjorn, 17 years ago

Could "Depth" be the same as our "layer"?

comment:11 by eriktorbjorn, 17 years ago

Other things I noticed:

This version sets talkPosX = 0 and talkPosY = -80. Ours does it the other way around. Of course, that shouldn't matter here.

This version sets forceClip to -1 (or is that 255?). Ours sets it to 0.

Though I don't think these matter to this problem either.

comment:12 by fingolfin, 17 years ago

Depth = layer, yeah, i am pretty sure I recall that from previous disassmbly pieces

Regarding all the other minor changes: yeah could be that they are significant, or not, but we should not just blindly modify our own code - rather, we'd have to compare that with older Scumm engine versions. Maybe some of these are V8 specific changes. Maybe some of them have an effect on the game, maybe not...

comment:13 by fingolfin, 17 years ago

Anyway, does any of this explain the bug we see related to the scaling ? I don't think the bug will go away if we change initActor to exactly match COMI... or will it ?!? Hrm, this unsatisfying :-/

comment:14 by eriktorbjorn, 17 years ago

I only did a quick test, so I may have screwed it up, but no... I don't think any of the differences in initActor() has any effect on this bug. Maybe the problem is at the other end, i.e. SO_ACTOR_COSTUME. Or somewhere in between -- I can't remember quite what was happening there.

comment:15 by fingolfin, 17 years ago

Bug #754421 is related

comment:16 by fingolfin, 17 years ago

So, we know that reseting ignoreBoxes as a consequence of SO_ACTOR_DEFAULT is correct. The real problem is that the actor gets repositioned. Since that happens in the course of SO_ACTOR_COSTUME, the logical next step is to compare SO_ACTOR_COSTUME with what we have...

aquadran, I don't suppose you could once again take a look at your IDA DB? :-)

comment:17 by aquadran, 17 years ago

what exactly you need ? :)

comment:18 by fingolfin, 17 years ago

The code for the SO_ACTOR_COSTUME subop of actorSet. And then probably the code of setActorCostume

comment:19 by fingolfin, 17 years ago

Based on the disassembly aquadran sent me of setActorCustome and the functions it calls, I rewrote our setActorCostume (for V8), which fixes bug #754421, and I hope also the issue which motivated this bug.

Erik, could you verify that?

comment:20 by eriktorbjorn, 17 years ago

The change definitely fixes the Dinghy Dog scaling problem.

Unless I see it happening again, I'm going to assume it also fixes the other cases where actors would shrink. I don't remember how to reproduce them anyway, and it seems likely that they were caused by the same bug.

comment:21 by fingolfin, 17 years ago

Owner: changed from aquadran to fingolfin
Resolution: outdated
Status: newclosed

comment:22 by fingolfin, 17 years ago

Great. Now the only question is, will this cause regressions... I hope not... it would be nice to have this fix in 0.5.0, but w/o any testing, i think that's not possible.. hm

comment:23 by digitall, 15 months ago

Component: Engine: SCUMM
Game: Monkey Island 3
Note: See TracTickets for help on using tickets.