[Scummvm-cvs-logs] scummvm master -> 00b37f8552962c26412eda347024db80de3d99ac
bluegr
md5 at scummvm.org
Thu Mar 3 18:39:57 CET 2011
This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
1aed9a1f34 SCI: Applied save/load dialog patching to all SCI0-SCI2.1 early games
51437ba5e6 SCI: Fixed path finding in Amiga SCI1 games
00b37f8552 SCI: Simplified arithmetic reg_t operations, based on fingolfin's comments
Commit: 1aed9a1f3401477d4d56e278fec63cd90fafd81d
https://github.com/scummvm/scummvm/commit/1aed9a1f3401477d4d56e278fec63cd90fafd81d
Author: md5 (md5 at scummvm.org)
Date: 2011-03-03T09:31:47-08:00
Commit Message:
SCI: Applied save/load dialog patching to all SCI0-SCI2.1 early games
Games that have the newer SCI2.1 kernel functions (i.e. kSave instead of
kSaveGame/kRestoreGame) aren't supported yet
Changed paths:
engines/sci/sci.cpp
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 4155fd2..657a7b9 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -444,9 +444,8 @@ static void patchGameSaveRestoreCode(SegManager *segMan, reg_t methodAddress, by
byte *patchPtr = const_cast<byte *>(script->getBuf(methodAddress.offset));
if (getSciVersion() <= SCI_VERSION_1_1)
memcpy(patchPtr, patchGameRestoreSave, sizeof(patchGameRestoreSave));
- else if (getSciVersion() == SCI_VERSION_2)
+ else // SCI2+
memcpy(patchPtr, patchGameRestoreSaveSci2, sizeof(patchGameRestoreSaveSci2));
- // TODO: SCI21/SCI3
patchPtr[8] = id;
}
@@ -459,10 +458,6 @@ void SciEngine::patchGameSaveRestore() {
byte kernelIdRestore = 0;
byte kernelIdSave = 0;
- // This feature is currently not supported in SCI21 or SCI3
- if (getSciVersion() >= SCI_VERSION_2_1)
- return;
-
switch (_gameId) {
case GID_MOTHERGOOSE256: // mother goose saves/restores directly and has no save/restore dialogs
case GID_JONES: // gets confused, when we patch us in, the game is only able to save to 1 slot, so hooking is not required
@@ -485,6 +480,11 @@ void SciEngine::patchGameSaveRestore() {
kernelIdSave = kernelNr;
}
+ // TODO: This feature does not yet work with the SCI2.1 middle and newer
+ // kernel functions (i.e. kSave)
+ if (!kernelIdRestore || !kernelIdSave)
+ return;
+
// Search for gameobject superclass ::restore
uint16 gameSuperObjectMethodCount = gameSuperObject->getMethodCount();
for (uint16 methodNr = 0; methodNr < gameSuperObjectMethodCount; methodNr++) {
Commit: 51437ba5e6e418d7e79cf341606bc6c1c50fd50b
https://github.com/scummvm/scummvm/commit/51437ba5e6e418d7e79cf341606bc6c1c50fd50b
Author: md5 (md5 at scummvm.org)
Date: 2011-03-03T09:34:11-08:00
Commit Message:
SCI: Fixed path finding in Amiga SCI1 games
Added wrapper functions to read/write from dynmem segments, as these are
treated as BE in Amiga versions (as we treat them like raw data instead
of reg_t's), whereas the rest are LE. Thanks to waltervn and wjp for their
help on this
Changed paths:
engines/sci/engine/kpathing.cpp
engines/sci/util.cpp
engines/sci/util.h
diff --git a/engines/sci/engine/kpathing.cpp b/engines/sci/engine/kpathing.cpp
index cb70cf9..c98d93c 100644
--- a/engines/sci/engine/kpathing.cpp
+++ b/engines/sci/engine/kpathing.cpp
@@ -264,9 +264,9 @@ struct PathfindingState {
static Common::Point readPoint(SegmentRef list_r, int offset) {
Common::Point point;
- if (list_r.isRaw) {
- point.x = (int16)READ_LE_UINT16(list_r.raw + offset * POLY_POINT_SIZE);
- point.y = (int16)READ_LE_UINT16(list_r.raw + offset * POLY_POINT_SIZE + 2);
+ if (list_r.isRaw) { // dynmem blocks are raw
+ point.x = (int16)READ_SCI1ENDIAN_UINT16(list_r.raw + offset * POLY_POINT_SIZE);
+ point.y = (int16)READ_SCI1ENDIAN_UINT16(list_r.raw + offset * POLY_POINT_SIZE + 2);
} else {
point.x = list_r.reg[offset * 2].toUint16();
point.y = list_r.reg[offset * 2 + 1].toUint16();
@@ -275,9 +275,9 @@ static Common::Point readPoint(SegmentRef list_r, int offset) {
}
static void writePoint(SegmentRef ref, int offset, const Common::Point &point) {
- if (ref.isRaw) {
- WRITE_LE_UINT16(ref.raw + offset * POLY_POINT_SIZE, point.x);
- WRITE_LE_UINT16(ref.raw + offset * POLY_POINT_SIZE + 2, point.y);
+ if (ref.isRaw) { // dynmem blocks are raw
+ WRITE_SCI1ENDIAN_UINT16(ref.raw + offset * POLY_POINT_SIZE, point.x);
+ WRITE_SCI1ENDIAN_UINT16(ref.raw + offset * POLY_POINT_SIZE + 2, point.y);
} else {
ref.reg[offset * 2] = make_reg(0, point.x);
ref.reg[offset * 2 + 1] = make_reg(0, point.y);
diff --git a/engines/sci/util.cpp b/engines/sci/util.cpp
index f6a2465..6bc08e6 100644
--- a/engines/sci/util.cpp
+++ b/engines/sci/util.cpp
@@ -30,6 +30,20 @@
namespace Sci {
+uint16 READ_SCI1ENDIAN_UINT16(const void *ptr) {
+ if (g_sci->getPlatform() == Common::kPlatformAmiga && getSciVersion() >= SCI_VERSION_1_EGA_ONLY && getSciVersion() <= SCI_VERSION_1_LATE)
+ return READ_BE_UINT16(ptr);
+
+ return READ_LE_UINT16(ptr);
+}
+
+void WRITE_SCI1ENDIAN_UINT16(void *ptr, uint16 val) {
+ if (g_sci->getPlatform() == Common::kPlatformAmiga && getSciVersion() >= SCI_VERSION_1_EGA_ONLY && getSciVersion() <= SCI_VERSION_1_LATE)
+ WRITE_BE_UINT16(ptr, val);
+
+ WRITE_LE_UINT16(ptr, val);
+}
+
uint16 READ_SCI11ENDIAN_UINT16(const void *ptr) {
if (g_sci->getPlatform() == Common::kPlatformMacintosh && getSciVersion() >= SCI_VERSION_1_1)
return READ_BE_UINT16(ptr);
diff --git a/engines/sci/util.h b/engines/sci/util.h
index d9ced5c..d1f1c2f 100644
--- a/engines/sci/util.h
+++ b/engines/sci/util.h
@@ -30,6 +30,12 @@
namespace Sci {
+// Wrappers for reading/writing integer values for SCI1 Amiga.
+// Amiga versions store big endian data in dynmem blocks, as
+// the game resources are in LE, but the actual system is BE.
+uint16 READ_SCI1ENDIAN_UINT16(const void *ptr);
+void WRITE_SCI1ENDIAN_UINT16(void *ptr, uint16 val);
+
// Wrappers for reading integer values for SCI1.1+.
// Mac versions have big endian data for some fields.
uint16 READ_SCI11ENDIAN_UINT16(const void *ptr);
Commit: 00b37f8552962c26412eda347024db80de3d99ac
https://github.com/scummvm/scummvm/commit/00b37f8552962c26412eda347024db80de3d99ac
Author: md5 (md5 at scummvm.org)
Date: 2011-03-03T09:38:30-08:00
Commit Message:
SCI: Simplified arithmetic reg_t operations, based on fingolfin's comments
- Folded all comparison operators in a single function, cmp()
- Simplified the + operator, and removed the SQ1 workaround, as it's not
needed anymore
- Removed the workaround for uninitialized variables in the * operator
- Removed division by zero workarounds in the / and % operators
- Added a better description of pointerComparisonWithInteger(), based
on fingolfin's description and comments. Also, changed the SCI versions
where this is used to SCI0-SCI1. The SCI1.1 case in QFG3 was a script
bug
Changed paths:
engines/sci/engine/vm_types.cpp
engines/sci/engine/vm_types.h
diff --git a/engines/sci/engine/vm_types.cpp b/engines/sci/engine/vm_types.cpp
index 85c83bb..c2119aa 100644
--- a/engines/sci/engine/vm_types.cpp
+++ b/engines/sci/engine/vm_types.cpp
@@ -35,7 +35,7 @@ reg_t reg_t::lookForWorkaround(const reg_t right) const {
SciTrackOriginReply originReply;
SciWorkaroundSolution solution = trackOriginAndFindWorkaround(0, arithmeticWorkarounds, &originReply);
if (solution.type == WORKAROUND_NONE)
- error("arithmetic operation on non-integer (%04x:%04x, %04x:%04x) from method %s::%s (script %d, room %d, localCall %x)",
+ error("Invalid arithmetic operation (params: %04x:%04x and %04x:%04x) from method %s::%s (script %d, room %d, localCall %x)",
PRINT_REG(*this), PRINT_REG(right), originReply.objectName.c_str(),
originReply.methodName.c_str(), originReply.scriptNr, g_sci->getEngineState()->currentRoomNumber(),
originReply.localCallOffset);
@@ -44,7 +44,7 @@ reg_t reg_t::lookForWorkaround(const reg_t right) const {
}
reg_t reg_t::operator+(const reg_t right) const {
- if (isPointer()) {
+ if (isPointer() && right.isNumber()) {
// Pointer arithmetics. Only some pointer types make sense here
SegmentObj *mobj = g_sci->getEngineState()->_segMan->getSegmentObj(segment);
@@ -56,9 +56,6 @@ reg_t reg_t::operator+(const reg_t right) const {
case SEG_TYPE_SCRIPT:
case SEG_TYPE_STACK:
case SEG_TYPE_DYNMEM:
- // Make sure that we are adding an offset to the pointer
- if (!right.isNumber())
- return lookForWorkaround(right);
return make_reg(segment, offset + right.toSint16());
default:
return lookForWorkaround(right);
@@ -66,19 +63,11 @@ reg_t reg_t::operator+(const reg_t right) const {
} else if (isNumber() && right.isPointer()) {
// Adding a pointer to a number, flip the order
return right + *this;
+ } else if (isNumber() && right.isNumber()) {
+ // Normal arithmetics
+ return make_reg(0, toSint16() + right.toSint16());
} else {
- // Normal arithmetics. Make sure we're adding a number
- if (right.isPointer())
- return lookForWorkaround(right);
- // If the current variable is uninitialized, it'll be set
- // to zero in order to perform the operation. Such a case
- // happens in SQ1, room 28, when throwing the water at Orat.
- if (!isInitialized())
- return make_reg(0, right.toSint16());
- else if (!right.isInitialized())
- return *this;
- else
- return make_reg(0, toSint16() + right.toSint16());
+ return lookForWorkaround(right);
}
}
@@ -95,24 +84,19 @@ reg_t reg_t::operator-(const reg_t right) const {
reg_t reg_t::operator*(const reg_t right) const {
if (isNumber() && right.isNumber())
return make_reg(0, toSint16() * right.toSint16());
- else if (!isInitialized() || !right.isInitialized())
- return NULL_REG; // unitialized variables - always return 0
else
return lookForWorkaround(right);
}
reg_t reg_t::operator/(const reg_t right) const {
- if (isNumber() && right.isNumber()) {
- if (right.isNull())
- return NULL_REG; // division by zero
- else
- return make_reg(0, toSint16() / right.toSint16());
- } else
+ if (isNumber() && right.isNumber() && !right.isNull())
+ return make_reg(0, toSint16() / right.toSint16());
+ else
return lookForWorkaround(right);
}
reg_t reg_t::operator%(const reg_t right) const {
- if (isNumber() && right.isNumber()) {
+ if (isNumber() && right.isNumber() && !right.isNull()) {
// Support for negative numbers was added in Iceman, and perhaps in
// SCI0 0.000.685 and later. Theoretically, this wasn't really used
// in SCI0, so the result is probably unpredictable. Such a case
@@ -123,7 +107,7 @@ reg_t reg_t::operator%(const reg_t right) const {
warning("Modulo of a negative number has been requested for SCI0. This *could* lead to issues");
int16 value = toSint16();
int16 modulo = ABS(right.toSint16());
- int16 result = (modulo != 0 ? value % modulo : 0);
+ int16 result = value % modulo;
if (result < 0)
result += modulo;
return make_reg(0, result);
@@ -157,6 +141,8 @@ uint16 reg_t::requireUint16() const {
if (isNumber())
return toUint16();
else
+ // The right parameter is NULL_REG because
+ // we're not comparing *this with anything here.
return lookForWorkaround(NULL_REG).toUint16();
}
@@ -164,6 +150,8 @@ int16 reg_t::requireSint16() const {
if (isNumber())
return toSint16();
else
+ // The right parameter is NULL_REG because
+ // we're not comparing *this with anything here.
return lookForWorkaround(NULL_REG).toSint16();
}
@@ -188,62 +176,35 @@ reg_t reg_t::operator^(const reg_t right) const {
return lookForWorkaround(right);
}
-bool reg_t::operator>(const reg_t right) const {
- if (isNumber() && right.isNumber())
- return toSint16() > right.toSint16();
- else if (isPointer() && segment == right.segment)
- return toUint16() > right.toUint16(); // pointer comparison
- else if (pointerComparisonWithInteger(right))
- return true;
- else if (right.pointerComparisonWithInteger(*this))
- return false;
- else
- return lookForWorkaround(right).toSint16();
-}
-
-bool reg_t::operator<(const reg_t right) const {
- if (isNumber() && right.isNumber())
- return toSint16() < right.toSint16();
- else if (isPointer() && segment == right.segment)
- return toUint16() < right.toUint16(); // pointer comparison
- else if (pointerComparisonWithInteger(right))
- return false;
- else if (right.pointerComparisonWithInteger(*this))
- return true;
- else
- return lookForWorkaround(right).toSint16();
-}
-
-bool reg_t::gtU(const reg_t right) const {
- if (isNumber() && right.isNumber())
- return toUint16() > right.toUint16();
- else if (isPointer() && segment == right.segment)
- return toUint16() > right.toUint16(); // pointer comparison
- else if (pointerComparisonWithInteger(right))
- return true;
- else if (right.pointerComparisonWithInteger(*this))
- return false;
- else
- return lookForWorkaround(right).toSint16();
-}
-
-bool reg_t::ltU(const reg_t right) const {
- if (isNumber() && right.isNumber())
- return toUint16() < right.toUint16();
- else if (isPointer() && segment == right.segment)
- return toUint16() < right.toUint16(); // pointer comparison
- else if (pointerComparisonWithInteger(right))
- return false;
- else if (right.pointerComparisonWithInteger(*this))
- return true;
- else
+int reg_t::cmp(const reg_t right, bool treatAsUnsigned) const {
+ if (segment == right.segment) { // can compare things in the same segment
+ if (treatAsUnsigned || !isNumber())
+ return toUint16() - right.toUint16();
+ else
+ return toSint16() - right.toSint16();
+ } else if (pointerComparisonWithInteger(right)) {
+ return 1;
+ } else if (right.pointerComparisonWithInteger(*this)) {
+ return -1;
+ } else
return lookForWorkaround(right).toSint16();
}
bool reg_t::pointerComparisonWithInteger(const reg_t right) const {
- // SCI0 - SCI1.1 scripts use this to check whether a parameter is a pointer
- // or a far text reference. It is used e.g. by the standard library Print
- // function to distinguish two ways of calling it:
+ // This function handles the case where a script tries to compare a pointer
+ // to a number. Normally, we would not want to allow that. However, SCI0 -
+ // SCI1 scripts do this in order to distinguish pointers (to resources)
+ // from plain numbers. In our SCI implementation, such a check may seem
+ // pointless, as one can simply use the segment value to achieve this goal.
+ // But Sierra's SCI did not have the notion of segment IDs, so both pointer
+ // and numbers were simple integers.
+ // But for some things, scripts had (and have) to distinguish between
+ // numbers and pointers. Lacking the segment information, Sierra's
+ // developers resorted to a hack: If an integer is smaller than a certain
+ // bound, it can be assumed to be a number, otherwise it is assumed to be a
+ // pointer. This allowed them to implement polymorphic functions, such as
+ // the Print function, which can be called in two different ways, with a
+ // pointer or a far text reference:
//
// (Print "foo") // Pointer to a string
// (Print 420 5) // Reference to the fifth message in text resource 420
@@ -256,9 +217,8 @@ bool reg_t::pointerComparisonWithInteger(const reg_t right) const {
// Hoyle 3, Pachisi, when any opponent is about to talk
// SQ1, room 28, when throwing water at the Orat
// SQ1, room 58, when giving the ID card to the robot
- // QFG3, room 440, when talking to Uhura
// Thus we check for all integers <= 2000
- return (isPointer() && right.isNumber() && right.offset <= 2000 && getSciVersion() <= SCI_VERSION_1_1);
+ return (isPointer() && right.isNumber() && right.offset <= 2000 && getSciVersion() <= SCI_VERSION_1_LATE);
}
} // End of namespace Sci
diff --git a/engines/sci/engine/vm_types.h b/engines/sci/engine/vm_types.h
index 82c881d..b927df3 100644
--- a/engines/sci/engine/vm_types.h
+++ b/engines/sci/engine/vm_types.h
@@ -73,32 +73,38 @@ struct reg_t {
return (offset != x.offset) || (segment != x.segment);
}
- bool operator>(const reg_t right) const;
+ bool operator>(const reg_t right) const {
+ return cmp(right, false) > 0;
+ }
+
bool operator>=(const reg_t right) const {
- if (*this == right)
- return true;
- return *this > right;
+ return cmp(right, false) >= 0;
+ }
+
+ bool operator<(const reg_t right) const {
+ return cmp(right, false) < 0;
}
- bool operator<(const reg_t right) const;
+
bool operator<=(const reg_t right) const {
- if (*this == right)
- return true;
- return *this < right;
+ return cmp(right, false) <= 0;
}
// Same as the normal operators, but perform unsigned
// integer checking
- bool gtU(const reg_t right) const;
+ bool gtU(const reg_t right) const {
+ return cmp(right, true) > 0;
+ }
+
bool geU(const reg_t right) const {
- if (*this == right)
- return true;
- return gtU(right);
+ return cmp(right, true) >= 0;
+ }
+
+ bool ltU(const reg_t right) const {
+ return cmp(right, true) < 0;
}
- bool ltU(const reg_t right) const;
+
bool leU(const reg_t right) const {
- if (*this == right)
- return true;
- return ltU(right);
+ return cmp(right, true) <= 0;
}
// Arithmetic operators
@@ -124,6 +130,14 @@ struct reg_t {
reg_t operator^(const reg_t right) const;
private:
+ /**
+ * Compares two reg_t's.
+ * Returns:
+ * - a positive number if *this > right
+ * - 0 if *this == right
+ * - a negative number if *this < right
+ */
+ int cmp(const reg_t right, bool treatAsUnsigned) const;
reg_t lookForWorkaround(const reg_t right) const;
bool pointerComparisonWithInteger(const reg_t right) const;
};
More information about the Scummvm-git-logs
mailing list