[Scummvm-git-logs] scummvm master -> 13925065cb90d3cb2300cbd17935aafd747f68b1

csnover csnover at users.noreply.github.com
Mon May 8 18:27:24 CEST 2017


This automated email contains information about 5 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
554a73e012 SCI: Improve detection and reporting of resource errors
c9cbb8e31c SCI: Translate messages passed to dialogues
130c9ecbb8 SCI: Fix some issues with ChunkResourceSource
f8d4ffa8ed SCI: Fix Audio36 patch suffix matching against lowercase extensions
13925065cb SCI: Fix unnecessary copy of Common::String


Commit: 554a73e01209643161f8adecba825a5bc39f87f8
    https://github.com/scummvm/scummvm/commit/554a73e01209643161f8adecba825a5bc39f87f8
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-08T11:26:46-05:00

Commit Message:
SCI: Improve detection and reporting of resource errors

Simple assertions in the resource manager are not sufficient to
track down resource corruption issues, and some error conditions
that were being checked already were either ignored or only raised
as warnings that casual users would be unlikely to see.

Ideally, error handling in ResourceManager would be improved to
the point where errors would correctly propagate out of it (so the
warning dialogue could be displayed from outside), but right now
error codes are dropped all over the place, and it would take more
effort to fix that without much benefit for the current situation.
If/until someone has the energy to fix that, the warning dialogue
is simply shown from ResourceManager::scanNewSources.

Refs Trac#9764.

Changed paths:
    engines/sci/resource.cpp
    engines/sci/resource.h
    engines/sci/resource_audio.cpp


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 94347ec..9a53627 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -26,6 +26,7 @@
 #include "common/fs.h"
 #include "common/macresman.h"
 #include "common/textconsole.h"
+#include "common/translation.h"
 #ifdef ENABLE_SCI32
 #include "common/memstream.h"
 #endif
@@ -543,6 +544,7 @@ Common::SeekableReadStream *ResourceSource::getVolumeFile(ResourceManager *resMa
 
 	if (!fileStream) {
 		warning("Failed to open %s", getLocationName().c_str());
+		resMan->_hasBadResources = true;
 		if (res)
 			res->unalloc();
 	}
@@ -638,9 +640,13 @@ int ResourceManager::addAppropriateSources() {
 				}
 			}
 
-			// GK2 on Steam comes with an extra bogus resource map file
-			if (!foundVolume) {
+			if (!foundVolume &&
+				// GK2 on Steam comes with an extra bogus resource map file;
+				// ignore it
+				(g_sci->getGameId() != GID_GK2 || mapFiles.size() != 2 || mapNumber != 1)) {
+
 				warning("Could not find corresponding volume for %s", mapName.c_str());
+				_hasBadResources = true;
 			}
 		}
 
@@ -759,7 +765,11 @@ void ResourceManager::addScriptChunkSources() {
 #endif
 }
 
