[Scummvm-git-logs] scummvm master -> ada17669dc9bb834511aa14bfac7db1e454d98fd

sluicebox noreply at scummvm.org
Tue Apr 16 01:24:24 UTC 2024


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

Summary:
d907c5d076 SCI: Fix lookupText error handling, documentation
2a66a9d8a7 AGI: Add bounds checking to strings
b04f59b464 AGI: Add Flag Quest version check workaround
ada17669dc AGI: Misc init cleanup


Commit: d907c5d076763ba11276ce1938ff66a8b64ed8c1
    https://github.com/scummvm/scummvm/commit/d907c5d076763ba11276ce1938ff66a8b64ed8c1
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-04-15T18:23:47-07:00

Commit Message:
SCI: Fix lookupText error handling, documentation

SSCI's text lookup function handled invalid offsets by returning an
empty string. We have been treating this as a fatal error.

Zork Demo uses this to loop through texts with kGetFarText to test for
a matching string.

Changed paths:
    engines/sci/engine/kernel.cpp
    engines/sci/engine/kernel.h


diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index e25b0bc8bfc..1d1913a8523 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -934,7 +934,8 @@ Common::String Kernel::lookupText(reg_t address, int index) {
 	if (textlen)
 		return seeker;
 
-	error("Index %d out of bounds in text.%03d", _index, address.getOffset());
+	warning("Index %d out of bounds in text.%03d", _index, address.getOffset());
+	return "";
 }
 
 // TODO: script_adjust_opcode_formats should probably be part of the
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index 522abd5c636..4cef9674018 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -215,8 +215,9 @@ public:
 	 * Looks up text referenced by scripts.
 	 * SCI uses two values to reference to text: An address, and an index. The
 	 * address determines whether the text should be read from a resource file,
-	 * or from the heap, while the index either refers to the number of the
-	 * string in the specified source, or to a relative position inside the text.
+	 * or from the heap. If the text is read from a resource file, then the
+	 * index refers to the index of the string in the text resource.
+	 * If the index does not exist in the resource, an empty string is returned.
 	 *
 	 * @param address The address to look up
 	 * @param index The relative index


Commit: 2a66a9d8a7aebc3218c41f073d2f040030848c24
    https://github.com/scummvm/scummvm/commit/2a66a9d8a7aebc3218c41f073d2f040030848c24
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-04-15T18:23:47-07:00

Commit Message:
AGI: Add bounds checking to strings

Fan games are known to use out of bounds string numbers

Changed paths:
    engines/agi/agi.cpp
    engines/agi/agi.h
    engines/agi/op_cmd.cpp
    engines/agi/op_test.cpp
    engines/agi/saveload.cpp
    engines/agi/text.cpp


diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index e94d9d3b0bd..8dda193a154 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -662,6 +662,25 @@ void AgiEngine::artificialDelayTrigger_DrawPicture(int16 newPictureNr) {
 	_artificialDelayCurrentPicture = newPictureNr;
 }
 
