Opened 16 years ago

Closed 16 years ago

Last modified 9 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 16 years ago.
Patch against a March 1 CVS snapshot

Download all attachments as: .zip

Change History (24)

Changed 16 years ago by eriktorbjorn

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

Patch against a March 1 CVS snapshot

comment:1 Changed 16 years ago by fingolfin

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

comment:2 Changed 16 years ago by fingolfin

Owner: set to SF/ender

comment:3 Changed 16 years ago by fingolfin

Owner: changed from SF/ender to aquadran

comment:4 Changed 16 years ago by fingolfin

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

comment:5 Changed 16 years ago by aquadran

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

comment:6 Changed 16 years ago by eriktorbjorn

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

comment:7 Changed 16 years ago by aquadran

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

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 Changed 16 years ago by aquadran

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

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

comment:11 Changed 16 years ago by eriktorbjorn

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

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

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

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

Bug #754421 is related

comment:16 Changed 16 years ago by fingolfin

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 Changed 16 years ago by aquadran

what exactly you need ? :)

comment:18 Changed 16 years ago by fingolfin

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

comment:19 Changed 16 years ago by fingolfin

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

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

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

comment:22 Changed 16 years ago by fingolfin

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 Changed 9 months ago by digitall

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