+extern void showScummVMDialog(const Common::String &message);
+
 void ResourceManager::scanNewSources() {
+	_hasBadResources = false;
+
 	for (Common::List<ResourceSource *>::iterator it = _sources.begin(); it != _sources.end(); ++it) {
 		ResourceSource *source = *it;
 
@@ -768,6 +778,18 @@ void ResourceManager::scanNewSources() {
 			source->scanSource(this);
 		}
 	}
+
+	// The warning dialog is shown here instead of someplace more obvious like
+	// SciEngine::run because resource sources can be dynamically added
+	// (e.g. KQ5 via kDoAudio, MGDX via kSetLanguage), and users really should
+	// be warned of bad resources in this situation (KQ Collection 1997 has a
+	// bad copy of KQ5 on CD 1; the working copy is on CD 2)
+	if (_hasBadResources) {
+		showScummVMDialog(_("Missing or corrupt game resources have been detected. "
+							"Some game features may not work properly. Please check "
+							"the console for more information, and verify that your "
+							"game files are valid."));
+	}
 }
 
 void DirectoryResourceSource::scanSource(ResourceManager *resMan) {
@@ -781,18 +803,27 @@ void DirectoryResourceSource::scanSource(ResourceManager *resMan) {
 }
 
 void ExtMapResourceSource::scanSource(ResourceManager *resMan) {
-	if (resMan->_mapVersion < kResVersionSci1Late)
-		resMan->readResourceMapSCI0(this);
-	else
-		resMan->readResourceMapSCI1(this);
+	if (resMan->_mapVersion < kResVersionSci1Late) {
+		if (resMan->readResourceMapSCI0(this) != SCI_ERROR_NONE) {
+			resMan->_hasBadResources = true;
+		}
+	} else {
+		if (resMan->readResourceMapSCI1(this) != SCI_ERROR_NONE) {
+			resMan->_hasBadResources = true;
+		}
+	}
 }
 
 void ExtAudioMapResourceSource::scanSource(ResourceManager *resMan) {
-	resMan->readAudioMapSCI1(this);
+	if (resMan->readAudioMapSCI1(this) != SCI_ERROR_NONE) {
+		resMan->_hasBadResources = true;
+	}
 }
 
 void IntMapResourceSource::scanSource(ResourceManager *resMan) {
-	resMan->readAudioMapSCI11(this);
+	if (resMan->readAudioMapSCI11(this) != SCI_ERROR_NONE) {
+		resMan->_hasBadResources = true;
+	}
 }
 
 #ifdef ENABLE_SCI32
@@ -838,7 +869,7 @@ void ChunkResourceSource::scanSource(ResourceManager *resMan) {
 
 		debugC(kDebugLevelResMan, 2, "Found %s in chunk %d", id.toString().c_str(), _number);
 
-		resMan->updateResource(id, this, entry.length);
+		resMan->updateResource(id, this, entry.length, chunk->_source->getLocationName());
 
 		// There's no end marker to the data table, but the first resource
 		// begins directly after the entry table. So, when we hit the first
@@ -856,10 +887,12 @@ void ChunkResourceSource::loadResource(ResourceManager *resMan, Resource *res) {
 	Resource *chunk = resMan->findResource(ResourceId(kResourceTypeChunk, _number), false);
 
 	if (!_resMap.contains(res->_id))
-		error("Trying to load non-existent resource from chunk %d: %s %d", _number, getResourceTypeName(res->_id.getType()), res->_id.getNumber());
+		error("Trying to load non-existent resource %s from chunk %d", res->_id.toString().c_str(), _number);
 
 	ResourceEntry entry = _resMap[res->_id];
-	assert(entry.offset + entry.length <= chunk->_size);
+	if (entry.offset + entry.length > chunk->size()) {
+		error("Resource %s is too large to exist within chunk %d (%u + %u > %u)", res->_id.toString().c_str(), _number, entry.offset, entry.length, chunk->size());
+	}
 	byte *ptr = new byte[entry.length];
 	res->_data = ptr;
 	res->_size = entry.length;
@@ -1494,10 +1527,8 @@ void ResourceManager::processPatch(ResourceSource *source, ResourceType resource
 	}
 
 	// Overwrite everything, because we're patching
-	newrsc = updateResource(resId, source, fsize - patchDataOffset);
+	newrsc = updateResource(resId, source, 0, fsize - patchDataOffset, source->getLocationName());
 	newrsc->_headerSize = patchDataOffset;
-	newrsc->_fileOffset = 0;
-
 
 	debugC(1, kDebugLevelResMan, "Patching %s - OK", source->getLocationName().c_str());
 }
@@ -1723,10 +1754,18 @@ int ResourceManager::readResourceMapSCI0(ResourceSource *map) {
 					bMask = (_mapVersion == kResVersionSci1Middle) ? 0xF0 : 0xFC;
 					bShift = (_mapVersion == kResVersionSci1Middle) ? 28 : 26;
 					source = findVolume(map, offset >> bShift);
+					if (!source) {
+						delete fileStream;
+						warning("Still couldn't find the volume");
+						return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
+					}
+				} else {
+					delete fileStream;
+					return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
 				}
 			}
 
-			addResource(resId, source, offset & (((~bMask) << 24) | 0xFFFFFF));
+			addResource(resId, source, offset & (((~bMask) << 24) | 0xFFFFFF), 0, map->getLocationName());
 		}
 	} while (!fileStream->eos());
 