+const char *AgiGame::getString(int number) {
+	if (0 <= number && number <= MAX_STRINGS) {
+		return strings[number];
+	} else {
+		// Occurs in Flag Quest during startup
+		warning("invalid string number: %d", number);
+		return "";
+	}
+}
+
+void AgiGame::setString(int number, const char *str) {
+	if (0 <= number && number <= MAX_STRINGS) {
+		Common::strlcpy(strings[number], str, MAX_STRINGLEN);
+	} else {
+		// Occurs in Groza, number = 150
+		warning("invalid string number: %d, '%s'", number, str);
+	}
+}
+
 void AgiGame::setAppleIIgsSpeedLevel(int i) {
 	appleIIgsSpeedLevel = i;
 	_vm->setVar(VM_VAR_WINDOW_AUTO_CLOSE_TIMER, 6);
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index 95eeb7e48da..acfdf0f16d4 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -474,6 +474,8 @@ struct AgiGame {
 	uint16 appleIIgsSpeedControllerSlot;
 	int appleIIgsSpeedLevel;
 
+	const char *getString(int number);
+	void setString(int number, const char *str);
 	void setAppleIIgsSpeedLevel(int appleIIgsSpeedLevel);
 
 	AgiGame() {
diff --git a/engines/agi/op_cmd.cpp b/engines/agi/op_cmd.cpp
index cf050e79fe3..05bdb0eb9c4 100644
--- a/engines/agi/op_cmd.cpp
+++ b/engines/agi/op_cmd.cpp
@@ -658,7 +658,7 @@ void cmdWordToString(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	uint16 stringNr = parameter[0];
 	uint16 wordNr = parameter[1];
 
-	Common::strlcpy(state->strings[stringNr], vm->_words->getEgoWord(wordNr), MAX_STRINGLEN);
+	state->setString(stringNr, vm->_words->getEgoWord(wordNr));
 }
 
 void cmdOpenDialogue(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -955,7 +955,7 @@ void cmdSetSimple(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 		state->automaticSave = false;
 
 		// Try to get description for automatic saves
-		const char *textPtr = state->strings[stringNr];
+		const char *textPtr = state->getString(stringNr);
 
 		strncpy(state->automaticSaveDescription, textPtr, sizeof(state->automaticSaveDescription));
 		state->automaticSaveDescription[sizeof(state->automaticSaveDescription) - 1] = 0;
@@ -1133,7 +1133,7 @@ void cmdParse(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	vm->setFlag(VM_FLAG_ENTERED_CLI, false);
 	vm->setFlag(VM_FLAG_SAID_ACCEPTED_INPUT, false);
 
-	vm->_words->parseUsingDictionary(vm->_text->stringPrintf(state->strings[stringNr]));
+	vm->_words->parseUsingDictionary(vm->_text->stringPrintf(state->getString(stringNr)));
 }
 
 void cmdCall(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -2051,7 +2051,7 @@ void cmdGetString(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 
 	// copy string to destination
 	// TODO: not sure if set all the time or only when ENTER is pressed
-	Common::strlcpy(&vm->_game.strings[stringDestNr][0], (char *)textMgr->_inputString, MAX_STRINGLEN);
+	vm->_game.setString(stringDestNr, (char *)textMgr->_inputString);
 
 	textMgr->charPos_Pop();
 
@@ -2093,7 +2093,7 @@ void cmdGetNum(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	number = atoi((char *)textMgr->_inputString);
 	vm->setVar(numberDestVarNr, number);
 
-	debugC(4, kDebugLevelScripts, "[%s] -> %d", state->strings[MAX_STRINGS], number);
+	debugC(4, kDebugLevelScripts, "[%s] -> %d", state->getString(MAX_STRINGS), number);
 }
 
 void cmdSetCursorChar(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
@@ -2136,10 +2136,8 @@ void cmdSetKey(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 void cmdSetString(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
 	uint16 stringNr = parameter[0];
 	uint16 textNr = parameter[1] - 1;
-	// CM: to avoid crash in Groza (str = 150)
-	if (stringNr > MAX_STRINGS)
-		return;
-	Common::strlcpy(state->strings[stringNr], state->_curLogic->texts[textNr], MAX_STRINGLEN);
+
+	state->setString(stringNr, state->_curLogic->texts[textNr]);
 }
 
 void cmdDisplay(AgiGame *state, AgiEngine *vm, uint8 *parameter) {
diff --git a/engines/agi/op_test.cpp b/engines/agi/op_test.cpp
index e9765c70e8a..f290f22cb14 100644
--- a/engines/agi/op_test.cpp
+++ b/engines/agi/op_test.cpp
@@ -191,7 +191,7 @@ void condBit(AgiGame *state, AgiEngine *vm, uint8 *p) {
 }
 
 void condCompareStrings(AgiGame *state, AgiEngine *vm, uint8 *p) {
-	debugC(7, kDebugLevelScripts, "comparing [%s], [%s]", state->strings[p[0]], state->strings[p[1]]);
+	debugC(7, kDebugLevelScripts, "comparing [%s], [%s]", state->getString(p[0]), state->getString(p[1]));
 	state->testResult = vm->testCompareStrings(p[0], p[1]);
 }
 
@@ -229,8 +229,8 @@ bool AgiEngine::testCompareStrings(uint8 s1, uint8 s2) {
 	char ms2[MAX_STRINGLEN];
 	int j, k, l;
 
-	Common::strlcpy(ms1, _game.strings[s1], MAX_STRINGLEN);
-	Common::strlcpy(ms2, _game.strings[s2], MAX_STRINGLEN);
+	Common::strlcpy(ms1, _game.getString(s1), MAX_STRINGLEN);
+	Common::strlcpy(ms2, _game.getString(s2), MAX_STRINGLEN);
 
 	l = strlen(ms1);
 	for (k = 0, j = 0; k < l; k++) {
diff --git a/engines/agi/saveload.cpp b/engines/agi/saveload.cpp
index 1e612c834ea..c0989879d24 100644
--- a/engines/agi/saveload.cpp
+++ b/engines/agi/saveload.cpp
@@ -217,7 +217,7 @@ int AgiEngine::saveGame(const Common::String &fileName, const Common::String &de
 
 	// game.ev_keyp
 	for (i = 0; i < MAX_STRINGS; i++)
-		out->write(_game.strings[i], MAX_STRINGLEN);
+		out->write(_game.getString(i), MAX_STRINGLEN);
 
 	// record info about loaded resources
 	for (i = 0; i < MAX_DIRECTORY_ENTRIES; i++) {
diff --git a/engines/agi/text.cpp b/engines/agi/text.cpp
index 7fa5daadc0a..d21ed26b5f6 100644
--- a/engines/agi/text.cpp
+++ b/engines/agi/text.cpp
@@ -731,7 +731,7 @@ void TextMgr::promptKeyPress(uint16 newKey) {
 	if (_messageState.dialogue_Open) {
 		maxChars = TEXT_STRING_MAX_SIZE - 4;
 	} else {
-		maxChars = TEXT_STRING_MAX_SIZE - strlen(_vm->_game.strings[0]); // string 0 is the prompt string prefix
+		maxChars = TEXT_STRING_MAX_SIZE - strlen(_vm->_game.getString(0)); // string 0 is the prompt string prefix
 	}
 
 	if (_promptCursorPos)
@@ -824,8 +824,6 @@ void TextMgr::promptEchoLine() {
 }
 
 void TextMgr::promptRedraw() {
-	char *textPtr = nullptr;
-
 	if (_promptEnabled) {
 		if (_optionCommandPromptWindow) {
 			// Abort, in case command prompt window is active
@@ -837,7 +835,7 @@ void TextMgr::promptRedraw() {
 		charPos_Set(_promptRow, 0);
 		// agi_printf(str_wordwrap(msg, state.string[0], 40) );
 
-		textPtr = _vm->_game.strings[0];
+		const char *textPtr = _vm->_game.getString(0);
 		textPtr = stringPrintf(textPtr);
 		textPtr = stringWordWrap(textPtr, 40);
 
@@ -1245,7 +1243,7 @@ char *TextMgr::stringPrintf(const char *originalText) {
 				break;
 			case 's':
 				i = strtoul(originalText, nullptr, 10);
-				safeStrcat(resultString, stringPrintf(_vm->_game.strings[i]));
+				safeStrcat(resultString, stringPrintf(_vm->_game.getString(i)));
 				break;
 			case 'm':
 				i = strtoul(originalText, nullptr, 10) - 1;


Commit: b04f59b464eb3707feb357ee85a7a1a6d6a4813d
    https://github.com/scummvm/scummvm/commit/b04f59b464eb3707feb357ee85a7a1a6d6a4813d
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-04-15T18:23:47-07:00

Commit Message:
AGI: Add Flag Quest version check workaround

Fixes bug #15060

Changed paths:
    engines/agi/agi.cpp


diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index 8dda193a154..9543eaff9fd 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -666,7 +666,13 @@ const char *AgiGame::getString(int number) {
 	if (0 <= number && number <= MAX_STRINGS) {
 		return strings[number];
 	} else {
-		// Occurs in Flag Quest during startup
+		// WORKAROUND: Flag Quest detects the interpreter version by comparing
+		// out of bounds strings to values know to be in memory in Sierra's
+		// interpreters. The game only starts if a known value matches an
+		// allowed version. We return the value for version 2.917. Bug #15060
+		if (number == 56) {
+			return ".917";
+		}
 		warning("invalid string number: %d", number);
 		return "";
 	}


Commit: ada17669dc9bb834511aa14bfac7db1e454d98fd
    https://github.com/scummvm/scummvm/commit/ada17669dc9bb834511aa14bfac7db1e454d98fd
Author: sluicebox (22204938+sluicebox at users.noreply.github.com)
Date: 2024-04-15T18:23:47-07:00

Commit Message:
AGI: Misc init cleanup

Changed paths:
    engines/agi/agi.cpp
    engines/agi/agi.h


diff --git a/engines/agi/agi.cpp b/engines/agi/agi.cpp
index 9543eaff9fd..6ed402a1f6c 100644
--- a/engines/agi/agi.cpp
+++ b/engines/agi/agi.cpp
@@ -118,8 +118,7 @@ int AgiEngine::agiInit() {
 	// to ask Ego's name again. The name is supposed to be maintained in string 1.
 	// Fixes bug #5673.
 	if (!_restartGame) {
-		for (int i = 0; i < MAX_STRINGS; i++)
-			_game.strings[i][0] = 0;
+		memset(_game.strings, 0, sizeof(_game.strings));
 	}
 
 	// setup emulation
diff --git a/engines/agi/agi.h b/engines/agi/agi.h
index acfdf0f16d4..1f0dfe25b36 100644
--- a/engines/agi/agi.h
+++ b/engines/agi/agi.h
@@ -484,20 +484,11 @@ struct AgiGame {
 		adjMouseX = 0;
 		adjMouseY = 0;
 
-		for (uint16 i = 0; i < ARRAYSIZE(name); i++) {
-			name[i] = 0;
-		}
-		for (uint16 i = 0; i < ARRAYSIZE(id); i++) {
-			id[i] = 0;
-		}
+		memset(name, 0, sizeof(name));
+		memset(id, 0, sizeof(id));
 		crc = 0;
-
-		for (uint16 i = 0; i < ARRAYSIZE(flags); i++) {
-			flags[i] = 0;
-		}
-		for (uint16 i = 0; i < ARRAYSIZE(vars); i++) {
-			vars[i] = 0;
-		}
+		memset(flags, 0, sizeof(flags));
+		memset(vars, 0, sizeof(vars));
 
 		horizon = 0;
 
@@ -518,17 +509,11 @@ struct AgiGame {
 
 		numObjects = 0;
 
-		for (uint16 i = 0; i < ARRAYSIZE(controllerOccurred); i++) {
-            controllerOccurred[i] = false;
-		}
+		memset(controllerOccurred, 0, sizeof(controllerOccurred));
 
 		// controllerKeyMapping defaulted by AgiControllerKeyMapping constructor
 
-		for (uint16 i = 0; i < MAX_STRINGS + 1; i++) {
-			for (uint16 j = 0; j < MAX_STRINGLEN; j++) {
-				strings[i][j] = 0;
-			}
-		}
+		memset(strings, 0, sizeof(strings));
 
 		// dirLogic cleared by AgiDir constructor
 		// dirPic cleared by AgiDir constructor
@@ -538,9 +523,7 @@ struct AgiGame {
 		// pictures cleared by AgiPicture constructor
 		// logics cleared by AgiLogic constructor
 		// views cleared by AgiView constructor
-		for (uint16 i = 0; i < ARRAYSIZE(sounds); i++) {
-			sounds[i] = nullptr;
-		}
+		memset(sounds, 0, sizeof(sounds));
 
 		_curLogic = nullptr;
 
@@ -549,9 +532,7 @@ struct AgiGame {
 		// addToPicView cleared by ScreenObjEntry constructor
 
 		automaticSave = false;
-		for (uint16 i = 0; i < ARRAYSIZE(automaticSaveDescription); i++) {
-			automaticSaveDescription[i] = 0;
-		}
+		memset(automaticSaveDescription, 0, sizeof(automaticSaveDescription));
 
 		// mouseFence cleared by Common::Rect constructor
 		mouseEnabled = false;
@@ -560,9 +541,7 @@ struct AgiGame {
 		testResult = false;
 
 		max_logics = 0;
-		for (uint16 i = 0; i < ARRAYSIZE(logic_list); i++) {
-			logic_list[i] = 0;
-		}
+		memset(logic_list, 0, sizeof(logic_list));
 
 		nonBlockingTextShown = false;
 		nonBlockingTextCyclesLeft = 0;




More information about the Scummvm-git-logs mailing list