Opened 17 years ago

Closed 16 years ago

Last modified 12 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, 16 years ago

Bug #754421 is related

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

what exactly you need ? :)

comment:18 by fingolfin, 16 years ago

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

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

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

comment:22 by fingolfin, 16 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, 12 months ago

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