@@ -1759,8 +1798,11 @@ int ResourceManager::readResourceMapSCI1(ResourceSource *map) {
 	do {
 		type = fileStream->readByte() & 0x1F;
 		resMap[type].wOffset = fileStream->readUint16LE();
-		if (fileStream->eos())
+		if (fileStream->eos()) {
+			delete fileStream;
+			warning("Premature end of file %s", map->getLocationName().c_str());
 			return SCI_ERROR_RESMAP_NOT_FOUND;
+		}
 
 		resMap[prevtype].wSize = (resMap[type].wOffset
 		                          - resMap[prevtype].wOffset) / nEntrySize;
@@ -1804,11 +1846,15 @@ int ResourceManager::readResourceMapSCI1(ResourceSource *map) {
 			int mapVolumeNr = volume_nr + map->_volumeNumber;
 			ResourceSource *source = findVolume(map, mapVolumeNr);
 
-			assert(source);
+			if (!source) {
+				delete fileStream;
+				warning("Could not get volume for resource %d, VolumeID %d", number, mapVolumeNr);
+				return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
+			}
 
 			Resource *resource = _resMap.getVal(resId, NULL);
 			if (!resource) {
-				addResource(resId, source, fileOffset);
+				addResource(resId, source, fileOffset, 0, map->getLocationName());
 			} else {
 				// If the resource is already present in a volume, change it to
 				// the new content (but only in a volume, so as not to overwrite
@@ -1825,9 +1871,7 @@ int ResourceManager::readResourceMapSCI1(ResourceSource *map) {
 					if (resId.getType() == kResourceTypeMap) {
 						resource->_status = kResStatusNoMalloc;
 					}
-					resource->_source = source;
-					resource->_fileOffset = fileOffset;
-					resource->_size = 0;
+					updateResource(resId, source, fileOffset, 0, map->getLocationName());
 				}
 			}
 
@@ -1839,7 +1883,9 @@ int ResourceManager::readResourceMapSCI1(ResourceSource *map) {
 			// processed immediately, since they will be replaced by the audio
 			// map from the next disc on the next call to readResourceMapSCI1
 			if (_multiDiscAudio && resId.getType() == kResourceTypeMap) {
-				IntMapResourceSource *audioMap = static_cast<IntMapResourceSource *>(addSource(new IntMapResourceSource("MAP", mapVolumeNr, resId.getNumber())));
+				IntMapResourceSource *audioMap = new IntMapResourceSource(source->getLocationName(), mapVolumeNr, resId.getNumber());
+				addSource(audioMap);
+
 				Common::String volumeName;
 				if (resId.getNumber() == 65535) {
 					volumeName = Common::String::format("RESSFX.%03d", mapVolumeNr);
@@ -1948,37 +1994,79 @@ void MacResourceForkResourceSource::scanSource(ResourceManager *resMan) {
 
 			// Overwrite Resource instance. Resource forks may contain patches.
 			// The size will be filled in later by decompressResource()
-			resMan->updateResource(resId, this, 0);
+			resMan->updateResource(resId, this, 0, getLocationName());
+		}
+	}
+}
+
+bool ResourceManager::validateResource(const ResourceId &resourceId, const Common::String &sourceMapLocation, const Common::String &sourceName, const uint32 offset, const uint32 size, const uint32 sourceSize) const {
+	if (size != 0) {
+		if (offset + size > sourceSize) {
+			warning("Resource %s from %s points beyond end of %s (%u + %u > %u)", resourceId.toString().c_str(), sourceMapLocation.c_str(), sourceName.c_str(), offset, size, sourceSize);
+			return false;
+		}
+	} else {
+		if (offset >= sourceSize) {
+			warning("Resource %s from %s points beyond end of %s (%u >= %u)", resourceId.toString().c_str(), sourceMapLocation.c_str(), sourceName.c_str(), offset, sourceSize);
+			return false;
 		}
 	}
+
+	return true;
 }
 
-void ResourceManager::addResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size) {
+void ResourceManager::addResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size, const Common::String &sourceMapLocation) {
 	// Adding new resource only if it does not exist
 	if (_resMap.contains(resId) == false) {
-		Resource *res = new Resource(this, resId);
-		_resMap.setVal(resId, res);
-		res->_source = src;
-		res->_fileOffset = offset;
-		res->_size = size;
+		Common::SeekableReadStream *volumeFile = getVolumeFile(src);
+		if (volumeFile == nullptr) {
+			error("Could not open %s for reading", src->getLocationName().c_str());
+		}
+
+		if (validateResource(resId, sourceMapLocation, src->getLocationName(), offset, size, volumeFile->size())) {
+			Resource *res = new Resource(this, resId);
+			_resMap.setVal(resId, res);
+			res->_source = src;
+			res->_fileOffset = offset;
+			res->_size = size;
+		} else {
+			_hasBadResources = true;
+		}
+	}
+}
+
+Resource *ResourceManager::updateResource(ResourceId resId, ResourceSource *src, uint32 size, const Common::String &sourceMapLocation) {
+	uint32 offset = 0;
+	if (_resMap.contains(resId)) {
+		const Resource *res = _resMap.getVal(resId);
+		offset = res->_fileOffset;
 	}
+	return updateResource(resId, src, offset, size, sourceMapLocation);
 }
 
-Resource *ResourceManager::updateResource(ResourceId resId, ResourceSource *src, uint32 size) {
+Resource *ResourceManager::updateResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size, const Common::String &sourceMapLocation) {
 	// Update a patched resource, whether it exists or not
-	Resource *res = 0;
+	Resource *res = _resMap.getVal(resId, nullptr);
 
-	if (_resMap.contains(resId)) {
-		res = _resMap.getVal(resId);
-	} else {
-		res = new Resource(this, resId);
-		_resMap.setVal(resId, res);
+	Common::SeekableReadStream *volumeFile = getVolumeFile(src);
+	if (volumeFile == nullptr) {
+		error("Could not open %s for reading", src->getLocationName().c_str());
 	}
 
-	res->_status = kResStatusNoMalloc;
-	res->_source = src;
-	res->_headerSize = 0;
-	res->_size = size;
+	if (validateResource(resId, sourceMapLocation, src->getLocationName(), offset, size, volumeFile->size())) {
+		if (res == nullptr) {
+			res = new Resource(this, resId);
+			_resMap.setVal(resId, res);
+		}
+
+		res->_status = kResStatusNoMalloc;
+		res->_source = src;
+		res->_headerSize = 0;
+		res->_fileOffset = offset;
+		res->_size = size;
+	} else {
+		_hasBadResources = true;
+	}
 
 	return res;
 }
@@ -2133,9 +2221,14 @@ int Resource::decompress(ResVersion volVersion, Common::SeekableReadStream *file
 		// instead of using a RESOURCE.SFX
 		if (getType() == kResourceTypeAudio) {
 			const uint8 headerSize = ptr[1];
-			assert(headerSize >= 11);
-			uint32 audioSize = READ_LE_UINT32(ptr + 9);
-			assert(audioSize + headerSize + kResourceHeaderSize == _size);
+			if (headerSize < 11) {
+				error("Unexpected audio header size for %s: should be >= 11, but got %d", _id.toString().c_str(), headerSize);
+			}
+			const uint32 audioSize = READ_LE_UINT32(ptr + 9);
+			const uint32 calculatedTotalSize = audioSize + headerSize + kResourceHeaderSize;
+			if (calculatedTotalSize != _size) {
+				error("Unexpected audio file size: the size of %s in %s is %d, but the volume says it should be %d", _id.toString().c_str(), _source->getLocationName().c_str(), calculatedTotalSize, _size);
+			}
 			_size = headerSize + audioSize;
 		}
 	}
diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index 85bd915..b869c7a 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -519,8 +519,10 @@ protected:
 	Common::SeekableReadStream *getVolumeFile(ResourceSource *source);
 	void loadResource(Resource *res);
 	void freeOldResources();
-	void addResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size = 0);
-	Resource *updateResource(ResourceId resId, ResourceSource *src, uint32 size);
+	bool validateResource(const ResourceId &resourceId, const Common::String &sourceMapLocation, const Common::String &sourceName, const uint32 offset, const uint32 size, const uint32 sourceSize) const;
+	void addResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size = 0, const Common::String &sourceMapLocation = Common::String("(no map location)"));
+	Resource *updateResource(ResourceId resId, ResourceSource *src, uint32 size, const Common::String &sourceMapLocation = Common::String("(no map location)"));
+	Resource *updateResource(ResourceId resId, ResourceSource *src, uint32 offset, uint32 size, const Common::String &sourceMapLocation = Common::String("(no map location)"));
 	void removeAudioResource(ResourceId resId);
 
 	/**--- Resource map decoding functions ---*/
@@ -592,6 +594,9 @@ protected:
 	bool checkResourceDataForSignature(Resource *resource, const byte *signature);
 	bool checkResourceForSignatures(ResourceType resourceType, uint16 resourceNr, const byte *signature1, const byte *signature2);
 	void detectSciVersion();
+
+private:
+	bool _hasBadResources;
 };
 
 class SoundResource {
diff --git a/engines/sci/resource_audio.cpp b/engines/sci/resource_audio.cpp
index 3be7b10..6e93252 100644
--- a/engines/sci/resource_audio.cpp
+++ b/engines/sci/resource_audio.cpp
@@ -116,7 +116,7 @@ bool Resource::loadFromAudioVolumeSCI11(Common::SeekableReadStream *file) {
 
 		if (type == kResourceTypeAudio) {
 			if (headerSize != 7 && headerSize != 11 && headerSize != 12) {
-				warning("Unsupported audio header size %d", headerSize);
+				warning("Unsupported audio header size %d in %s", headerSize, _id.toString().c_str());
 				unalloc();
 				return false;
 			}
@@ -125,7 +125,11 @@ bool Resource::loadFromAudioVolumeSCI11(Common::SeekableReadStream *file) {
 				// Load sample size
 				file->seek(7, SEEK_CUR);
 				_size = file->readUint32LE() + headerSize + kResourceHeaderSize;
-				assert(!file->err() && !file->eos());
+				if (file->err() || file->eos()) {
+					warning("Error while reading size of %s", _id.toString().c_str());
+					unalloc();
+					return false;
+				}
 				// Adjust offset to point at the beginning of the audio file
 				// again
 				file->seek(-11, SEEK_CUR);
@@ -197,7 +201,7 @@ void ResourceManager::processWavePatch(ResourceId resourceId, Common::String nam
 	Common::File file;
 	file.open(name);
 
-	updateResource(resourceId, resSrc, file.size());
+	updateResource(resourceId, resSrc, file.size(), name);
 	_sources.push_back(resSrc);
 
 	debugC(1, kDebugLevelResMan, "Patching %s - OK", name.c_str());
@@ -293,21 +297,29 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 #endif
 
 	uint32 offset = 0;
-	Resource *mapRes = findResource(ResourceId(kResourceTypeMap, map->_mapNumber), false);
+	const ResourceId mapResId(kResourceTypeMap, map->_mapNumber);
+	Resource *mapRes = findResource(mapResId, false);
 
 	if (!mapRes) {
-		warning("Failed to open %i.MAP", map->_mapNumber);
+		warning("Failed to open %s", mapResId.toString().c_str());
 		return SCI_ERROR_RESMAP_NOT_FOUND;
 	}
 
 	ResourceSource *src = findVolume(map, map->_volumeNumber);
 
 	if (!src) {
-		warning("Failed to find volume for %i.MAP", map->_mapNumber);
+		warning("Failed to find volume for %s", mapResId.toString().c_str());
+		return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
+	}
+
+	Common::SeekableReadStream *fileStream = getVolumeFile(src);
+
+	if (!fileStream) {
+		warning("Failed to open file stream for %s", mapResId.toString().c_str());
 		return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
 	}
 
-	const uint32 srcSize = getVolumeFile(src)->size();
+	const uint32 srcSize = fileStream->size();
 
 	SciSpan<const byte>::const_iterator ptr = mapRes->cbegin();
 
@@ -336,8 +348,7 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 				ptr += 3;
 			}
 
-			assert(offset < srcSize);
-			addResource(ResourceId(kResourceTypeAudio, n), src, offset);
+			addResource(ResourceId(kResourceTypeAudio, n), src, offset, 0, map->getLocationName());
 		}
 	} else if (map->_mapNumber == 0 && entrySize == 10 && ptr[3] == 0) {
 		// QFG3 demo format
@@ -354,8 +365,7 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 			uint32 size = ptr.getUint32LE();
 			ptr += 4;
 
-			assert(offset + size <= srcSize);
-			addResource(ResourceId(kResourceTypeAudio, n), src, offset, size);
+			addResource(ResourceId(kResourceTypeAudio, n), src, offset, size, map->getLocationName());
 		}
 	} else if (map->_mapNumber == 0 && entrySize == 8 && (ptr + 2).getUint16LE() == 0xffff) {
 		// LB2 Floppy/Mother Goose SCI1.1 format
@@ -368,6 +378,8 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 			if (n == 0xffff)
 				break;
 
+			const ResourceId audioResId(kResourceTypeAudio, n);
+
 			offset = ptr.getUint32LE();
 			ptr += 4;
 
@@ -375,13 +387,14 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 			// We need to dig into the audio resource in the volume to get the size.
 			stream->seek(offset + 1);
 			byte headerSize = stream->readByte();
-			assert(headerSize == 11 || headerSize == 12);
+			if (headerSize != 11 && headerSize != 12) {
+				error("Unexpected header size in %s: should be 11 or 12, got %d", audioResId.toString().c_str(), headerSize);
+			}
 
 			stream->skip(7);
 			uint32 size = stream->readUint32LE() + headerSize + 2;
 
-			assert(offset + size <= srcSize);
-			addResource(ResourceId(kResourceTypeAudio, n), src, offset, size);
+			addResource(audioResId, src, offset, size, map->getLocationName());
 		}
 	} else {
 		bool isEarly = (entrySize != 11);
@@ -414,8 +427,7 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 				// FIXME: The sync36 resource seems to be two bytes too big in KQ6CD
 				// (bytes taken from the RAVE resource right after it)
 				if (syncSize > 0) {
-					assert(offset + syncSize <= srcSize);
-					addResource(ResourceId(kResourceTypeSync36, map->_mapNumber, n & 0xffffff3f), src, offset, syncSize);
+					addResource(ResourceId(kResourceTypeSync36, map->_mapNumber, n & 0xffffff3f), src, offset, syncSize, map->getLocationName());
 				}
 			}
 
@@ -428,13 +440,12 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 				ptr += 2;
 
 				if (kq6HiresSyncSize > 0) {
-					assert(offset + syncSize + kq6HiresSyncSize <= srcSize);
-					addResource(ResourceId(kResourceTypeRave, map->_mapNumber, n & 0xffffff3f), src, offset + syncSize, kq6HiresSyncSize);
+					addResource(ResourceId(kResourceTypeRave, map->_mapNumber, n & 0xffffff3f), src, offset + syncSize, kq6HiresSyncSize, map->getLocationName());
 					syncSize += kq6HiresSyncSize;
 				}
 			}
 
