diff options
author | Colin Snover | 2017-06-07 20:59:34 -0500 |
---|---|---|
committer | Colin Snover | 2017-06-09 23:00:14 -0500 |
commit | 85e35943fe27b99a91d97eace3072117c2073f69 (patch) | |
tree | b298ba66c3cdafde9d48f9d8b6690fb19cec3eef /engines/sci/engine/kscripts.cpp | |
parent | 4311d1b18274d25384c5492c27f0611db28d6c4a (diff) | |
download | scummvm-rg350-85e35943fe27b99a91d97eace3072117c2073f69.tar.gz scummvm-rg350-85e35943fe27b99a91d97eace3072117c2073f69.tar.bz2 scummvm-rg350-85e35943fe27b99a91d97eace3072117c2073f69.zip |
SCI32: Implement kLock & kDoAudio(1) for SCI32
1. Unlocking all resources of a type using a resource ID of -1 is
gone in SCI32;
2. Audio locks need to be serialized starting in GK2 for the game's
modified kDoAudio(1) call;
3. Audio locks in SCI3 must work more like SSCI, since at least
Lighthouse's `BackMusic::fade` method will attempt to unlock
audio that was never locked by a script. In SSCI (and now in
ScummVM too) this is a no-op; previously in ScummVM, it would
remove Audio32's own lock on the audio resource, resulting in a
use-after-free;
4. kDoAudio(1) starting in GK2 returns the number of active
*not-in-memory* channels being played, not the total number of
active channels.
Fixes Trac#9675.
Diffstat (limited to 'engines/sci/engine/kscripts.cpp')
-rw-r--r-- | engines/sci/engine/kscripts.cpp | 51 |
1 files changed, 40 insertions, 11 deletions
diff --git a/engines/sci/engine/kscripts.cpp b/engines/sci/engine/kscripts.cpp index 7bcbd0e359..5a330237e9 100644 --- a/engines/sci/engine/kscripts.cpp +++ b/engines/sci/engine/kscripts.cpp @@ -27,6 +27,10 @@ #include "sci/engine/state.h" #include "sci/engine/selector.h" #include "sci/engine/kernel.h" +#ifdef ENABLE_SCI32 +#include "sci/engine/features.h" +#include "sci/sound/audio32.h" +#endif #include "common/file.h" @@ -49,6 +53,11 @@ reg_t kLoad(EngineState *s, int argc, reg_t *argv) { // behavior of this call didn't change between sci0->sci1.1 parameter wise, which means getting called with // 1 or 3+ parameters is not right according to sierra sci reg_t kUnLoad(EngineState *s, int argc, reg_t *argv) { + // NOTE: Locked resources in SSCI could be disposed by kUnLoad regardless + // of lock state. With this ScummVM implementation of kUnLoad, game scripts + // that dispose locked resources via kUnLoad without unlocking them with + // kLock will leak the resource until the engine is restarted. + ResourceType restype = g_sci->getResMan()->convertResType(argv[0].toUint16()); reg_t resnr = argv[1]; @@ -59,27 +68,48 @@ reg_t kUnLoad(EngineState *s, int argc, reg_t *argv) { } reg_t kLock(EngineState *s, int argc, reg_t *argv) { - int state = argc > 2 ? argv[2].toUint16() : 1; + // NOTE: In SSCI, kLock uses a boolean lock flag, not a lock counter. + // ScummVM's current counter-based implementation should be better than SSCI + // at dealing with game scripts that unintentionally lock & unlock the same + // resource multiple times (e.g. through recursion), but it will introduce + // memory bugs (resource leaks lasting until the engine is restarted, or + // destruction of kernel locks that lead to a use-after-free) that are + // masked by ResourceManager's LRU cache if scripts rely on kLock being + // idempotent like it was in SSCI. + // + // Like SSCI, resource locks are not persisted in save games in ScummVM + // until GK2, so it is also possible that kLock bugs will appear only after + // restoring a save game. + // + // See also kUnLoad. + ResourceType type = g_sci->getResMan()->convertResType(argv[0].toUint16()); if (type == kResourceTypeSound && getSciVersion() >= SCI_VERSION_1_1) { type = g_sci->_soundCmd->getSoundResourceType(argv[1].toUint16()); } const ResourceId id(type, argv[1].toUint16()); + const bool lock = argc > 2 ? argv[2].toUint16() : true; + +#ifdef ENABLE_SCI32 + // SSCI GK2+SCI3 also saves lock states for View, Pic, and Sync resources, + // but so far it seems like audio resources are the only ones that actually + // need to be handled + if (g_sci->_features->hasSci3Audio() && type == kResourceTypeAudio) { + g_sci->_audio32->lockResource(id, lock); + return s->r_acc; + } +#endif if (getSciVersion() == SCI_VERSION_1_1 && (type == kResourceTypeAudio36 || type == kResourceTypeSync36)) { return s->r_acc; } - Resource *which; - - switch (state) { - case 1 : - g_sci->getResMan()->findResource(id, 1); - break; - case 0 : - if (id.getNumber() == 0xFFFF) { + if (lock) { + g_sci->getResMan()->findResource(id, true); + } else { + if (getSciVersion() < SCI_VERSION_2 && id.getNumber() == 0xFFFF) { // Unlock all resources of the requested type Common::List<ResourceId> resources = g_sci->getResMan()->listResources(type); Common::List<ResourceId>::iterator itr; @@ -89,7 +119,7 @@ reg_t kLock(EngineState *s, int argc, reg_t *argv) { g_sci->getResMan()->unlockResource(res); } } else { - which = g_sci->getResMan()->findResource(id, 0); + Resource *which = g_sci->getResMan()->findResource(id, false); if (which) g_sci->getResMan()->unlockResource(which); @@ -103,7 +133,6 @@ reg_t kLock(EngineState *s, int argc, reg_t *argv) { debugC(kDebugLevelResMan, "[resMan] Attempt to unlock non-existent resource %s", id.toString().c_str()); } } - break; } return s->r_acc; } |