[Scummvm-cvs-logs] CVS: scummvm/scumm actor.cpp,1.94,1.95 actor.h,1.20,1.21 boxes.cpp,1.26,1.27 scummvm.cpp,2.163,2.164
Max Horn
fingolfin at users.sourceforge.net
Sun May 18 12:45:05 CEST 2003
Update of /cvsroot/scummvm/scummvm/scumm
In directory sc8-pr-cvs1:/tmp/cvs-serv32435
Modified Files:
actor.cpp actor.h boxes.cpp scummvm.cpp
Log Message:
implemented proper 'invalid walkbox' handling in older games (in newer games, box 0 is used as invalid box, while in older games this is a legal box and box 255 is the 'invalid' box); removed lots of FIXME's which were needed to cope with ScummVM not implementing the invalid walkbox stuff properly in the past; removed other actor FIXMEs.
Index: actor.cpp
===================================================================
RCS file: /cvsroot/scummvm/scummvm/scumm/actor.cpp,v
retrieving revision 1.94
retrieving revision 1.95
diff -u -d -r1.94 -r1.95
--- actor.cpp 17 May 2003 18:07:07 -0000 1.94
+++ actor.cpp 18 May 2003 19:44:22 -0000 1.95
@@ -32,6 +32,9 @@
#include <math.h>
+byte Actor::INVALID_BOX = 0;
+
+
void Actor::initActor(int mode) {
if (mode == 1) {
costume = 0;
@@ -64,7 +67,7 @@
setActorWalkSpeed(8, 2);
animSpeed = 0;
- ignoreBoxes = 0;
+ ignoreBoxes = false;
forceClip = 0;
ignoreTurns = false;
@@ -199,9 +202,6 @@
if (specdir & 0x8000) {
dir = specdir & 0x3FFF;
} else {
- // FIXME - I am not 100% if this code is right (Fingolfin)
- warning("remapDirection: special dir");
-
specdir = specdir & 0x3FFF;
if (specdir - 90 < dir && dir < specdir + 90)
dir = specdir;
@@ -345,11 +345,9 @@
return 0;
}
- // FIXME: Fingolfin asks: what do these 8000 here ?!? That looks wrong... maybe
- // a 0x8000 was meant? But even that makes not much sense to me
- tmpX = ((actorX + 8000) << 16) + walkdata.xfrac + (walkdata.deltaXFactor >> 8) * scalex;
+ tmpX = (actorX << 16) + walkdata.xfrac + (walkdata.deltaXFactor >> 8) * scalex;
walkdata.xfrac = (uint16)tmpX;
- actorX = (tmpX >> 16) - 8000;
+ actorX = (tmpX >> 16);
tmpY = (actorY << 16) + walkdata.yfrac + (walkdata.deltaYFactor >> 8) * scaley;
walkdata.yfrac = (uint16)tmpY;
@@ -378,7 +376,7 @@
return;
}
- if (ignoreBoxes != 0)
+ if (ignoreBoxes)
return;
if (_vm->getBoxFlags(walkbox) & kBoxPlayerOnly)
@@ -596,97 +594,103 @@
}
AdjustBoxResult Actor::adjustXYToBeInBox(int dstX, int dstY, int pathfrom) {
+ const uint thresholdTable[] = { 30, 80, 0 };
AdjustBoxResult abr, tmp;
uint threshold;
- uint best;
- int box, iterations = 0; /* Use iterations for those odd times we get stuck in the loop */
- int firstValidBox, i, j;
- byte flags, b;
-
- if (_vm->_features & GF_SMALL_HEADER)
- firstValidBox = 0;
- else
- firstValidBox = 1;
+ uint bestDist;
+ int numBoxes;
+ int box;
+ byte flags, bestBox;
+ const int firstValidBox = (_vm->_features & GF_SMALL_HEADER) ? 0 : 1;
abr.x = dstX;
abr.y = dstY;
- abr.dist = 0;
+ abr.dist = INVALID_BOX;
- if (ignoreBoxes == 0) {
- threshold = 30;
+ if (ignoreBoxes)
+ return abr;
- while (1) {
- iterations++;
- if (iterations > 1000)
- return abr; /* Safety net */
- box = _vm->getNumBoxes() - 1;
- if (box < firstValidBox)
- return abr;
+ for (int tIdx = 0; tIdx < ARRAYSIZE(thresholdTable); tIdx++) {
+ threshold = thresholdTable[tIdx];
- best = (uint) 0xFFFF;
- b = 0;
+ numBoxes = _vm->getNumBoxes() - 1;
+ if (numBoxes < firstValidBox)
+ return abr;
-// FIXME - why was that check here? It apparently causes bug #643001
-// if (!(_vm->_features & GF_OLD256) || box)
- for (j = box; j >= firstValidBox; j--) {
- flags = _vm->getBoxFlags(j);
+ bestDist = (uint) 0xFFFF;
+ bestBox = INVALID_BOX;
- if (flags & kBoxInvisible && (!(flags & kBoxPlayerOnly) || isInClass(31)))
+ // We iterate (backwards) over all boxes, searching the one closes
+ // to the desired coordinates.
+ for (box = numBoxes; box >= firstValidBox; box--) {
+ flags = _vm->getBoxFlags(box);
+
+ if (flags & kBoxInvisible && (!(flags & kBoxPlayerOnly) || isInClass(31)))
+ continue;
+
+ // FIXME: the following is essentially a hack to fix issues in Zak256
+ // and possibly elsewhere; but it seems the original did nothing like this
+ // so hopefully one day we'll find a 'proper' solution and can remove
+ // this hack.
+ if (pathfrom >= firstValidBox) {
+
+ if (flags & kBoxLocked && (!(flags & kBoxPlayerOnly)))
continue;
-
- if (pathfrom >= firstValidBox) {
+ int i = _vm->getPathToDestBox(pathfrom, box);
+ if (i == -1)
+ continue;
+
+ if (_vm->_features & GF_OLD256) {
+ // FIXME - we check here if the box suggested by getPathToDestBox
+ // is locked or not. This prevents us from walking thru
+ // closed doors in some cases in Zak256. However a better fix
+ // would be to recompute the box matrix whenever flags change.
+ flags = _vm->getBoxFlags(i);
if (flags & kBoxLocked && (!(flags & kBoxPlayerOnly)))
continue;
-
- i = _vm->getPathToDestBox(pathfrom, j);
- if (i == -1)
+ if (flags & kBoxInvisible && (!(flags & kBoxPlayerOnly) || isInClass(31)))
continue;
-
- if (_vm->_features & GF_OLD256) {
- // FIXME - we check here if the box suggested by getPathToDestBox
- // is locked or not. This prevents us from walking thru
- // closed doors in some cases in Zak256. However a better fix
- // would be to recompute the box matrix whenever flags change.
- flags = _vm->getBoxFlags(i);
- if (flags & kBoxLocked && (!(flags & kBoxPlayerOnly)))
- continue;
- if (flags & kBoxInvisible && (!(flags & kBoxPlayerOnly) || isInClass(31)))
- continue;
- }
}
+ }
- if (!_vm->inBoxQuickReject(j, dstX, dstY, threshold))
- continue;
-
- if (_vm->checkXYInBoxBounds(j, dstX, dstY)) {
- abr.x = dstX;
- abr.y = dstY;
- abr.dist = j;
- return abr;
- }
+ // For increased performance, we perform a quick test if
+ // the coordinates can even be within a distance of 'threshold'
+ // pixels of the box.
+ if (!_vm->inBoxQuickReject(box, dstX, dstY, threshold))
+ continue;
- tmp = _vm->getClosestPtOnBox(j, dstX, dstY);
+ // Check if the point is contained in the box. If it is,
+ // we don't have to search anymore.
+ if (_vm->checkXYInBoxBounds(box, dstX, dstY)) {
+ abr.x = dstX;
+ abr.y = dstY;
+ abr.dist = box;
+ return abr;
+ }
- if (tmp.dist >= best)
- continue;
+ // Find the point in the box which is closest to our point.
+ tmp = _vm->getClosestPtOnBox(box, dstX, dstY);
+ // Check if the box is closer than the previous boxes.
+ if (tmp.dist < bestDist) {
abr.x = tmp.x;
abr.y = tmp.y;
-
+
if (tmp.dist == 0) {
- abr.dist = j;
+ abr.dist = box;
return abr;
}
- best = tmp.dist;
- b = j;
+ bestDist = tmp.dist;
+ bestBox = box;
}
+ }
- if (threshold == 0 || threshold * threshold >= best) {
- abr.dist = b;
- return abr;
- }
- threshold = (threshold == 30) ? 80 : 0;
+ // If the closest ('best') box we found is within the threshold, or if
+ // we are on the last run (i.e. threshold == 0), return that box.
+ if (threshold == 0 || threshold * threshold >= bestDist) {
+ abr.dist = bestBox;
+ return abr;
}
}
@@ -695,7 +699,6 @@
void Actor::adjustActorPos() {
AdjustBoxResult abr;
- byte flags;
abr = adjustXYToBeInBox(x, y, -1);
@@ -714,9 +717,11 @@
stopActorMoving();
}
- flags = _vm->getBoxFlags(walkbox);
- if (flags & 7) {
- turnToDirection(facing);
+ if (walkbox != INVALID_BOX) {
+ byte flags = _vm->getBoxFlags(walkbox);
+ if (flags & 7) {
+ turnToDirection(facing);
+ }
}
}
@@ -863,9 +868,10 @@
for (ac = actors; ac != end; ++ac) {
a = *ac;
if (a->costume) {
- CHECK_HEAP getMaskFromBox(a->walkbox);
+ CHECK_HEAP
a->drawActorCostume();
- CHECK_HEAP a->animateCostume();
+ CHECK_HEAP
+ a->animateCostume();
}
}
@@ -881,9 +887,10 @@
for (i = 1; i < _numActors; i++) {
a = derefActor(i);
if (a->isInCurrentRoom() && a->costume && a->layer < 0) {
- CHECK_HEAP getMaskFromBox(a->walkbox);
+ CHECK_HEAP
a->drawActorCostume();
- CHECK_HEAP a->animateCostume();
+ CHECK_HEAP
+ a->animateCostume();
}
}
}
@@ -906,58 +913,15 @@
cr._outheight = _vm->virtscr[0].height;
- // FIXME - Hack to fix two glitches in the scene where Bobbin
- // heals Rusty: Bobbin's feet get masked when Rusty shows him
- // what happens to The Forge, and Rusty gets masked after
- // Bobbin heals him. (Room 34)
- //
- // It also fixes a much less noticable glitch when Bobbin
- // jumps out of Mandible's cage. (Room 43)
- //
- // When an actor is moved around without regards to walkboxes,
- // its walkbox is set to 0. Unfortunately that's a valid
- // walkbox in older games, and its mask may be completely
- // wrong for this purpose.
- //
- // So instead use the mask of the box where the actor happens
- // to be at the moment or, if it's not in any box, don't mask
- // at all.
- //
- // This is similar to the _zbuf == 100 check used for AKOS
- // costumes, except I haven't been able to figure out the
- // proper check here. It's not quite enough to check if
- // ignoreBoxes != 0 and checking if walkbox == 0 yields too
- // many false positives, e.g. Bobbin leaving the sandy beach
- // towards the forest, or Stoke leaving the room where he
- // locks up "Rusty".
- //
- // Until someone can find the proper fix, only apply it to the
- // rooms where it's actually known to be needed.
-
- if (_vm->_gameId == GID_LOOM256 && (_vm->_currentRoom == 34 || _vm->_currentRoom == 43) && walkbox == 0) {
- int num_boxes, i;
-
- cr._zbuf = 0;
- num_boxes = _vm->getNumBoxes();
-
- // Sometimes boxes overlap, so the direction of this
- // loop matters in some rooms.
-
- for (i = 0; i < num_boxes; i++) {
- if (_vm->checkXYInBoxBounds(i, x, y)) {
- cr._zbuf = _vm->getMaskFromBox(i);
- break;
- }
- }
- } else
- cr._zbuf = _vm->getMaskFromBox(walkbox);
-
if (forceClip)
cr._zbuf = forceClip;
else if (isInClass(20))
cr._zbuf = 0;
- else if (cr._zbuf > _vm->gdi._numZBuffer)
- cr._zbuf = (byte)_vm->gdi._numZBuffer;
+ else {
+ cr._zbuf = _vm->getMaskFromBox(walkbox);
+ if (cr._zbuf > _vm->gdi._numZBuffer)
+ cr._zbuf = _vm->gdi._numZBuffer;
+ }
cr._shadow_mode = shadow_mode;
if (_vm->_features & GF_SMALL_HEADER)
@@ -1232,16 +1196,10 @@
void Actor::startWalkActor(int destX, int destY, int dir) {
AdjustBoxResult abr;
- // FIXME: Fingolfin isn't convinced calling adjustXYToBeInBox here is the
- // right thing. In fact I think it might completly wrong...
- abr = adjustXYToBeInBox(destX, destY, walkbox);
+ abr.x = destX;
+ abr.y = destY;
if (!isInCurrentRoom()) {
- // FIXME: Should we really use abr.x / abr.y here? Shouldn't it be destX / destY?
- // Considering that abr was obtained by adjustXYToBeInBox which works on
- // the boxes in the *current* room no in the room the actor actually is in.
- // Occurs in Monkey Island 1 demo, after title name.
- warning("When is this ever triggered anyway? (%d,%d) -> (%d,%d)", x, y, abr.x, abr.y);
x = abr.x;
y = abr.y;
if (dir != -1)
@@ -1249,27 +1207,10 @@
return;
}
- if (ignoreBoxes != 0) {
- // FIXME: this seems wrong for GF_SMALL_HEADER games where walkbox
- // is a valid box. Rather, I'd think that for these games, we should
- // set walkbox to -1 or some other "illegal" value. The same for
- // abr.dist (which is used to set walkdata.destbox after all).
- // Also together with my above FIXME comment that would mean that up to
- // here there is no reason to even call adjustXYToBeInBox....
- abr.dist = 0;
- walkbox = 0;
+ if (ignoreBoxes) {
+ abr.dist = INVALID_BOX;
+ walkbox = INVALID_BOX;
} else {
- // FIXME: this prevents part of bug #605970 (Loom) from
- // occuring, and also fixes a walk bug with Rusty's ghost.
- // Not sure if there is a better way to achieve this.
- if (walkbox == 0)
- adjustActorPos();
-
- // Fingolfin remarks on the above FIXME: this is yet another case of the
- // walbox 0 problem... In many parts of the code we use "0" to denote "illegal walkbox",
- // which is correct for newer games but wrong for GF_SMALL_HEADER games
- // which use -1 / 0xFF for the same purpose.
-
if (_vm->checkXYInBoxBounds(walkdata.destbox, abr.x, abr.y)) {
abr.dist = walkdata.destbox;
} else {
@@ -1378,7 +1319,7 @@
do {
moving &= ~MF_NEW_LEG;
- if ((!walkbox && (!(_vm->_features & GF_SMALL_HEADER)))) {
+ if (walkbox == INVALID_BOX) {
setBox(walkdata.destbox);
walkdata.curbox = walkdata.destbox;
break;
@@ -1388,7 +1329,7 @@
break;
box = _vm->getPathToDestBox(walkbox, walkdata.destbox);
- if (box == -1 || box > 0xF0) {
+ if (box < 0 || box > 0xF0) {
walkdata.destbox = walkbox;
moving |= MF_LAST_LEG;
return;
@@ -1419,7 +1360,7 @@
restart:
moving &= ~MF_NEW_LEG;
- if (walkbox == 0xFF) {
+ if (walkbox == INVALID_BOX) {
walkbox = walkdata.destbox;
walkdata.curbox = walkdata.destbox;
moving |= MF_LAST_LEG;
@@ -1435,7 +1376,7 @@
next_box = _vm->getPathToDestBox(walkbox, walkdata.destbox);
- if (next_box == -1) {
+ if (next_box < 0) {
moving |= MF_LAST_LEG;
return;
}
Index: actor.h
===================================================================
RCS file: /cvsroot/scummvm/scummvm/scumm/actor.h,v
retrieving revision 1.20
retrieving revision 1.21
diff -u -d -r1.20 -r1.21
--- actor.h 5 May 2003 12:09:22 -0000 1.20
+++ actor.h 18 May 2003 19:44:22 -0000 1.21
@@ -83,7 +83,7 @@
byte charset;
int16 newDirection;
byte moving;
- byte ignoreBoxes;
+ bool ignoreBoxes;
byte forceClip;
byte initFrame, walkFrame, standFrame, talkFrame1, talkFrame2;
bool needRedraw, needBgReset, costumeNeedsInit, visible;
@@ -102,6 +102,8 @@
uint16 sound[8];
CostumeData cost;
byte palette[256];
+
+ static byte INVALID_BOX;
protected:
Scumm *_vm;
Index: boxes.cpp
===================================================================
RCS file: /cvsroot/scummvm/scummvm/scumm/boxes.cpp,v
retrieving revision 1.26
retrieving revision 1.27
diff -u -d -r1.26 -r1.27
--- boxes.cpp 17 May 2003 19:11:59 -0000 1.26
+++ boxes.cpp 18 May 2003 19:44:22 -0000 1.27
@@ -135,6 +135,7 @@
void Scumm::setBoxScale(int box, int scale) {
Box *ptr = getBoxBaseAddr(box);
+ assert(ptr);
if (_features & GF_AFTER_V8)
ptr->v8.scale = TO_LE_32(scale);
else if (_features & GF_AFTER_V2)
@@ -144,13 +145,18 @@
}
void Scumm::setBoxScaleSlot(int box, int slot) {
- Box *b = getBoxBaseAddr(box);
- b->v8.scaleSlot = TO_LE_32(slot);
+ Box *ptr = getBoxBaseAddr(box);
+ assert(ptr);
+ ptr->v8.scaleSlot = TO_LE_32(slot);
}
int Scumm::getScale(int box, int x, int y) {
+ if (_features & GF_NO_SCALLING)
+ return 255;
+
Box *ptr = getBoxBaseAddr(box);
- assert(ptr);
+ if (!ptr)
+ return 255;
if (_features & GF_AFTER_V8) {
int slot = FROM_LE_32(ptr->v8.scaleSlot);
@@ -181,9 +187,6 @@
}
} else
return FROM_LE_32(ptr->v8.scale);
- } else if (_features & GF_AFTER_V2) {
- // FIXME - nothing ?!?
- return 255;
} else {
uint16 scale = READ_LE_UINT16(&ptr->old.scale);
@@ -227,15 +230,19 @@
Box *Scumm::getBoxBaseAddr(int box) {
byte *ptr = getResourceAddress(rtMatrix, 2);
- if (!ptr)
+ if (!ptr || box == 255)
return NULL;
+
// FIXME: In "pass to adventure", the loom demo, when bobbin enters
// the tent to the elders, box = 2, but ptr[0] = 2 -> errors out.
// Hence we disable the check for now. Maybe in PASS (and other old games)
// we shouldn't subtract 1 from ptr[0] when performing the check?
// this also seems to be incorrect for atari st demo of zak
// and assumingly other v2 games
- if ((_gameId != GID_MONKEY_EGA) && (_gameId != GID_ZAK))
+ if (_gameId == GID_MONKEY_EGA) {
+ if (box < 0 || box > ptr[0] - 1)
+ warning("Illegal box %d", box);
+ } else
checkRange(ptr[0] - 1, 0, box, "Illegal box %d");
if (_features & GF_AFTER_V2)
@@ -273,7 +280,7 @@
bool Scumm::checkXYInBoxBounds(int b, int x, int y) {
BoxCoords box;
- if (b == 0 && (!(_features & GF_SMALL_HEADER)))
+ if (b < 0 || b == Actor::INVALID_BOX)
return false;
getBoxCoordinates(b, &box);
@@ -316,6 +323,7 @@
void Scumm::getBoxCoordinates(int boxnum, BoxCoords *box) {
Box *bp = getBoxBaseAddr(boxnum);
+ assert(bp);
if (_features & GF_AFTER_V8) {
box->ul.x = (short)FROM_LE_32(bp->v8.ulx);
Index: scummvm.cpp
===================================================================
RCS file: /cvsroot/scummvm/scummvm/scumm/scummvm.cpp,v
retrieving revision 2.163
retrieving revision 2.164
diff -u -d -r2.163 -r2.164
--- scummvm.cpp 18 May 2003 11:21:31 -0000 2.163
+++ scummvm.cpp 18 May 2003 19:44:22 -0000 2.164
@@ -542,6 +542,9 @@
_hexdumpScripts = false;
_showStack = false;
+ if (_features & GF_SMALL_HEADER)
+ Actor::INVALID_BOX = 255;
+
if (_gameId == GID_ZAK256) { // FmTowns is 320x240
_screenWidth = 320;
_screenHeight = 240;
More information about the Scummvm-git-logs
mailing list