[Scummvm-git-logs] scummvm master -> ef69a594672bb1e6549269ca695d6feae3a9c579
csnover
csnover at users.noreply.github.com
Sun May 21 04:20:47 CEST 2017
This automated email contains information about 13 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
cb657c0c0f SCI: Ignore patch resources with .DOS and .WIN extensions
14a521a211 SCI32: Fix kPlatform operation for SCI2 through SCI2.1early
66efb750a0 SCI: Add more support for >16-bit SCI3 offsets
881be25fcd SCI: Stop making copies of ObjMap and remove related dead code
bc9835ba5e SCI: Always use SegManager::getObjectName to get object names
71630a7cb2 SCI: Remove duplicate relocateBlock function
eda836f21a SCI: Nitpicky cleanup of some magic numbers and what-not-why comments
d09ae57fd8 SCI32: Remove bad assertion in relocateSci3
1f29e6f241 SCI: Refactor relocation code
4917330038 SCI: Find and store the original static names of objects
eba1e883e9 SCI: Minor punctuation fix in buggy script alert
bc728a1c93 SCI: Fix warning about missing base object when loading save games
ef69a59467 SCI: Stop leaking locals segments during script reuse
Commit: cb657c0c0f0ba291218cde205f3f8ab5d228ff52
https://github.com/scummvm/scummvm/commit/cb657c0c0f0ba291218cde205f3f8ab5d228ff52
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Ignore patch resources with .DOS and .WIN extensions
Type mismatch is triggered on THEGUIDE.DOS and THEGUIDE.WIN from
at least Phant1 French 1.100.000.
Changed paths:
engines/sci/resource.cpp
diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 068be66..42c1027 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -1598,7 +1598,7 @@ void ResourceManager::readResourcePatchesBase36() {
// The S/T prefixes often conflict with non-patch files and generate
// spurious warnings about invalid patches
- if (name.hasSuffix(".DLL") || name.hasSuffix(".EXE") || name.hasSuffix(".TXT") || name.hasSuffix(".OLD")) {
+ if (name.hasSuffix(".DLL") || name.hasSuffix(".EXE") || name.hasSuffix(".TXT") || name.hasSuffix(".OLD") || name.hasSuffix(".WIN") || name.hasSuffix(".DOS")) {
continue;
}
Commit: 14a521a21177361184fee38242065a64c5fcdf05
https://github.com/scummvm/scummvm/commit/14a521a21177361184fee38242065a64c5fcdf05
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI32: Fix kPlatform operation for SCI2 through SCI2.1early
Fixes Trac#9795.
Changed paths:
engines/sci/engine/kmisc.cpp
diff --git a/engines/sci/engine/kmisc.cpp b/engines/sci/engine/kmisc.cpp
index f0090cf..31b9f77 100644
--- a/engines/sci/engine/kmisc.cpp
+++ b/engines/sci/engine/kmisc.cpp
@@ -617,7 +617,16 @@ reg_t kPlatform32(EngineState *s, int argc, reg_t *argv) {
kGetCDDrive = 3
};
- const Operation operation = argc > 0 ? (Operation)argv[0].toSint16() : kGetPlatform;
+ Operation operation;
+ if (getSciVersion() < SCI_VERSION_2_1_MIDDLE) {
+ if (argc == 0 || argv[0].toSint16() == 0) {
+ operation = kGetPlatform;
+ } else {
+ return NULL_REG;
+ }
+ } else {
+ operation = argc > 0 ? (Operation)argv[0].toSint16() : kGetPlatform;
+ }
switch (operation) {
case kGetPlatform:
@@ -640,7 +649,7 @@ reg_t kPlatform32(EngineState *s, int argc, reg_t *argv) {
case kGetCDSpeed:
case kGetCDDrive:
default:
- return make_reg(0, 0);
+ return NULL_REG;
}
}
Commit: 66efb750a0ccaa3c2ff8b77f60544a345328da8b
https://github.com/scummvm/scummvm/commit/66efb750a0ccaa3c2ff8b77f60544a345328da8b
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Add more support for >16-bit SCI3 offsets
Basically just grepped for getOffset calls being assigned to
uint16s and expanded those to uint32 when they looked trivial.
While some of these changes seem superfluous, at least for the
US/English SCI3 games where potentially impacted game scripts are
not large enough to have a problem with 16-bit offsets (e.g. when
feature detecting the sound type), at least some of these changes
are necessary for correct operation of the find_callk debugger
command in SCI3 games. There should not be a reason why any of
these variables need to be kept as uint16, in any case.
Changed paths:
engines/sci/console.cpp
engines/sci/engine/features.cpp
engines/sci/engine/kscripts.cpp
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index d67bd69..bc11534 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -1104,8 +1104,8 @@ bool Console::cmdVerifyScripts(int argc, const char **argv) {
debugPrintf("Error: script and heap %d together are larger than 64KB (%u bytes)\n",
itr->getNumber(), script->size() + heap->size());
} else { // SCI3
- if (script && script->size() > 65535)
- debugPrintf("Error: script %d is larger than 64KB (%u bytes)\n",
+ if (script && script->size() > 0x3FFFF)
+ debugPrintf("Error: script %d is larger than 256KB (%u bytes)\n",
itr->getNumber(), script->size());
}
}
@@ -1922,7 +1922,7 @@ bool Console::cmdSavedBits(int argc, const char **argv) {
Common::Array<reg_t> entries = hunks->listAllDeallocatable(id);
for (uint i = 0; i < entries.size(); ++i) {
- uint16 offset = entries[i].getOffset();
+ uint32 offset = entries[i].getOffset();
const Hunk& h = hunks->at(offset);
if (strcmp(h.type, "SaveBits()") == 0) {
byte* memoryPtr = (byte *)h.mem;
@@ -3556,7 +3556,7 @@ void Console::printKernelCallsFound(int kernelFuncNum, bool showFoundScripts) {
// Now dissassemble each method of the script object
for (uint16 i = 0; i < obj->getMethodCount(); i++) {
reg_t fptr = obj->getFunction(i);
- uint16 offset = fptr.getOffset();
+ uint32 offset = fptr.getOffset();
int16 opparams[4];
byte extOpcode;
byte opcode;
diff --git a/engines/sci/engine/features.cpp b/engines/sci/engine/features.cpp
index 1e8cc6d..9251ac6 100644
--- a/engines/sci/engine/features.cpp
+++ b/engines/sci/engine/features.cpp
@@ -77,7 +77,7 @@ bool GameFeatures::autoDetectSoundType() {
if (!addr.getSegment())
return false;
- uint16 offset = addr.getOffset();
+ uint32 offset = addr.getOffset();
Script *script = _segMan->getScript(addr.getSegment());
uint16 intParam = 0xFFFF;
bool foundTarget = false;
@@ -224,7 +224,7 @@ bool GameFeatures::autoDetectLofsType(Common::String gameSuperClassName, int met
if (!addr.getSegment())
return false;
- uint16 offset = addr.getOffset();
+ uint32 offset = addr.getOffset();
Script *script = _segMan->getScript(addr.getSegment());
while (true) {
@@ -323,7 +323,7 @@ bool GameFeatures::autoDetectGfxFunctionsType(int methodNum) {
if (!addr.getSegment())
return false;
- uint16 offset = addr.getOffset();
+ uint32 offset = addr.getOffset();
Script *script = _segMan->getScript(addr.getSegment());
while (true) {
@@ -485,7 +485,7 @@ bool GameFeatures::autoDetectSci21KernelType() {
if (!addr.getSegment())
return false;
- uint16 offset = addr.getOffset();
+ uint32 offset = addr.getOffset();
Script *script = _segMan->getScript(addr.getSegment());
while (true) {
@@ -620,7 +620,7 @@ bool GameFeatures::autoDetectMoveCountType() {
if (!addr.getSegment())
return false;
- uint16 offset = addr.getOffset();
+ uint32 offset = addr.getOffset();
Script *script = _segMan->getScript(addr.getSegment());
bool foundTarget = false;
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index 98c35fc..af4b8ff 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -160,7 +160,7 @@ reg_t kClone(EngineState *s, int argc, reg_t *argv) {
debugC(kDebugLevelMemory, "Attempting to clone from %04x:%04x", PRINT_REG(parentAddr));
- uint16 infoSelector = parentObj->getInfoSelector().getOffset();
+ uint16 infoSelector = parentObj->getInfoSelector().toUint16();
cloneObj = s->_segMan->allocateClone(&cloneAddr);
if (!cloneObj) {
@@ -211,7 +211,7 @@ reg_t kDisposeClone(EngineState *s, int argc, reg_t *argv) {
// At least kq4early relies on this behavior. The scripts clone "Sound", then set bit 1 manually
// and call kDisposeClone later. In that case we may not free it, otherwise we will run into issues
// later, because kIsObject would then return false and Sound object wouldn't get checked.
- uint16 infoSelector = object->getInfoSelector().getOffset();
+ uint16 infoSelector = object->getInfoSelector().toUint16();
if ((infoSelector & 3) == kInfoFlagClone)
object->markAsFreed();
Commit: 881be25fcd82d765235e95ebf041a70ec1ae5ae5
https://github.com/scummvm/scummvm/commit/881be25fcd82d765235e95ebf041a70ec1ae5ae5
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Stop making copies of ObjMap and remove related dead code
ObjMap owns Objects, so every time this map gets copied instead of
referenced, it creates a copy of every single object in the
associated script. This is expensive, and it breaks things like
the `Object::syncBaseObject` call in savegame.cpp, which hasn't
actually been doing anything since
58190c36b4cc84b3200239211d91b0291301db56 because it has been
operating on copies.
Changed paths:
engines/sci/console.cpp
engines/sci/engine/object.h
engines/sci/engine/savegame.cpp
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index bc11534..0e90f22 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -2175,11 +2175,11 @@ bool Console::segmentInfo(int nr) {
else
debugPrintf(" Locals : none\n");
- ObjMap objects = scr->getObjectMap();
+ const ObjMap &objects = scr->getObjectMap();
debugPrintf(" Objects: %4d\n", objects.size());
- ObjMap::iterator it;
- const ObjMap::iterator end = objects.end();
+ ObjMap::const_iterator it;
+ const ObjMap::const_iterator end = objects.end();
for (it = objects.begin(); it != end; ++it) {
debugPrintf(" ");
// Object header
@@ -3546,9 +3546,9 @@ void Console::printKernelCallsFound(int kernelFuncNum, bool showFoundScripts) {
script = customSegMan->getScript(scriptSegment);
// Iterate through all the script's objects
- ObjMap objects = script->getObjectMap();
- ObjMap::iterator it;
- const ObjMap::iterator end = objects.end();
+ const ObjMap &objects = script->getObjectMap();
+ ObjMap::const_iterator it;
+ const ObjMap::const_iterator end = objects.end();
for (it = objects.begin(); it != end; ++it) {
const Object *obj = customSegMan->getObject(it->_value.getPos());
const char *objName = customSegMan->getObjectName(it->_value.getPos());
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 136a831..71b366d 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -309,7 +309,6 @@ public:
void initSpecies(SegManager *segMan, reg_t addr);
void initSuperClass(SegManager *segMan, reg_t addr);
bool initBaseObject(SegManager *segMan, reg_t addr, bool doInitSuperClass = true);
- void syncBaseObject(const SciSpan<const byte> &ptr) { _baseObj = ptr; }
#ifdef ENABLE_SCI32
bool mustSetViewVisible(const int index, const bool fromPropertyOp) const;
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index dce6430..1095b7f 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -255,11 +255,6 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
if (s.isLoading()) {
// Hook the script up in the script->segment map
_scriptSegMap[scr->getScriptNumber()] = i;
-
- ObjMap objects = scr->getObjectMap();
- for (ObjMap::iterator it = objects.begin(); it != objects.end(); ++it) {
- it->_value.syncBaseObject(scr->getSpan(it->_value.getPos().getOffset()));
- }
}
// Sync the script's string heap
@@ -287,7 +282,7 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
Script *scr = (Script *)_heap[i];
scr->syncLocalsBlock(this);
- ObjMap objects = scr->getObjectMap();
+ ObjMap &objects = scr->getObjectMap();
for (ObjMap::iterator it = objects.begin(); it != objects.end(); ++it) {
reg_t addr = it->_value.getPos();
Object *obj = scr->scriptObjInit(addr, false);
Commit: bc9835ba5ee32679a715f4f5f734786b7120191f
https://github.com/scummvm/scummvm/commit/bc9835ba5ee32679a715f4f5f734786b7120191f
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Always use SegManager::getObjectName to get object names
This ensures that all object name reading code works the same and
is in one place in the codebase.
Changed paths:
engines/sci/console.cpp
engines/sci/engine/guest_additions.cpp
diff --git a/engines/sci/console.cpp b/engines/sci/console.cpp
index 0e90f22..99c5498 100644
--- a/engines/sci/console.cpp
+++ b/engines/sci/console.cpp
@@ -4211,14 +4211,14 @@ bool Console::cmdMapVocab994(int argc, const char **argv) {
if (obj && ofs < obj->getVarCount()) {
uint16 varSelector = obj->getVarSelector(ofs);
debugPrintf("%d: property at index %04x of %s is %s %s\n", i, ofs,
- s->_segMan->derefString(obj->getNameSelector()),
+ s->_segMan->getObjectName(reg),
_engine->getKernel()->getSelectorName(varSelector).c_str(),
markers[varSelector] ? "(repeat!)" : "");
markers[varSelector] = true;
}
else {
debugPrintf("%d: property at index %04x doesn't match up with %s\n", i, ofs,
- s->_segMan->derefString(obj->getNameSelector()));
+ s->_segMan->getObjectName(reg));
}
}
diff --git a/engines/sci/engine/guest_additions.cpp b/engines/sci/engine/guest_additions.cpp
index e2b7354..23f600f 100644
--- a/engines/sci/engine/guest_additions.cpp
+++ b/engines/sci/engine/guest_additions.cpp
@@ -359,14 +359,10 @@ static const byte SRDialogPatch[] = {
};
void GuestAdditions::patchGameSaveRestoreSCI32(Script &script) const {
- ObjMap &objMap = script.getObjectMap();
- for (ObjMap::iterator it = objMap.begin(); it != objMap.end(); ++it) {
- Object &obj = it->_value;
- if (obj.getNameSelector().isNull()) {
- continue;
- }
-
- if (Common::String(_segMan->derefString(obj.getNameSelector())) != "SRDialog") {
+ const ObjMap &objMap = script.getObjectMap();
+ for (ObjMap::const_iterator it = objMap.begin(); it != objMap.end(); ++it) {
+ const Object &obj = it->_value;
+ if (strncmp(_segMan->getObjectName(obj.getPos()), "SRDialog", 8) != 0) {
continue;
}
Commit: 71630a7cb242d208518bb9ff19e414211edae74c
https://github.com/scummvm/scummvm/commit/71630a7cb242d208518bb9ff19e414211edae74c
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Remove duplicate relocateBlock function
Changed paths:
engines/sci/engine/object.cpp
engines/sci/engine/script.cpp
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index a09d530..079106f 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -30,29 +30,8 @@
namespace Sci {
-// This helper function is used by Script::relocateLocal and Object::relocate
-// Duplicate in segment.cpp and script.cpp
-static bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, size_t scriptSize) {
- int rel = location - block_location;
+extern bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, uint32 heapOffset);
- if (rel < 0)
- return false;
-
- uint idx = rel >> 1;
-
- if (idx >= block.size())
- return false;
-
- if (rel & 1) {
- error("Attempt to relocate odd variable #%d.5e (relative to %04x)\n", idx, block_location);
- return false;
- }
- block[idx].setSegment(segment); // Perform relocation
- if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE)
- block[idx].incOffset(scriptSize);
-
- return true;
-}
void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariables) {
const SciSpan<const byte> data = buf.subspan(obj_pos.getOffset());
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 7e3a661..2f04534 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -661,9 +661,7 @@ Object *Script::scriptObjInit(reg_t obj_pos, bool fullObjectInit) {
return obj;
}
-// This helper function is used by Script::relocateLocal and Object::relocate
-// Duplicate in segment.cpp and script.cpp
-static bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, size_t scriptSize) {
+bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, uint32 heapOffset) {
int rel = location - block_location;
if (rel < 0)
Commit: eda836f21a3331e5a3a1e3c3965081f64a6e64eb
https://github.com/scummvm/scummvm/commit/eda836f21a3331e5a3a1e3c3965081f64a6e64eb
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Nitpicky cleanup of some magic numbers and what-not-why comments
Changed paths:
engines/sci/engine/object.cpp
engines/sci/engine/savegame.cpp
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 079106f..0ab8449 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -54,8 +54,8 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
}
_methodCount = data.getUint16LEAt(header.getUint16LEAt(kOffsetHeaderFunctionArea) - 2);
- for (int i = 0; i < _methodCount * 2 + 2; ++i) {
- _baseMethod.push_back(data.getUint16SEAt(header.getUint16LEAt(kOffsetHeaderFunctionArea) + i * 2));
+ for (uint i = 0; i < _methodCount * sizeof(uint16) + 2; ++i) {
+ _baseMethod.push_back(data.getUint16SEAt(header.getUint16LEAt(kOffsetHeaderFunctionArea) + i * sizeof(uint16)));
}
} else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
_variables.resize(data.getUint16SEAt(2));
@@ -72,8 +72,8 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
}
_methodCount = buf.getUint16SEAt(data.getUint16SEAt(6));
- for (int i = 0; i < _methodCount * 2 + 3; ++i) {
- _baseMethod.push_back(buf.getUint16SEAt(data.getUint16SEAt(6) + i * 2));
+ for (uint i = 0; i < _methodCount * sizeof(uint16) + 3; ++i) {
+ _baseMethod.push_back(buf.getUint16SEAt(data.getUint16SEAt(6) + i * sizeof(uint16)));
}
#ifdef ENABLE_SCI32
} else if (getSciVersion() == SCI_VERSION_3) {
@@ -90,7 +90,7 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
{
#endif
for (uint i = 0; i < _variables.size(); i++)
- _variables[i] = make_reg(0, data.getUint16SEAt(i * 2));
+ _variables[i] = make_reg(0, data.getUint16SEAt(i * sizeof(uint16)));
}
}
}
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 1095b7f..8767744 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -233,7 +233,6 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
}
#ifdef ENABLE_SCI32
} else if (type == SEG_TYPE_ARRAY) {
- // Set the correct segment for SCI32 arrays
_arraysSegId = i;
} else if (s.getVersion() >= 36 && type == SEG_TYPE_BITMAP) {
_bitmapSegId = i;
@@ -251,13 +250,10 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
if (type == SEG_TYPE_SCRIPT) {
Script *scr = (Script *)mobj;
- // If we are loading a script, perform some extra steps
if (s.isLoading()) {
- // Hook the script up in the script->segment map
_scriptSegMap[scr->getScriptNumber()] = i;
}
- // Sync the script's string heap
if (s.getVersion() >= 28)
scr->syncStringHeap(s);
}
Commit: d09ae57fd8a22e3b0a3946a028a500c15f05aa0a
https://github.com/scummvm/scummvm/commit/d09ae57fd8a22e3b0a3946a028a500c15f05aa0a
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI32: Remove bad assertion in relocateSci3
While extremely rare (only Rama script 64948 seems to have this
profile), it *is* possible for an object to have zero properties
(and thus, zero property offsets).
Changed paths:
engines/sci/engine/object.cpp
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 0ab8449..386c3e2 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -129,7 +129,6 @@ bool Object::relocateSci0Sci21(SegmentId segment, int location, size_t scriptSiz
#ifdef ENABLE_SCI32
bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize) {
- assert(_propertyOffsetsSci3.size());
assert(offset >= 0 && (uint)offset < scriptSize);
for (uint i = 0; i < _variables.size(); ++i) {
Commit: 1f29e6f241f77e028b31d72d6d4adaf8ce1b29ae
https://github.com/scummvm/scummvm/commit/1f29e6f241f77e028b31d72d6d4adaf8ce1b29ae
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Refactor relocation code
This groundwork enables an object to look up its static name
separately from the normal process that is used to populate
Object::_variables when an object is first constructed.
(The static name property needs to be able to be retrieved from
objects inside of earlier save games whose name properties may
have already been modified at runtime, so the code cannot simply
pluck the value out of Object::_variables when they are first
initialised and then persisted into the save game, as nice and
easy as that would have been.)
This commit also helps to clarify the situation with relocation
tables in SCI1 games that start with a zero entry.
Refs Trac#9780.
Changed paths:
engines/sci/engine/kscripts.cpp
engines/sci/engine/object.cpp
engines/sci/engine/object.h
engines/sci/engine/script.cpp
engines/sci/engine/script.h
engines/sci/engine/vm.cpp
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp
index af4b8ff..75f905e 100644
--- a/engines/sci/engine/kscripts.cpp
+++ b/engines/sci/engine/kscripts.cpp
@@ -246,11 +246,7 @@ reg_t kScriptID(EngineState *s, int argc, reg_t *argv) {
return NULL_REG;
}
- uint32 address = scr->validateExportFunc(index, true);
-
- // Point to the heap for SCI1.1 - SCI2.1 games
- if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE)
- address += scr->getScriptSize();
+ const uint32 address = scr->validateExportFunc(index, true) + scr->getHeapOffset();
// Bugfix for the intro speed in PQ2 version 1.002.011.
// This is taken from the patch by NewRisingSun(NRS) / Belzorash. Global 3
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 386c3e2..640feb1 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -123,12 +123,12 @@ int Object::locateVarSelector(SegManager *segMan, Selector slc) const {
return -1; // Failed
}
-bool Object::relocateSci0Sci21(SegmentId segment, int location, size_t scriptSize) {
- return relocateBlock(_variables, getPos().getOffset(), segment, location, scriptSize);
+bool Object::relocateSci0Sci21(SegmentId segment, int location, uint32 heapOffset) {
+ return relocateBlock(_variables, getPos().getOffset(), segment, location, heapOffset);
}
#ifdef ENABLE_SCI32
-bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize) {
+bool Object::relocateSci3(SegmentId segment, uint32 location, int offset, uint32 scriptSize) {
assert(offset >= 0 && (uint)offset < scriptSize);
for (uint i = 0; i < _variables.size(); ++i) {
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index 71b366d..be20e3b 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -299,9 +299,9 @@ public:
#endif
}
- bool relocateSci0Sci21(SegmentId segment, int location, size_t scriptSize);
+ bool relocateSci0Sci21(SegmentId segment, int location, uint32 heapOffset);
#ifdef ENABLE_SCI32
- bool relocateSci3(SegmentId segment, uint32 location, int offset, size_t scriptSize);
+ bool relocateSci3(SegmentId segment, uint32 location, int offset, uint32 scriptSize);
#endif
int propertyOffsetToId(SegManager *segMan, int propertyOffset) const;
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 2f04534..ea1b6f2 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -183,7 +183,7 @@ void Script::load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptP
_exports = _buf->subspan<const uint16>(kSci11ExportTableOffset, _numExports * sizeof(uint16));
}
- _localsOffset = _script.size() + 4;
+ _localsOffset = getHeapOffset() + 4;
_localsCount = _buf->getUint16SEAt(_localsOffset - 2);
} else if (getSciVersion() == SCI_VERSION_3) {
_localsCount = _buf->getUint16LEAt(12);
@@ -676,10 +676,9 @@ bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId se
error("Attempt to relocate odd variable #%d.5e (relative to %04x)\n", idx, block_location);
return false;
}
- block[idx].setSegment(segment); // Perform relocation
- if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE)
- block[idx].incOffset(scriptSize);
+ block[idx].setSegment(segment);
+ block[idx].incOffset(heapOffset);
return true;
}
@@ -702,60 +701,123 @@ int Script::relocateOffsetSci3(uint32 offset) const {
bool Script::relocateLocal(SegmentId segment, int location) {
if (_localsBlock)
- return relocateBlock(_localsBlock->_locals, _localsOffset, segment, location, _script.size());
+ return relocateBlock(_localsBlock->_locals, _localsOffset, segment, location, getHeapOffset());
else
return false;
}
-void Script::relocateSci0Sci21(reg_t block) {
- SciSpan<const byte> heap = *_buf;
- uint16 heapOffset = 0;
+uint32 Script::getRelocationOffset(const uint32 offset) const {
+ if (getSciVersion() == SCI_VERSION_3) {
+ SciSpan<const byte> relocStart = _buf->subspan(_buf->getUint32SEAt(8));
+ const uint relocCount = _buf->getUint16SEAt(18);
- if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
- heap = _heap;
- heapOffset = _script.size();
+ for (uint i = 0; i < relocCount; ++i) {
+ if (offset == relocStart.getUint32SEAt(0)) {
+ return relocStart.getUint32SEAt(4);
+ }
+ relocStart += 10;
+ }
+ } else {
+ const SciSpan<const uint16> relocTable = getRelocationTableSci0Sci21();
+ for (uint i = 0; i < relocTable.size(); ++i) {
+ if (relocTable.getUint16SEAt(i) == offset) {
+ return getHeapOffset();
+ }
+ }
}
- if (block.getOffset() >= (uint16)heap.size() ||
- heap.getUint16SEAt(block.getOffset()) * 2 + block.getOffset() >= (uint16)heap.size())
- error("Relocation block outside of script");
-
- int count = heap.getUint16SEAt(block.getOffset());
- int exportIndex = 0;
- int pos = 0;
-
- for (int i = 0; i < count; i++) {
- pos = heap.getUint16SEAt(block.getOffset() + 2 + (exportIndex * 2)) + heapOffset;
- // This occurs in SCI01/SCI1 games where usually one export value is
- // zero. It seems that in this situation, we should skip the export and
- // move to the next one, though the total count of valid exports remains
- // the same
- if (!pos) {
- exportIndex++;
- pos = heap.getUint16SEAt(block.getOffset() + 2 + (exportIndex * 2)) + heapOffset;
- if (!pos)
- error("Script::relocate(): Consecutive zero exports found");
+ return kNoRelocation;
+}
+
+const SciSpan<const uint16> Script::getRelocationTableSci0Sci21() const {
+ SciSpan<const byte> relocationBlock;
+ uint16 numEntries;
+ uint16 dataOffset;
+
+ if (getSciVersion() < SCI_VERSION_1_1) {
+ relocationBlock = findBlockSCI0(SCI_OBJ_POINTERS);
+
+ if (!relocationBlock) {
+ return SciSpan<const uint16>();
+ }
+
+ if (relocationBlock != findBlockSCI0(SCI_OBJ_POINTERS, true)) {
+ warning("script.%u has multiple relocation tables", _nr);
}
+ numEntries = relocationBlock.getUint16SEAt(4);
+
+ if (!numEntries) {
+ return SciSpan<const uint16>();
+ }
+
+ dataOffset = 6;
+
+ // Starting somewhere around SQ1, and continuing through the rest of
+ // SCI1, the relocation table in scripts started including an extra
+ // null entry at the beginning of the table, without a corresponding
+ // increase in the entry count. While this change is consistent in
+ // most of the SCI1mid+ games (all scripts in LSL1, Jones CD,
+ // EQ floppy, SQ1, LSL5, and Ms Astro Chicken have the null entry),
+ // a few games include scripts without the null entry (Castle of Dr
+ // Brain 947 & 997, PQ3 997, KQ5 CD 975 & 997). Since 0 is never a
+ // valid relocation offset, we just skip it if we see it
+ const uint16 firstEntry = relocationBlock.getUint16SEAt(6);
+ if (firstEntry == 0) {
+ dataOffset += 2;
+ }
+ } else if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
+ relocationBlock = _heap.subspan(_heap.getUint16SEAt(0));
+
+ if (!relocationBlock) {
+ return SciSpan<const uint16>();
+ }
+
+ numEntries = relocationBlock.getUint16SEAt(0);
+
+ if (!numEntries) {
+ return SciSpan<const uint16>();
+ }
+
+ dataOffset = 2;
+ } else {
+ error("Invalid engine version called Script::getRelocationTableSci0Sci21");
+ }
+
+ // This check should work correctly even with SCI1.1+ because the relocation
+ // table is always at the very end of the heap in these games
+ if (dataOffset + numEntries * sizeof(uint16) != relocationBlock.size()) {
+ warning("script.%u unexpected relocation table size %u", _nr, relocationBlock.size());
+ }
+
+ return relocationBlock.subspan<const uint16>(dataOffset, numEntries * sizeof(uint16));
+}
+
+void Script::relocateSci0Sci21(const SegmentId segmentId) {
+ const SciSpan<const uint16> relocEntries = getRelocationTableSci0Sci21();
+
+ const uint32 heapOffset = getHeapOffset();
+
+ for (uint i = 0; i < relocEntries.size(); ++i) {
+ const uint pos = relocEntries.getUint16SEAt(i) + heapOffset;
+
// In SCI0-SCI1, script local variables, objects and code are relocated.
// We only relocate locals and objects here, and ignore relocation of
// code blocks. In SCI1.1 and newer versions, only locals and objects
// are relocated.
- if (!relocateLocal(block.getSegment(), pos)) {
+ if (!relocateLocal(segmentId, pos)) {
// Not a local? It's probably an object or code block. If it's an
// object, relocate it.
const ObjMap::iterator end = _objects.end();
for (ObjMap::iterator it = _objects.begin(); it != end; ++it)
- if (it->_value.relocateSci0Sci21(block.getSegment(), pos, _script.size()))
+ if (it->_value.relocateSci0Sci21(segmentId, pos, getHeapOffset()))
break;
}
-
- exportIndex++;
}
}
#ifdef ENABLE_SCI32
-void Script::relocateSci3(reg_t block) {
+void Script::relocateSci3(const SegmentId segmentId) {
SciSpan<const byte> relocStart = _buf->subspan(_buf->getUint32SEAt(8));
const uint relocCount = _buf->getUint16SEAt(18);
@@ -763,7 +825,7 @@ void Script::relocateSci3(reg_t block) {
for (it = _objects.begin(); it != _objects.end(); ++it) {
SciSpan<const byte> seeker = relocStart;
for (uint i = 0; i < relocCount; ++i) {
- it->_value.relocateSci3(block.getSegment(),
+ it->_value.relocateSci3(segmentId,
seeker.getUint32SEAt(0),
seeker.getUint32SEAt(4),
_script.size());
@@ -831,7 +893,7 @@ uint32 Script::validateExportFunc(int pubfunct, bool relocSci3) {
return offset;
}
-SciSpan<const byte> Script::findBlockSCI0(ScriptObjectTypes type, bool findLastBlock) {
+SciSpan<const byte> Script::findBlockSCI0(ScriptObjectTypes type, bool findLastBlock) const {
SciSpan<const byte> foundBlock;
bool oldScriptHeader = (getSciVersion() == SCI_VERSION_0_EARLY);
@@ -1054,9 +1116,7 @@ void Script::initializeObjectsSci0(SegManager *segMan, SegmentId segmentId) {
} while ((uint32)(seeker - *_buf) < getScriptSize() - 2);
}
- const SciSpan<const byte> relocationBlock = findBlockSCI0(SCI_OBJ_POINTERS);
- if (relocationBlock)
- relocateSci0Sci21(make_reg(segmentId, relocationBlock - *_buf + 4));
+ relocateSci0Sci21(segmentId);
}
void Script::initializeObjectsSci11(SegManager *segMan, SegmentId segmentId) {
@@ -1108,7 +1168,7 @@ void Script::initializeObjectsSci11(SegManager *segMan, SegmentId segmentId) {
seeker += seeker.getUint16SEAt(2) * 2;
}
- relocateSci0Sci21(make_reg(segmentId, _heap.getUint16SEAt(0)));
+ relocateSci0Sci21(segmentId);
for (uint i = 0; i < mismatchedVarCountObjects.size(); ++i) {
const reg_t pos = mismatchedVarCountObjects[i];
@@ -1143,7 +1203,7 @@ void Script::initializeObjectsSci3(SegManager *segMan, SegmentId segmentId) {
seeker += seeker.getUint16SEAt(2);
}
- relocateSci3(make_reg(segmentId, 0));
+ relocateSci3(segmentId);
}
#endif
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index 65f3ffb..b59f87f 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -56,6 +56,10 @@ enum ScriptOffsetEntryTypes {
SCI_SCR_OFFSET_TYPE_SAID
};
+enum {
+ kNoRelocation = 0xFFFFFFFF
+};
+
struct offsetLookupArrayEntry {
uint16 type; // type of entry
uint16 id; // id of this type, first item inside script data is 1, second item is 2, etc.
@@ -105,6 +109,13 @@ public:
uint32 getScriptSize() const { return _script.size(); }
uint32 getHeapSize() const { return _heap.size(); }
uint32 getBufSize() const { return _buf->size(); }
+ inline uint32 getHeapOffset() const {
+ if (getSciVersion() >= SCI_VERSION_1_1 && getSciVersion() <= SCI_VERSION_2_1_LATE) {
+ return _script.size();
+ }
+
+ return 0;
+ }
const byte *getBuf(uint offset = 0) const { return _buf->getUnsafeDataAt(offset); }
SciSpan<const byte> getSpan(uint offset) const { return _buf->subspan(offset); }
@@ -247,7 +258,7 @@ public:
* Finds the pointer where a block of a specific type starts from,
* in SCI0 - SCI1 games
*/
- SciSpan<const byte> findBlockSCI0(ScriptObjectTypes type, bool findLastBlock = false);
+ SciSpan<const byte> findBlockSCI0(ScriptObjectTypes type, bool findLastBlock = false) const;
/**
* Syncs the string heap of a script. Used when saving/loading.
@@ -276,23 +287,35 @@ public:
uint16 getOffsetStringCount() { return _offsetLookupStringCount; };
uint16 getOffsetSaidCount() { return _offsetLookupSaidCount; };
+ /**
+ * @returns kNoRelocation if no relocation exists for the given offset,
+ * otherwise returns a delta for the offset to its relocated position.
+ */
+ uint32 getRelocationOffset(const uint32 offset) const;
+
private:
/**
+ * Returns a Span containing the relocation table for a SCI0-SCI2.1 script.
+ * (The SCI0-SCI2.1 relocation table is simply a list of all of the
+ * offsets in the script heap whose values should be treated as pointers to
+ * objects (vs just being numbers).)
+ */
+ const SciSpan<const uint16> getRelocationTableSci0Sci21() const;
+
+ /**
* Processes a relocation block within a SCI0-SCI2.1 script
* This function is idempotent, but it must only be called after all
* objects have been instantiated, or a run-time error will occur.
- * @param obj_pos Location (segment, offset) of the block
*/
- void relocateSci0Sci21(reg_t block);
+ void relocateSci0Sci21(const SegmentId segmentId);
#ifdef ENABLE_SCI32
/**
* Processes a relocation block within a SCI3 script
* This function is idempotent, but it must only be called after all
* objects have been instantiated, or a run-time error will occur.
- * @param obj_pos Location (segment, offset) of the block
*/
- void relocateSci3(reg_t block);
+ void relocateSci3(const SegmentId segmentId);
#endif
bool relocateLocal(SegmentId segment, int location);
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 1d86948..f4071a4 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -570,7 +570,7 @@ uint32 findOffset(const int16 relOffset, const Script *scr, const uint32 pcOffse
offset = relOffset;
break;
case SCI_VERSION_1_1:
- offset = relOffset + scr->getScriptSize();
+ offset = relOffset + scr->getHeapOffset();
break;
#ifdef ENABLE_SCI32
case SCI_VERSION_3:
Commit: 49173300385f902ca29d187fd7d2ac6e9eaeea61
https://github.com/scummvm/scummvm/commit/49173300385f902ca29d187fd7d2ac6e9eaeea61
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Find and store the original static names of objects
See code comment in Object::init for more details.
Fixes Trac#9780.
Changed paths:
engines/sci/engine/object.cpp
engines/sci/engine/object.h
engines/sci/engine/script.cpp
engines/sci/engine/seg_manager.cpp
engines/sci/engine/workarounds.cpp
diff --git a/engines/sci/engine/object.cpp b/engines/sci/engine/object.cpp
index 640feb1..b0e9939 100644
--- a/engines/sci/engine/object.cpp
+++ b/engines/sci/engine/object.cpp
@@ -23,6 +23,7 @@
#include "sci/engine/kernel.h"
#include "sci/engine/object.h"
+#include "sci/engine/script.h"
#include "sci/engine/seg_manager.h"
#ifdef ENABLE_SCI32
#include "sci/engine/features.h"
@@ -32,9 +33,9 @@ namespace Sci {
extern bool relocateBlock(Common::Array<reg_t> &block, int block_location, SegmentId segment, int location, uint32 heapOffset);
-
-void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariables) {
- const SciSpan<const byte> data = buf.subspan(obj_pos.getOffset());
+void Object::init(const Script &owner, reg_t obj_pos, bool initVariables) {
+ const SciSpan<const byte> buf = owner.getSpan(0);
+ const SciSpan<const byte> data = owner.getSpan(obj_pos.getOffset());
_baseObj = data;
_pos = obj_pos;
@@ -81,6 +82,35 @@ void Object::init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariab
#endif
}
+ // Some objects, like the unnamed LarryTalker instance in LSL6hires script
+ // 610, and the File class in Torin script 64993, have a `name` property
+ // that is assigned dynamically by game scripts, overriding the static name
+ // value that is normally created by the SC compiler. When this happens, the
+ // value can be set to anything: in LSL6hires it becomes a Str object; in
+ // Torin, it becomes a dynamically allocated string that is disposed before
+ // the corresponding File instance is disposed.
+ // To ensure `SegManager::getObjectName` works consistently and correctly,
+ // without hacks to bypass unexpected/invalid types of dynamic `name` data,
+ // the reg_t pointer to the original static name value for the object is
+ // stored here, ensuring that it is constant and guaranteed to be either a
+ // valid dereferenceable string or NULL_REG.
+ if (getSciVersion() != SCI_VERSION_3) {
+ const uint32 heapOffset = owner.getHeapOffset();
+ const uint32 nameOffset = (obj_pos.getOffset() - heapOffset) + (_offset + 3) * sizeof(uint16);
+ const uint32 relocOffset = owner.getRelocationOffset(nameOffset);
+ if (relocOffset != kNoRelocation) {
+ _name = make_reg(obj_pos.getSegment(), relocOffset + _baseObj.getUint16SEAt((_offset + 3) * sizeof(uint16)));
+ }
+#ifdef ENABLE_SCI32
+ } else if (_propertyOffsetsSci3.size()) {
+ const uint32 nameOffset = _propertyOffsetsSci3[0];
+ const uint32 relocOffset = owner.getRelocationOffset(nameOffset);
+ if (relocOffset != kNoRelocation) {
+ _name = make_reg(obj_pos.getSegment(), relocOffset + buf.getUint16SEAt(nameOffset));
+ }
+#endif
+ }
+
if (initVariables) {
#ifdef ENABLE_SCI32
if (getSciVersion() == SCI_VERSION_3) {
diff --git a/engines/sci/engine/object.h b/engines/sci/engine/object.h
index be20e3b..12387bc 100644
--- a/engines/sci/engine/object.h
+++ b/engines/sci/engine/object.h
@@ -34,6 +34,7 @@
namespace Sci {
class SegManager;
+class Script;
enum infoSelectorFlags {
kInfoFlagClone = 0x0001,
@@ -69,6 +70,7 @@ enum ObjectOffsets {
class Object {
public:
Object() :
+ _name(NULL_REG),
_offset(getSciVersion() < SCI_VERSION_1_1 ? 0 : 5),
_isFreed(false),
_baseObj(),
@@ -81,6 +83,7 @@ public:
{}
Object &operator=(const Object &other) {
+ _name = other._name;
_baseObj = other._baseObj;
_baseMethod = other._baseMethod;
_variables = other._variables;
@@ -181,12 +184,7 @@ public:
#endif
reg_t getNameSelector() const {
-#ifdef ENABLE_SCI32
- if (getSciVersion() == SCI_VERSION_3)
- return _variables.size() ? _variables[0] : NULL_REG;
- else
-#endif
- return _offset + 3 < (uint16)_variables.size() ? _variables[_offset + 3] : NULL_REG;
+ return _name;
}
// No setter for the name selector
@@ -278,7 +276,7 @@ public:
uint getVarCount() const { return _variables.size(); }
- void init(const SciSpan<const byte> &buf, reg_t obj_pos, bool initVariables = true);
+ void init(const Script &owner, reg_t obj_pos, bool initVariables = true);
reg_t getVariable(uint var) const { return _variables[var]; }
reg_t &getVariableRef(uint var) { return _variables[var]; }
@@ -289,6 +287,7 @@ public:
void saveLoadWithSerializer(Common::Serializer &ser);
void cloneFromObject(const Object *obj) {
+ _name = obj ? obj->_name : NULL_REG;
_baseObj = obj ? obj->_baseObj : SciSpan<const byte>();
_baseMethod = obj ? obj->_baseMethod : Common::Array<uint32>();
_baseVars = obj ? obj->_baseVars : Common::Array<uint16>();
@@ -320,6 +319,11 @@ private:
#endif
/**
+ * The name of the object.
+ */
+ reg_t _name;
+
+ /**
* A pointer to the raw object data within the object's owner script.
*/
SciSpan<const byte> _baseObj;
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index ea1b6f2..4932c16 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -656,7 +656,7 @@ Object *Script::scriptObjInit(reg_t obj_pos, bool fullObjectInit) {
// Get the object at the specified position and init it. This will
// automatically "allocate" space for it in the _objects map if necessary.
Object *obj = &_objects[obj_pos.getOffset()];
- obj->init(*_buf, obj_pos, fullObjectInit);
+ obj->init(*this, obj_pos, fullObjectInit);
return obj;
}
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index a941dd3..1683036 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -275,30 +275,10 @@ const char *SegManager::getObjectName(reg_t pos) {
if (nameReg.isNull())
return "<no name>";
- const char *name = nullptr;
-
- if (nameReg.getSegment()) {
-#ifdef ENABLE_SCI32
- // At least Torin script 64000 creates objects with names that are
- // pointed to dynamically generated strings which are freed before the
- // objects themselves are freed. This causes a crash when using
- // `findObjectByName`, since the name of the object is no longer valid
- if (nameReg.getSegment() != _arraysSegId ||
- _heap[_arraysSegId]->isValidOffset(nameReg.getOffset())) {
-#endif
- name = derefString(nameReg);
-#ifdef ENABLE_SCI32
- }
-#endif
- }
+ const char *name = derefString(nameReg);
if (!name) {
- // Crazy Nick Laura Bow is missing some object names needed for the static
- // selector vocabulary
- if (g_sci->getGameId() == GID_CNICK_LAURABOW && pos == make_reg(1, 0x2267))
- return "Character";
- else
- return "<invalid name>";
+ return "<invalid name>";
}
return name;
diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 9301fe6..c471e6a 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -257,7 +257,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
{ GID_CNICK_KQ, -1, 700, 0, "gcWindow", "open", NULL, -1, { WORKAROUND_FAKE, 0 } }, // when entering the control menu, like in hoyle 3
{ GID_CNICK_KQ, 300, 303, 0, "theDoubleCube", "<noname520>", NULL, 5, { WORKAROUND_FAKE, 0 } }, // while playing backgammon with doubling enabled - bug #6426 (same as the theDoubleCube::make workaround for Hoyle 3)
{ GID_CNICK_KQ, 300, 303, 0, "theDoubleCube", "<noname519>", NULL, 9, { WORKAROUND_FAKE, 0 } }, // when accepting a double, while playing backgammon with doubling enabled (same as the theDoubleCube::accept workaround for Hoyle 3)
- { GID_CNICK_LAURABOW, -1, 0, 1, "Character", "say", NULL, -1, { WORKAROUND_FAKE, 0 } }, // Yatch, like in hoyle 3 - temps 504 and 505 - bug #6424
+ { GID_CNICK_LAURABOW,500, 0, 1, "<no name>", "<noname446>", NULL, -1, { WORKAROUND_FAKE, 0 } }, // Yacht, like in hoyle 3 - temps 504 and 505 - bug #6424
{ GID_CNICK_LAURABOW, -1, 700, 0, NULL, "open", NULL, -1, { WORKAROUND_FAKE, 0 } }, // when entering control menu - bug #6423 (same as the gcWindow workaround for Hoyle 3)
{ GID_CNICK_LAURABOW,100, 100, 0, NULL, "<noname144>", NULL, 1, { WORKAROUND_FAKE, 0 } }, // while playing domino - bug #6429 (same as the dominoHand2 workaround for Hoyle 3)
{ GID_CNICK_LAURABOW,100, 110, 0, NULL, "doit", NULL, -1, { WORKAROUND_FAKE, 0 } }, // when changing the "Dominoes per hand" setting - bug #6430
Commit: eba1e883e99b3ee23423a61cb6af927076256344
https://github.com/scummvm/scummvm/commit/eba1e883e99b3ee23423a61cb6af927076256344
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Minor punctuation fix in buggy script alert
Changed paths:
engines/sci/sci.cpp
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 35f35e5..7dab65a 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -371,7 +371,7 @@ Common::Error SciEngine::run() {
"prevent you from progressing later on in the game, during "
"the sequence with the Green Man's riddles. Please, apply "
"the latest patch for this game by Sierra to avoid possible "
- "problems"));
+ "problems."));
}
}
Commit: bc728a1c939b7a753af5885ff42860dde1b0da5d
https://github.com/scummvm/scummvm/commit/bc728a1c939b7a753af5885ff42860dde1b0da5d
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Fix warning about missing base object when loading save games
This situation occurs rarely, but normally, when unreachable but
not yet GC'd objects use a superclass which has already been GC'd.
Thanks to @wjp for looking at this with me and clarifying what
was going on.
Changed paths:
engines/sci/engine/savegame.cpp
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index 8767744..a59b240 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -284,11 +284,52 @@ void SegManager::saveLoadWithSerializer(Common::Serializer &s) {
Object *obj = scr->scriptObjInit(addr, false);
if (pass == 2) {
- if (!obj->initBaseObject(this, addr, false)) {
- // TODO/FIXME: This should not be happening at all. It might indicate a possible issue
- // with the garbage collector. It happens for example in LSL5 (German, perhaps English too).
- warning("Failed to locate base object for object at %04X:%04X; skipping", PRINT_REG(addr));
- objects.erase(addr.toUint16());
+ // When a game disposes a script with kDisposeScript,
+ // the script is marked as deleted and its lockers are
+ // set to 0, which makes the GC stop using the script
+ // as a retainer of its own objects. Most of the time,
+ // this means that the script and all of its objects are
+ // cleaned up on the next GC cycle, but occasionally a
+ // game will retain a reference to an object within a
+ // disposed script somewhere else, which keeps the
+ // script (and all of its objects) alive. This does not
+ // prevent the GC from safely collecting other objects
+ // that had only been retained by now-unreachable script
+ // objects, so references held by these unreachable
+ // objects may be invalidated. If the superclass of one
+ // of these objects is GC'd (because it was the only
+ // retainer of the superclass), and a save game is
+ // created after kDisposeScript is called but before
+ // the script actually becomes collectable, it will
+ // cause the `initBaseObject` call to fail on restore,
+ // but this is fine because the object isn't reachable
+ // anyway (it is just waiting to be GC'd).
+ //
+ // For example, in EcoQuest floppy, after opening the
+ // gate for Delphineus at the beginning of the game,
+ // the game calls to dispose script 380, but there are
+ // still reachable references to the script 380 object
+ // `outsideGateLever` at `CueObj::client` and
+ // `OnMeAndLowY::theObj`, so script 380 (and all of its
+ // objects) are retained. However, the now-unreachable
+ // `fJump` object had been the only retainer of its
+ // superclass `JumpTo`, so the `JumpTo` class gets
+ // GC'd, and the `fJump` object is left with no valid
+ // superclass. If the game is saved and restored at this
+ // point, `initBaseObject` will fail on `fJump` because
+ // it has no superclass (but, again, this is fine
+ // because this is an unreachable object). Later,
+ // `outsideGateLever` becomes unreachable as the
+ // `CueObj::client` and `OnMeAndLowY::theObj` properties
+ // are changed, which means that all script 380 objects
+ // are finally unreachable and the script and its
+ // objects get fully disposed.
+ //
+ // All that said, if a script has lockers and the base
+ // object necessary for restoring the object is still
+ // missing, that is probably a real bug.
+ if (!obj->initBaseObject(this, addr, false) && scr->getLockers()) {
+ warning("Failed to locate base object %04x:%04x for object %04x:%04x (%s); skipping", PRINT_REG(obj->getSpeciesSelector()), PRINT_REG(addr), getObjectName(addr));
}
}
}
Commit: ef69a594672bb1e6549269ca695d6feae3a9c579
https://github.com/scummvm/scummvm/commit/ef69a594672bb1e6549269ca695d6feae3a9c579
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-20T21:14:18-05:00
Commit Message:
SCI: Stop leaking locals segments during script reuse
When a game deletes a script and then loads the same script again
before it has been fully deallocated,
SegManager::instantiateScript tries to reuse the same script
& locals segments, but it was failing to reuse the old locals
segment because Script::freeScript would unconditionally clear
the old locals SegmentId, which meant the old locals segment would
just leak.
This patch does not fix old save games which may contain orphaned
locals segments, but should prevent the problem from occurring
going forward. (It is possible to clean up these old save games,
but this is not a big leak so it doesn't seem worth the extra
effort to do so.)
Changed paths:
engines/sci/engine/script.cpp
engines/sci/engine/script.h
engines/sci/engine/seg_manager.cpp
diff --git a/engines/sci/engine/script.cpp b/engines/sci/engine/script.cpp
index 4932c16..5191c1a 100644
--- a/engines/sci/engine/script.cpp
+++ b/engines/sci/engine/script.cpp
@@ -47,7 +47,7 @@ Script::~Script() {
freeScript();
}
-void Script::freeScript() {
+void Script::freeScript(const bool keepLocalsSegment) {
_nr = 0;
_buf.clear();
@@ -59,7 +59,9 @@ void Script::freeScript() {
_numSynonyms = 0;
_localsOffset = 0;
- _localsSegment = 0;
+ if (!keepLocalsSegment) {
+ _localsSegment = 0;
+ }
_localsBlock = NULL;
_localsCount = 0;
diff --git a/engines/sci/engine/script.h b/engines/sci/engine/script.h
index b59f87f..1befef1 100644
--- a/engines/sci/engine/script.h
+++ b/engines/sci/engine/script.h
@@ -132,7 +132,7 @@ public:
Script();
~Script();
- void freeScript();
+ void freeScript(const bool keepLocalsSegment = false);
void load(int script_nr, ResourceManager *resMan, ScriptPatcher *scriptPatcher);
virtual bool isValidOffset(uint32 offset) const;
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index 1683036..3cf9d08 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -1017,7 +1017,7 @@ int SegManager::instantiateScript(int scriptNum) {
scr->incrementLockers();
return segmentId;
} else {
- scr->freeScript();
+ scr->freeScript(true);
}
} else {
scr = allocateScript(scriptNum, &segmentId);
More information about the Scummvm-git-logs
mailing list