-			const ResourceId id = ResourceId(kResourceTypeAudio36, map->_mapNumber, n & 0xffffff3f);
+			const ResourceId id(kResourceTypeAudio36, map->_mapNumber, n & 0xffffff3f);
 
 			// Map 405 on CD 1 of the US release of PQ:SWAT 1.000 is broken
 			// and points to garbage in the RESOURCE.AUD. The affected audio36
@@ -454,8 +465,7 @@ int ResourceManager::readAudioMapSCI11(IntMapResourceSource *map) {
 				continue;
 			}
 
-			assert(offset + syncSize < srcSize);
-			addResource(id, src, offset + syncSize);
+			addResource(id, src, offset + syncSize, 0, map->getLocationName());
 		}
 	}
 
@@ -481,9 +491,6 @@ int ResourceManager::readAudioMapSCI1(ResourceSource *map, bool unload) {
 	bool oldFormat = (file.readUint16LE() >> 11) == kResourceTypeAudio;
 	file.seek(0);
 
-	uint32 volumeSize;
-	int lastVolumeNo = -1;
-
 	for (;;) {
 		uint16 n = file.readUint16LE();
 		uint32 offset = file.readUint32LE();
@@ -511,19 +518,15 @@ int ResourceManager::readAudioMapSCI1(ResourceSource *map, bool unload) {
 		ResourceSource *src = findVolume(map, volume_nr);
 
 		if (src) {
-			if (volume_nr != lastVolumeNo) {
-				volumeSize = getVolumeFile(src)->size();
-				lastVolumeNo = volume_nr;
-			}
+			const ResourceId resId(kResourceTypeAudio, n);
 
 			if (unload)
-				removeAudioResource(ResourceId(kResourceTypeAudio, n));
-			else {
-				assert(offset + size <= volumeSize);
-				addResource(ResourceId(kResourceTypeAudio, n), src, offset, size);
-			}
+				removeAudioResource(resId);
+			else
+				addResource(resId, src, offset, size, map->getLocationName());
 		} else {
 			warning("Failed to find audio volume %i", volume_nr);
+			return SCI_ERROR_NO_RESOURCE_FILES_FOUND;
 		}
 	}
 
@@ -538,7 +541,9 @@ void ResourceManager::setAudioLanguage(int language) {
 		}
 
 		// We already have a map loaded, so we unload it first
-		readAudioMapSCI1(_audioMapSCI1, true);
+		if (readAudioMapSCI1(_audioMapSCI1, true) != SCI_ERROR_NONE) {
+			_hasBadResources = true;
+		}
 
 		// Remove all volumes that use this map from the source list
 		Common::List<ResourceSource *>::iterator it = _sources.begin();
@@ -908,7 +913,6 @@ void WaveResourceSource::loadResource(ResourceManager *resMan, Resource *res) {
 	if (!fileStream)
 		return;
 
-	assert(fileStream->size() == -1 || res->_fileOffset < fileStream->size());
 	fileStream->seek(res->_fileOffset, SEEK_SET);
 	res->loadFromWaveFile(fileStream);
 	if (_resourceFile)
@@ -962,7 +966,6 @@ void AudioVolumeResourceSource::loadResource(ResourceManager *resMan, Resource *
 			break;
 		}
 	} else {
-		assert(fileStream->size() == -1 || res->_fileOffset < fileStream->size());
 		// original file, directly seek to given offset and get SCI1/SCI1.1 audio resource
 		fileStream->seek(res->_fileOffset, SEEK_SET);
 	}
@@ -988,7 +991,8 @@ bool ResourceManager::addAudioSources() {
 	Common::List<ResourceId>::iterator itr;
 
 	for (itr = resources.begin(); itr != resources.end(); ++itr) {
-		ResourceSource *src = addSource(new IntMapResourceSource("MAP", 0, itr->getNumber()));
+		const Resource *mapResource = _resMap.getVal(*itr);
+		ResourceSource *src = addSource(new IntMapResourceSource(mapResource->getResourceLocation(), 0, itr->getNumber()));
 
 		if (itr->getNumber() == 65535 && Common::File::exists("RESOURCE.SFX"))
 			addSource(new AudioVolumeResourceSource(this, "RESOURCE.SFX", src, 0));
@@ -1045,12 +1049,7 @@ void ResourceManager::changeAudioDirectory(const Common::String &path) {
 		}
 	}
 
-	Common::String mapName = "MAP";
-	Common::String audioResourceName = "RESOURCE.AUD";
-	if (!path.empty()) {
-		mapName = Common::String::format("%s/MAP", path.c_str());
-		audioResourceName = Common::String::format("%s/RESOURCE.AUD", path.c_str());
-	}
+	const Common::String audioResourceName = (path.empty() ? "" : path + "/") + "RESOURCE.AUD";
 
 	Common::List<ResourceId> resources = listResources(kResourceTypeMap);
 	Common::List<ResourceId>::iterator it;
@@ -1059,7 +1058,8 @@ void ResourceManager::changeAudioDirectory(const Common::String &path) {
 		if (it->getNumber() == 65535)
 			continue;
 
-		ResourceSource *src = addSource(new IntMapResourceSource(mapName, 0, it->getNumber()));
+		const Resource *mapResource = _resMap.getVal(*it);
+		ResourceSource *src = addSource(new IntMapResourceSource(mapResource->getResourceLocation(), 0, it->getNumber()));
 		addSource(new AudioVolumeResourceSource(this, audioResourceName, src, 0));
 	}
 


Commit: c9cbb8e31cba794259905a1c2263d8a3d6d63b9b
    https://github.com/scummvm/scummvm/commit/c9cbb8e31cba794259905a1c2263d8a3d6d63b9b
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-08T11:26:46-05:00

Commit Message:
SCI: Translate messages passed to dialogues

Changed paths:
    engines/sci/engine/kgraphics.cpp
    engines/sci/engine/kgraphics32.cpp
    engines/sci/engine/savegame.cpp
    engines/sci/sci.cpp


diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index ffb9042..9bfeb9a 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -21,6 +21,7 @@
  */
 
 #include "common/system.h"
+#include "common/translation.h"
 
 #include "engines/util.h"
 #include "graphics/cursorman.h"
@@ -70,7 +71,7 @@ static int16 adjustGraphColor(int16 color) {
 }
 
 void showScummVMDialog(const Common::String &message) {
-	GUI::MessageDialog dialog(message, "OK");
+	GUI::MessageDialog dialog(message, _("OK"));
 	dialog.runModal();
 }
 
@@ -981,12 +982,12 @@ reg_t kDrawControl(EngineState *s, int argc, reg_t *argv) {
 		if (!changeDirButton.isNull()) {
 			// check if checkDirButton is still enabled, in that case we are called the first time during that room
 			if (!(readSelectorValue(s->_segMan, changeDirButton, SELECTOR(state)) & SCI_CONTROLS_STYLE_DISABLED)) {
-				showScummVMDialog("Characters saved inside ScummVM are shown "
+				showScummVMDialog(_("Characters saved inside ScummVM are shown "
 						"automatically. Character files saved in the original "
 						"interpreter need to be put inside ScummVM's saved games "
 						"directory and a prefix needs to be added depending on which "
 						"game it was saved in: 'qfg1-' for Quest for Glory 1, 'qfg2-' "
-						"for Quest for Glory 2. Example: 'qfg2-thief.sav'.");
+						"for Quest for Glory 2. Example: 'qfg2-thief.sav'."));
 			}
 		}
 
diff --git a/engines/sci/engine/kgraphics32.cpp b/engines/sci/engine/kgraphics32.cpp
index 38c760f..76ed27a 100644
--- a/engines/sci/engine/kgraphics32.cpp
+++ b/engines/sci/engine/kgraphics32.cpp
@@ -21,6 +21,7 @@
  */
 
 #include "common/system.h"
+#include "common/translation.h"
 
 #include "engines/util.h"
 #include "graphics/cursorman.h"
@@ -347,7 +348,7 @@ reg_t kWinHelp(EngineState *s, int argc, reg_t *argv) {
 	case 1:
 		// Load a help file
 		// Maybe in the future we can implement this, but for now this message should suffice
-		showScummVMDialog("Please use an external viewer to open the game's help file: " + s->_segMan->getString(argv[1]));
+		showScummVMDialog(Common::String::format(_("Please use an external viewer to open the game's help file: %s"), s->_segMan->getString(argv[1]).c_str()));
 		break;
 	case 2:
 		// Looks like some init function
diff --git a/engines/sci/engine/savegame.cpp b/engines/sci/engine/savegame.cpp
index e2099a4..5172de3 100644
--- a/engines/sci/engine/savegame.cpp
+++ b/engines/sci/engine/savegame.cpp
@@ -25,6 +25,7 @@
 #include "common/system.h"
 #include "common/func.h"
 #include "common/serializer.h"
+#include "common/translation.h"
 #include "graphics/thumbnail.h"
 
 #include "sci/sci.h"
@@ -1258,9 +1259,9 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 
 	if ((meta.version < MINIMUM_SAVEGAME_VERSION) || (meta.version > CURRENT_SAVEGAME_VERSION)) {
 		if (meta.version < MINIMUM_SAVEGAME_VERSION) {
-			showScummVMDialog("The format of this saved game is obsolete, unable to load it");
+			showScummVMDialog(_("The format of this saved game is obsolete, unable to load it"));
 		} else {
-			Common::String msg = Common::String::format("Savegame version is %d, maximum supported is %0d", meta.version, CURRENT_SAVEGAME_VERSION);
+			Common::String msg = Common::String::format(_("Savegame version is %d, maximum supported is %0d"), meta.version, CURRENT_SAVEGAME_VERSION);
 			showScummVMDialog(msg);
 		}
 
@@ -1271,7 +1272,7 @@ void gamestate_restore(EngineState *s, Common::SeekableReadStream *fh) {
 	if (meta.gameObjectOffset > 0 && meta.script0Size > 0) {
 		Resource *script0 = g_sci->getResMan()->findResource(ResourceId(kResourceTypeScript, 0), false);
 		if (script0->size() != meta.script0Size || g_sci->getGameObject().getOffset() != meta.gameObjectOffset) {
-			showScummVMDialog("This saved game was created with a different version of the game, unable to load it");
+			showScummVMDialog(_("This saved game was created with a different version of the game, unable to load it"));
 
 			s->r_acc = TRUE_REG;	// signal failure
 			return;
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 9d6e4e0..c518948 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -23,6 +23,7 @@
 #include "common/system.h"
 #include "common/config-manager.h"
 #include "common/debug-channels.h"
+#include "common/translation.h"
 
 #include "engines/advancedDetector.h"
 #include "engines/util.h"
@@ -363,23 +364,23 @@ Common::Error SciEngine::run() {
 		Resource *buggyScript = _resMan->findResource(ResourceId(kResourceTypeScript, 180), 0);
 
 		if (buggyScript && (buggyScript->size() == 12354 || buggyScript->size() == 12362)) {
-			showScummVMDialog("A known buggy game script has been detected, which could "
+			showScummVMDialog(_("A known buggy game script has been detected, which could "
 			                  "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"));
 		}
 	}
 
 	if (getGameId() == GID_KQ7 && ConfMan.getBool("subtitles")) {
-		showScummVMDialog("Subtitles are enabled, but subtitling in King's"
+		showScummVMDialog(_("Subtitles are enabled, but subtitling in King's"
 						  " Quest 7 was unfinished and disabled in the release"
 						  " version of the game. ScummVM allows the subtitles"
 						  " to be re-enabled, but because they were removed from"
 						  " the original game, they do not always render"
 						  " properly or reflect the actual game speech."
 						  " This is not a ScummVM bug -- it is a problem with"
-						  " the game's assets.");
+						  " the game's assets."));
 	}
 
 	// Show a warning if the user has selected a General MIDI device, no GM patch exists
@@ -396,7 +397,7 @@ Common::Error SciEngine::run() {
 			case GID_SQ1:
 			case GID_SQ4:
 			case GID_FAIRYTALES:
-				showScummVMDialog("You have selected General MIDI as a sound device. Sierra "
+				showScummVMDialog(_("You have selected General MIDI as a sound device. Sierra "
 				                  "has provided after-market support for General MIDI for this "
 				                  "game in their \"General MIDI Utility\". Please, apply this "
 				                  "patch in order to enjoy MIDI music with this game. Once you "
@@ -406,7 +407,7 @@ Common::Error SciEngine::run() {
 				                  "the instructions in the READ.ME file included in the patch and "
 				                  "rename the associated *.PAT file to 4.PAT and place it in the "
 				                  "game folder. Without this patch, General MIDI music for this "
-				                  "game will sound badly distorted.");
+				                  "game will sound badly distorted."));
 				break;
 			default:
 				break;
@@ -415,11 +416,11 @@ Common::Error SciEngine::run() {
 	}
 
 	if (gameHasFanMadePatch()) {
-		showScummVMDialog("Your game is patched with a fan made script patch. Such patches have "
+		showScummVMDialog(_("Your game is patched with a fan made script patch. Such patches have "
 		                  "been reported to cause issues, as they modify game scripts extensively. "
 		                  "The issues that these patches fix do not occur in ScummVM, so you are "
 		                  "advised to remove this patch from your game folder in order to avoid "
-		                  "having unexpected errors and/or issues later on.");
+		                  "having unexpected errors and/or issues later on."));
 	}
 
 	runGame();


Commit: 130c9ecbb81f0c078d1cd54f790ee0071d7af752
    https://github.com/scummvm/scummvm/commit/130c9ecbb81f0c078d1cd54f790ee0071d7af752
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-08T11:26:46-05:00

Commit Message:
SCI: Fix some issues with ChunkResourceSource

1. The chunk number was hard-coded to zero and inaccessible.
2. Running ResourceManager::getVolumeFile for a chunk resource
   would always return nullptr instead of a stream of the chunk,
   which made it impossible to generically validate that resources
   being added were within bounds of the container file (or, in
   this case, container chunk).

Changed paths:
    engines/sci/resource.cpp
    engines/sci/resource_intern.h


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 9a53627..0846b93 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -345,6 +345,14 @@ Common::SeekableReadStream *ResourceManager::getVolumeFile(ResourceSource *sourc
 	Common::List<Common::File *>::iterator it = _volumeFiles.begin();
 	Common::File *file;
 
+#ifdef ENABLE_SCI32
+	ChunkResourceSource *chunkSource = dynamic_cast<ChunkResourceSource *>(source);
+	if (chunkSource != nullptr) {
+		Resource *res = findResource(ResourceId(kResourceTypeChunk, chunkSource->getNumber()), false);
+		return res ? res->makeStream() : nullptr;
+	}
+#endif
+
 	if (source->_resourceFile)
 		return source->_resourceFile->createReadStream();
 
@@ -843,7 +851,7 @@ void IntMapResourceSource::scanSource(ResourceManager *resMan) {
 ChunkResourceSource::ChunkResourceSource(const Common::String &name, uint16 number)
 	: ResourceSource(kSourceChunk, name) {
 
-	_number = 0;
+	_number = number;
 }
 
 void ChunkResourceSource::scanSource(ResourceManager *resMan) {
diff --git a/engines/sci/resource_intern.h b/engines/sci/resource_intern.h
index fe4b0a9..f198edd 100644
--- a/engines/sci/resource_intern.h
+++ b/engines/sci/resource_intern.h
@@ -204,6 +204,8 @@ public:
 	virtual void scanSource(ResourceManager *resMan);
 	virtual void loadResource(ResourceManager *resMan, Resource *res);
 
+	uint16 getNumber() const { return _number; }
+
 protected:
 	uint16 _number;
 


Commit: f8d4ffa8ed86e25476956d6d506ca3cccd5864e0
    https://github.com/scummvm/scummvm/commit/f8d4ffa8ed86e25476956d6d506ca3cccd5864e0
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-08T11:26:47-05:00

Commit Message:
SCI: Fix Audio36 patch suffix matching against lowercase extensions

The Lighthouse glider demo comes with a file named SDirectX.dll
which was failing to match the case-sensitive suffix search for
.DLL.

Changed paths:
    engines/sci/resource.cpp


diff --git a/engines/sci/resource.cpp b/engines/sci/resource.cpp
index 0846b93..089b466 100644
--- a/engines/sci/resource.cpp
+++ b/engines/sci/resource.cpp
@@ -1591,6 +1591,7 @@ void ResourceManager::readResourcePatchesBase36() {
 
 		for (Common::ArchiveMemberList::const_iterator x = files.begin(); x != files.end(); ++x) {
 			name = (*x)->getName();
+			name.toUppercase();
 
 			// The S/T prefixes often conflict with non-patch files and generate
 			// spurious warnings about invalid patches


Commit: 13925065cb90d3cb2300cbd17935aafd747f68b1
    https://github.com/scummvm/scummvm/commit/13925065cb90d3cb2300cbd17935aafd747f68b1
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-05-08T11:26:47-05:00

Commit Message:
SCI: Fix unnecessary copy of Common::String

Changed paths:
    engines/sci/resource.h
    engines/sci/resource_audio.cpp


diff --git a/engines/sci/resource.h b/engines/sci/resource.h
index b869c7a..d865130 100644
--- a/engines/sci/resource.h
+++ b/engines/sci/resource.h
@@ -571,7 +571,7 @@ protected:
 	 * Process wave files as patches for Audio resources.
 	 */
 	void readWaveAudioPatches();
-	void processWavePatch(ResourceId resourceId, Common::String name);
+	void processWavePatch(ResourceId resourceId, const Common::String &name);
 
 	/**
 	 * Applies to all versions before 0.000.395 (i.e. KQ4 old, XMAS 1988 and LSL2).
diff --git a/engines/sci/resource_audio.cpp b/engines/sci/resource_audio.cpp
index 6e93252..a5501cd 100644
--- a/engines/sci/resource_audio.cpp
+++ b/engines/sci/resource_audio.cpp
@@ -196,7 +196,7 @@ void ResourceManager::addNewGMPatch(SciGameId gameId) {
 	}
 }
 
-void ResourceManager::processWavePatch(ResourceId resourceId, Common::String name) {
+void ResourceManager::processWavePatch(ResourceId resourceId, const Common::String &name) {
 	ResourceSource *resSrc = new WaveResourceSource(name);
 	Common::File file;
 	file.open(name);





More information about the Scummvm-git-logs mailing list