[Scummvm-git-logs] scummvm master -> 30038fe4263e9943102e4b7d5ccae2d3ee71dcae

criezy noreply at scummvm.org
Sun Jun 19 19:50:38 UTC 2022


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

Summary:
884048108d AGS: Replaced freadstring/fwritestring with safer StrUtil functions
2c7d51ce16 AGS: Replaced EnginePlugin's filename from char array to String
8d36266724 AGS: Replaced strcpy with snprintf in old string API impls
e90fb00412 AGS: Replaced more strcpy with snprintf, for safety
9e940e4360 AGS: Put more unused code under OBSOLETE
b49d3b2e9f AGS: In unload_game_file() don't error if quit during the of dialog
10033d195e AGS: Store camera & viewport handles, not script ptrs, for safety
30038fe426 AGS: Store certain overlay handles, not script ptrs, for safety


Commit: 884048108dfa08e0c5b94c587d83eb6edcd2ce7e
    https://github.com/scummvm/scummvm/commit/884048108dfa08e0c5b94c587d83eb6edcd2ce7e
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Replaced freadstring/fwritestring with safer StrUtil functions

>From upstream 73af6bafd2e590956b7a248cf3615c3b6df8dcd2

Changed paths:
    engines/ags/shared/script/cc_script.cpp
    engines/ags/shared/util/string_utils.cpp
    engines/ags/shared/util/string_utils.h


diff --git a/engines/ags/shared/script/cc_script.cpp b/engines/ags/shared/script/cc_script.cpp
index 3536300e1e2..5df52ab3fe5 100644
--- a/engines/ags/shared/script/cc_script.cpp
+++ b/engines/ags/shared/script/cc_script.cpp
@@ -24,36 +24,12 @@
 #include "ags/shared/script/cc_internal.h"
 #include "ags/shared/util/stream.h"
 #include "ags/shared/util/string_compat.h"
+#include "ags/shared/util/string_utils.h"
 #include "ags/globals.h"
 
 namespace AGS3 {
 
-using AGS::Shared::Stream;
-
-// [IKM] I reckon this function is almost identical to fgetstring in string_utils
-void freadstring(char **strptr, Stream *in) {
-	static char ibuffer[300];
-	int idxx = 0;
-
-	while ((ibuffer[idxx] = in->ReadInt8()) != 0)
-		idxx++;
-
-	if (ibuffer[0] == 0) {
-		strptr[0] = nullptr;
-		return;
-	}
-
-	strptr[0] = (char *)malloc(strlen(ibuffer) + 1);
-	strcpy(strptr[0], ibuffer);
-}
-
-void fwritestring(const char *strptr, Stream *out) {
-	if (strptr == nullptr) {
-		out->WriteByte(0);
-	} else {
-		out->Write(strptr, strlen(strptr) + 1);
-	}
-}
+using namespace AGS::Shared;
 
 ccScript *ccScript::CreateFromStream(Stream *in) {
 	ccScript *scri = new ccScript();
@@ -190,15 +166,15 @@ void ccScript::Write(Stream *out) {
 	}
 	out->WriteInt32(numimports);
 	for (n = 0; n < numimports; n++)
-		fwritestring(imports[n], out);
+		StrUtil::WriteCStr(imports[n], out);
 	out->WriteInt32(numexports);
 	for (n = 0; n < numexports; n++) {
-		fwritestring(exports[n], out);
+		StrUtil::WriteCStr(exports[n], out);
 		out->WriteInt32(export_addr[n]);
 	}
 	out->WriteInt32(numSections);
 	for (n = 0; n < numSections; n++) {
-		fwritestring(sectionNames[n], out);
+		StrUtil::WriteCStr(sectionNames[n], out);
 		out->WriteInt32(sectionOffsets[n]);
 	}
 	out->WriteInt32(ENDFILESIG);
@@ -256,13 +232,13 @@ bool ccScript::Read(Stream *in) {
 
 	imports = (char **)malloc(sizeof(char *) * numimports);
 	for (n = 0; n < numimports; n++)
-		freadstring(&imports[n], in);
+		imports[n] = StrUtil::ReadMallocCStrOrNull(in);
 
 	numexports = in->ReadInt32();
 	exports = (char **)malloc(sizeof(char *) * numexports);
 	export_addr = (int32_t *)malloc(sizeof(int32_t) * numexports);
 	for (n = 0; n < numexports; n++) {
-		freadstring(&exports[n], in);
+		exports[n] = StrUtil::ReadMallocCStrOrNull(in);
 		export_addr[n] = in->ReadInt32();
 	}
 
@@ -272,7 +248,7 @@ bool ccScript::Read(Stream *in) {
 		sectionNames = (char **)malloc(numSections * sizeof(char *));
 		sectionOffsets = (int32_t *)malloc(numSections * sizeof(int32_t));
 		for (n = 0; n < numSections; n++) {
-			freadstring(&sectionNames[n], in);
+			sectionNames[n] = StrUtil::ReadMallocCStrOrNull(in);
 			sectionOffsets[n] = in->ReadInt32();
 		}
 	} else {
diff --git a/engines/ags/shared/util/string_utils.cpp b/engines/ags/shared/util/string_utils.cpp
index f5bf7773922..4ab22b15855 100644
--- a/engines/ags/shared/util/string_utils.cpp
+++ b/engines/ags/shared/util/string_utils.cpp
@@ -25,6 +25,7 @@
 #include "ags/lib/std/regex.h"
 #include "ags/shared/util/math.h"
 #include "ags/shared/util/stream.h"
+#include "ags/shared/util/string_compat.h"
 #include "ags/globals.h"
 
 namespace AGS3 {
@@ -199,13 +200,28 @@ void StrUtil::ReadCStr(char *buf, Stream *in, size_t buf_limit) {
 	}
 }
 
+char *StrUtil::ReadMallocCStrOrNull(Stream *in) {
+	char buf[1024];
+	for (auto ptr = buf; (ptr < buf + sizeof(buf)); ++ptr) {
+		auto ichar = in->ReadByte();
+		if (ichar <= 0) {
+			*ptr = 0;
+			break;
+		}
+		*ptr = static_cast<char>(ichar);
+	}
+	return buf[0] != 0 ? ags_strdup(buf) : nullptr;
+}
+
 void StrUtil::SkipCStr(Stream *in) {
 	while (in->ReadByte() > 0);
 }
 
 void StrUtil::WriteCStr(const char *cstr, Stream *out) {
-	size_t len = strlen(cstr);
-	out->Write(cstr, len + 1);
+	if (cstr)
+		out->Write(cstr, strlen(cstr) + 1);
+	else
+		out->WriteByte(0);
 }
 
 void StrUtil::WriteCStr(const String &s, Stream *out) {
diff --git a/engines/ags/shared/util/string_utils.h b/engines/ags/shared/util/string_utils.h
index fb9ac1d144f..298a6f0490f 100644
--- a/engines/ags/shared/util/string_utils.h
+++ b/engines/ags/shared/util/string_utils.h
@@ -78,6 +78,11 @@ void            WriteString(const char *cstr, size_t len, Stream *out);
 
 // Serialize and unserialize string as c-string (null-terminated sequence)
 void            ReadCStr(char *buf, Stream *in, size_t buf_limit);
+// Reads a null-terminated string and !! mallocs !! a char buffer for it;
+// returns nullptr if the read string is empty.
+// Buffer is hard-limited to 1024 bytes, including null-terminator.
+// Strictly for compatibility with the C lib code!
+char *          ReadMallocCStrOrNull(Stream *in);
 void            SkipCStr(Stream *in);
 void            WriteCStr(const char *cstr, Stream *out);
 void            WriteCStr(const String &s, Stream *out);


Commit: 2c7d51ce167bfa65594d8fad638dca45e5729281
    https://github.com/scummvm/scummvm/commit/2c7d51ce167bfa65594d8fad638dca45e5729281
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Replaced EnginePlugin's filename from char array to String

>From upstream 252d9d198ac665546bd7ed3f3b8afa018bd62b9f

Changed paths:
    engines/ags/plugins/ags_plugin.cpp
    engines/ags/plugins/ags_plugin.h


diff --git a/engines/ags/plugins/ags_plugin.cpp b/engines/ags/plugins/ags_plugin.cpp
index fb1d423c91b..53727609a0e 100644
--- a/engines/ags/plugins/ags_plugin.cpp
+++ b/engines/ags/plugins/ags_plugin.cpp
@@ -846,7 +846,7 @@ Engine::GameInitError pl_register_plugins(const std::vector<Shared::PluginInfo>
 		EnginePlugin *apl = &_GP(plugins).back();
 
 		// Copy plugin info
-		snprintf(apl->filename, sizeof(apl->filename), "%s", name.GetCStr());
+		apl->filename = name;
 		if (info.DataLen) {
 			apl->savedata = (char *)malloc(info.DataLen);
 			memcpy(apl->savedata, info.Data.get(), info.DataLen);
@@ -854,18 +854,18 @@ Engine::GameInitError pl_register_plugins(const std::vector<Shared::PluginInfo>
 		apl->savedatasize = info.DataLen;
 
 		// Compatibility with the old SnowRain module
-		if (ags_stricmp(apl->filename, "ags_SnowRain20") == 0) {
-			strcpy(apl->filename, "ags_snowrain");
+		if (apl->filename.CompareNoCase("ags_SnowRain20") == 0) {
+			apl->filename = "ags_snowrain";
 		}
 
 		String expect_filename = apl->library.GetFilenameForLib(apl->filename);
 		if (apl->library.Load(apl->filename)) {
 			apl->_plugin = apl->library.getPlugin();
-			AGS::Shared::Debug::Printf(kDbgMsg_Info, "Plugin '%s' loaded as '%s', resolving imports...", apl->filename, expect_filename.GetCStr());
+			AGS::Shared::Debug::Printf(kDbgMsg_Info, "Plugin '%s' loaded as '%s', resolving imports...", apl->filename.GetCStr(), expect_filename.GetCStr());
 
 		} else {
 			AGS::Shared::Debug::Printf(kDbgMsg_Info, "Plugin '%s' could not be loaded (expected '%s')",
-			                           apl->filename, expect_filename.GetCStr());
+			                           apl->filename.GetCStr(), expect_filename.GetCStr());
 			_GP(plugins).pop_back();
 			continue;
 		}
diff --git a/engines/ags/plugins/ags_plugin.h b/engines/ags/plugins/ags_plugin.h
index c00e8ec64de..752e8fec427 100644
--- a/engines/ags/plugins/ags_plugin.h
+++ b/engines/ags/plugins/ags_plugin.h
@@ -33,6 +33,7 @@
 #include "common/array.h"
 #include "ags/shared/core/types.h"
 #include "ags/shared/font/ags_font_renderer.h"
+#include "ags/shared/util/string.h"
 #include "ags/plugins/plugin_base.h"
 #include "ags/engine/util/library_scummvm.h"
 
@@ -564,8 +565,8 @@ public:
 };
 
 struct EnginePlugin {
-	char filename[PLUGIN_FILENAME_MAX + 1] = { 0 };
-	AGS::Engine::Library   library;
+	AGS::Shared::String  filename;
+	AGS::Engine::Library library;
 	Plugins::PluginBase *_plugin = nullptr;
 	bool available = false;
 	char *savedata = nullptr;


Commit: 8d3626672486635d659e40409ca5ae60a572e78c
    https://github.com/scummvm/scummvm/commit/8d3626672486635d659e40409ca5ae60a572e78c
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Replaced strcpy with snprintf in old string API impls

>From upstream 44d99badb2708fb99b1f7b9bc024082d570706c7

Changed paths:
    engines/ags/engine/ac/button.cpp
    engines/ags/engine/ac/global_game.cpp
    engines/ags/engine/ac/global_hotspot.cpp
    engines/ags/engine/ac/global_inventory_item.cpp
    engines/ags/engine/ac/global_object.cpp
    engines/ags/engine/ac/global_parser.cpp
    engines/ags/engine/ac/global_translation.cpp
    engines/ags/engine/ac/label.cpp
    engines/ags/engine/ac/properties.cpp
    engines/ags/engine/ac/textbox.cpp


diff --git a/engines/ags/engine/ac/button.cpp b/engines/ags/engine/ac/button.cpp
index befc3ecb0e3..438b38025e2 100644
--- a/engines/ags/engine/ac/button.cpp
+++ b/engines/ags/engine/ac/button.cpp
@@ -127,7 +127,7 @@ const char *Button_GetText_New(GUIButton *butt) {
 }
 
 void Button_GetText(GUIButton *butt, char *buffer) {
-	strcpy(buffer, butt->GetText().GetCStr());
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", butt->GetText().GetCStr());
 }
 
 void Button_SetText(GUIButton *butt, const char *newtx) {
diff --git a/engines/ags/engine/ac/global_game.cpp b/engines/ags/engine/ac/global_game.cpp
index 828954c4076..b9c7ecfcdf4 100644
--- a/engines/ags/engine/ac/global_game.cpp
+++ b/engines/ags/engine/ac/global_game.cpp
@@ -548,7 +548,7 @@ void GetLocationName(int xxx, int yyy, char *tempo) {
 			if (_GP(play).get_loc_name_last_time != 1000 + mover)
 				GUI::MarkSpecialLabelsForUpdate(kLabelMacro_Overhotspot);
 			_GP(play).get_loc_name_last_time = 1000 + mover;
-			strcpy(tempo, get_translation(_GP(game).invinfo[mover].name));
+			snprintf(tempo, MAX_MAXSTRLEN, "%s", get_translation(_GP(game).invinfo[mover].name));
 		} else if ((_GP(play).get_loc_name_last_time > 1000) && (_GP(play).get_loc_name_last_time < 1000 + MAX_INV)) {
 			// no longer selecting an item
 			GUI::MarkSpecialLabelsForUpdate(kLabelMacro_Overhotspot);
@@ -578,7 +578,7 @@ void GetLocationName(int xxx, int yyy, char *tempo) {
 	// on character
 	if (loctype == LOCTYPE_CHAR) {
 		onhs = _G(getloctype_index);
-		strcpy(tempo, get_translation(_GP(game).chars[onhs].name));
+		snprintf(tempo, MAX_MAXSTRLEN, "%s", get_translation(_GP(game).chars[onhs].name));
 		if (_GP(play).get_loc_name_last_time != 2000 + onhs)
 			GUI::MarkSpecialLabelsForUpdate(kLabelMacro_Overhotspot);
 		_GP(play).get_loc_name_last_time = 2000 + onhs;
@@ -587,7 +587,7 @@ void GetLocationName(int xxx, int yyy, char *tempo) {
 	// on object
 	if (loctype == LOCTYPE_OBJ) {
 		aa = _G(getloctype_index);
-		strcpy(tempo, get_translation(_G(croom)->obj[aa].name.GetCStr()));
+		snprintf(tempo, MAX_MAXSTRLEN, "%s", get_translation(_G(croom)->obj[aa].name.GetCStr()));
 		// Compatibility: < 3.1.1 games returned space for nameless object
 		// (presumably was a bug, but fixing it affected certain games behavior)
 		if (_G(loaded_game_file_version) < kGameVersion_311 && tempo[0] == 0) {
@@ -600,7 +600,8 @@ void GetLocationName(int xxx, int yyy, char *tempo) {
 		return;
 	}
 	onhs = _G(getloctype_index);
-	if (onhs > 0) strcpy(tempo, get_translation(_G(croom)->hotspot[onhs].Name.GetCStr()));
+	if (onhs > 0)
+		snprintf(tempo, MAX_MAXSTRLEN, "%s", get_translation(_G(croom)->hotspot[onhs].Name.GetCStr()));
 	if (_GP(play).get_loc_name_last_time != onhs)
 		GUI::MarkSpecialLabelsForUpdate(kLabelMacro_Overhotspot);
 	_GP(play).get_loc_name_last_time = onhs;
diff --git a/engines/ags/engine/ac/global_hotspot.cpp b/engines/ags/engine/ac/global_hotspot.cpp
index 5a291fefaed..fc9392a3446 100644
--- a/engines/ags/engine/ac/global_hotspot.cpp
+++ b/engines/ags/engine/ac/global_hotspot.cpp
@@ -86,7 +86,7 @@ void GetHotspotName(int hotspot, char *buffer) {
 	if ((hotspot < 0) || (hotspot >= MAX_ROOM_HOTSPOTS))
 		quit("!GetHotspotName: invalid hotspot number");
 
-	strcpy(buffer, get_translation(_G(croom)->hotspot[hotspot].Name.GetCStr()));
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", get_translation(_G(croom)->hotspot[hotspot].Name.GetCStr()));
 }
 
 void RunHotspotInteraction(int hotspothere, int mood) {
diff --git a/engines/ags/engine/ac/global_inventory_item.cpp b/engines/ags/engine/ac/global_inventory_item.cpp
index 1e5037f2a9a..c9c707606db 100644
--- a/engines/ags/engine/ac/global_inventory_item.cpp
+++ b/engines/ags/engine/ac/global_inventory_item.cpp
@@ -85,7 +85,7 @@ int GetInvAt(int atx, int aty) {
 void GetInvName(int indx, char *buff) {
 	VALIDATE_STRING(buff);
 	if ((indx < 0) | (indx >= _GP(game).numinvitems)) quit("!GetInvName: invalid inventory item specified");
-	strcpy(buff, get_translation(_GP(game).invinfo[indx].name));
+	snprintf(buff, MAX_MAXSTRLEN, "%s", get_translation(_GP(game).invinfo[indx].name));
 }
 
 int GetInvGraphic(int indx) {
diff --git a/engines/ags/engine/ac/global_object.cpp b/engines/ags/engine/ac/global_object.cpp
index bd9e0457427..0aad35ab0b8 100644
--- a/engines/ags/engine/ac/global_object.cpp
+++ b/engines/ags/engine/ac/global_object.cpp
@@ -386,7 +386,7 @@ void GetObjectName(int obj, char *buffer) {
 	if (!is_valid_object(obj))
 		quit("!GetObjectName: invalid object number");
 
-	strcpy(buffer, get_translation(_G(croom)->obj[obj].name.GetCStr()));
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", get_translation(_G(croom)->obj[obj].name.GetCStr()));
 }
 
 void MoveObject(int objj, int xx, int yy, int spp) {
diff --git a/engines/ags/engine/ac/global_parser.cpp b/engines/ags/engine/ac/global_parser.cpp
index e2965e5bdc4..57a5cb2deb1 100644
--- a/engines/ags/engine/ac/global_parser.cpp
+++ b/engines/ags/engine/ac/global_parser.cpp
@@ -29,7 +29,7 @@ namespace AGS3 {
 
 int SaidUnknownWord(char *buffer) {
 	VALIDATE_STRING(buffer);
-	strcpy(buffer, _GP(play).bad_parsed_word);
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", _GP(play).bad_parsed_word);
 	if (_GP(play).bad_parsed_word[0] == 0)
 		return 0;
 	return 1;
diff --git a/engines/ags/engine/ac/global_translation.cpp b/engines/ags/engine/ac/global_translation.cpp
index f4c19a056ba..bfc1162bb42 100644
--- a/engines/ags/engine/ac/global_translation.cpp
+++ b/engines/ags/engine/ac/global_translation.cpp
@@ -68,7 +68,7 @@ int IsTranslationAvailable() {
 
 int GetTranslationName(char *buffer) {
 	VALIDATE_STRING(buffer);
-	strcpy(buffer, get_translation_name().GetCStr());
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", get_translation_name().GetCStr());
 	return IsTranslationAvailable();
 }
 
diff --git a/engines/ags/engine/ac/label.cpp b/engines/ags/engine/ac/label.cpp
index d7e4f88411b..81348ea41a7 100644
--- a/engines/ags/engine/ac/label.cpp
+++ b/engines/ags/engine/ac/label.cpp
@@ -39,7 +39,7 @@ const char *Label_GetText_New(GUILabel *labl) {
 }
 
 void Label_GetText(GUILabel *labl, char *buffer) {
-	strcpy(buffer, labl->GetText().GetCStr());
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", labl->GetText().GetCStr());
 }
 
 void Label_SetText(GUILabel *labl, const char *newtx) {
diff --git a/engines/ags/engine/ac/properties.cpp b/engines/ags/engine/ac/properties.cpp
index 82f633f50bd..fa719306c8f 100644
--- a/engines/ags/engine/ac/properties.cpp
+++ b/engines/ags/engine/ac/properties.cpp
@@ -76,7 +76,7 @@ void get_text_property(const StringIMap &st_prop, const StringIMap &rt_prop, con
 		return;
 
 	String val = get_property_value(st_prop, rt_prop, property, desc.DefaultValue);
-	strcpy(bufer, val.GetCStr());
+	snprintf(bufer, MAX_MAXSTRLEN, "%s", val.GetCStr());
 }
 
 const char *get_text_property_dynamic_string(const StringIMap &st_prop, const StringIMap &rt_prop, const char *property) {
diff --git a/engines/ags/engine/ac/textbox.cpp b/engines/ags/engine/ac/textbox.cpp
index a47629dfcfd..7aa110b90ac 100644
--- a/engines/ags/engine/ac/textbox.cpp
+++ b/engines/ags/engine/ac/textbox.cpp
@@ -38,7 +38,7 @@ const char *TextBox_GetText_New(GUITextBox *texbox) {
 }
 
 void TextBox_GetText(GUITextBox *texbox, char *buffer) {
-	strcpy(buffer, texbox->Text.GetCStr());
+	snprintf(buffer, MAX_MAXSTRLEN, "%s", texbox->Text.GetCStr());
 }
 
 void TextBox_SetText(GUITextBox *texbox, const char *newtex) {


Commit: e90fb004127f009a90c2841d238f408f91288a97
    https://github.com/scummvm/scummvm/commit/e90fb004127f009a90c2841d238f408f91288a97
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Replaced more strcpy with snprintf, for safety

>From upstream 1b6cf053ebbe85e7ba6c5828e2582d757503bc16

Changed paths:
    engines/ags/engine/ac/dialog.cpp
    engines/ags/engine/ac/game.cpp
    engines/ags/engine/ac/gui.cpp
    engines/ags/engine/ac/interface_element.cpp
    engines/ags/engine/debugging/debug.cpp
    engines/ags/engine/main/engine.cpp
    engines/ags/engine/main/main.cpp
    engines/ags/engine/script/executing_script.h
    engines/ags/shared/ac/character_info.cpp
    engines/ags/shared/ac/game_setup_struct.cpp
    engines/ags/shared/ac/words_dictionary.cpp
    engines/ags/shared/util/string_compat.cpp


diff --git a/engines/ags/engine/ac/dialog.cpp b/engines/ags/engine/ac/dialog.cpp
index 09c0927356e..01e9b542e9a 100644
--- a/engines/ags/engine/ac/dialog.cpp
+++ b/engines/ags/engine/ac/dialog.cpp
@@ -1003,7 +1003,7 @@ void DialogOptions::Close() {
 	invalidate_screen();
 
 	if (parserActivated) {
-		strcpy(_GP(play).lastParserEntry, parserInput->Text.GetCStr());
+		snprintf(_GP(play).lastParserEntry, MAX_MAXSTRLEN, "%s", parserInput->Text.GetCStr());
 		ParseText(parserInput->Text.GetCStr());
 		chose = CHOSE_TEXTPARSER;
 	}
diff --git a/engines/ags/engine/ac/game.cpp b/engines/ags/engine/ac/game.cpp
index 48f3e6459d8..ad73c2adad4 100644
--- a/engines/ags/engine/ac/game.cpp
+++ b/engines/ags/engine/ac/game.cpp
@@ -873,7 +873,8 @@ void save_game(int slotn, const char *descript) {
 	can_run_delayed_command();
 
 	if (_G(inside_script)) {
-		strcpy(_G(curscript)->postScriptSaveSlotDescription[_G(curscript)->queue_action(ePSASaveGame, slotn, "SaveGameSlot")], descript);
+		snprintf(_G(curscript)->postScriptSaveSlotDescription[_G(curscript)->queue_action(ePSASaveGame, slotn, "SaveGameSlot")],
+					 MAX_QUEUED_ACTION_DESC, "%s", descript);
 		return;
 	}
 
@@ -1290,7 +1291,7 @@ void replace_tokens(const char *srcmes, char *destm, int maxlen) {
 					quit("!Display: invalid global int index speicifed in @GI@");
 				snprintf(tval, sizeof(tval), "%d", GetGlobalInt(inx));
 			}
-			strcpy(destp, tval);
+			snprintf(destp, maxlen, "%s", tval);
 			indxdest += strlen(tval);
 		} else {
 			destp[0] = srcp[0];
diff --git a/engines/ags/engine/ac/gui.cpp b/engines/ags/engine/ac/gui.cpp
index f5e5f996c70..a227f4b6564 100644
--- a/engines/ags/engine/ac/gui.cpp
+++ b/engines/ags/engine/ac/gui.cpp
@@ -378,13 +378,13 @@ void replace_macro_tokens(const char *text, String &fixed_text) {
 			macroname[idd] = 0;
 			tempo[0] = 0;
 			if (ags_stricmp(macroname, "score") == 0)
-				sprintf(tempo, "%d", _GP(play).score);
+				snprintf(tempo, sizeof(tempo), "%d", _GP(play).score);
 			else if (ags_stricmp(macroname, "totalscore") == 0)
-				sprintf(tempo, "%d", MAXSCORE);
+				snprintf(tempo, sizeof(tempo), "%d", MAXSCORE);
 			else if (ags_stricmp(macroname, "scoretext") == 0)
-				sprintf(tempo, "%d of %d", _GP(play).score, MAXSCORE);
+				snprintf(tempo, sizeof(tempo), "%d of %d", _GP(play).score, MAXSCORE);
 			else if (ags_stricmp(macroname, "gamename") == 0)
-				strcpy(tempo, _GP(play).game_name);
+				snprintf(tempo, sizeof(tempo), "%s", _GP(play).game_name);
 			else if (ags_stricmp(macroname, "overhotspot") == 0) {
 				// While game is in Wait mode, no overhotspot text
 				if (!IsInterfaceEnabled())
@@ -393,7 +393,7 @@ void replace_macro_tokens(const char *text, String &fixed_text) {
 					GetLocationName(game_to_data_coord(_G(mousex)), game_to_data_coord(_G(mousey)), tempo);
 			} else { // not a macro, there's just a @ in the message
 				curptr = curptrWasAt + 1;
-				strcpy(tempo, "@");
+				snprintf(tempo, sizeof(tempo), "%s", "@");
 			}
 
 			fixed_text.Append(tempo);
diff --git a/engines/ags/engine/ac/interface_element.cpp b/engines/ags/engine/ac/interface_element.cpp
index 2f0dba02200..4bde84b60a1 100644
--- a/engines/ags/engine/ac/interface_element.cpp
+++ b/engines/ags/engine/ac/interface_element.cpp
@@ -27,7 +27,7 @@ namespace AGS3 {
 InterfaceElement::InterfaceElement() {
 	vtextxp = 0;
 	vtextyp = 1;
-	strcpy(vtext, "@SCORETEXT@$r at GAMENAME@");
+	snprintf(vtext, sizeof(vtext), "%s", "@SCORETEXT@$r at GAMENAME@");
 
 	numbuttons = 0;
 	bgcol = 8;
diff --git a/engines/ags/engine/debugging/debug.cpp b/engines/ags/engine/debugging/debug.cpp
index 6c9bdba5bdd..ab88b34c130 100644
--- a/engines/ags/engine/debugging/debug.cpp
+++ b/engines/ags/engine/debugging/debug.cpp
@@ -418,7 +418,7 @@ int check_for_messages_from_editor() {
 				}
 			} else {
 				_G(breakpoints).push_back(Globals::Breakpoint());
-				strcpy(_G(breakpoints)[_G(numBreakpoints)].scriptName, scriptNameBuf);
+				snprintf(_G(breakpoints)[_G(numBreakpoints)].scriptName, sizeof(Breakpoint::scriptName), "%s", scriptNameBuf);
 				_G(breakpoints)[_G(numBreakpoints)].lineNumber = lineNumber;
 				_G(numBreakpoints)++;
 			}
diff --git a/engines/ags/engine/main/engine.cpp b/engines/ags/engine/main/engine.cpp
index 73ebbd5f8c8..f6afcf00cd1 100644
--- a/engines/ags/engine/main/engine.cpp
+++ b/engines/ags/engine/main/engine.cpp
@@ -738,7 +738,7 @@ void engine_init_game_settings() {
 	_GP(play).speech_textwindow_gui = _GP(game).options[OPT_TWCUSTOM];
 	if (_GP(play).speech_textwindow_gui == 0)
 		_GP(play).speech_textwindow_gui = -1;
-	strcpy(_GP(play).game_name, _GP(game).gamename);
+	snprintf(_GP(play).game_name, sizeof(_GP(play).game_name), "%s", _GP(game).gamename);
 	_GP(play).lastParserEntry[0] = 0;
 	_GP(play).follow_change_room_timer = 150;
 	for (ee = 0; ee < MAX_ROOM_BGFRAMES; ee++)
diff --git a/engines/ags/engine/main/main.cpp b/engines/ags/engine/main/main.cpp
index 3699a8fe7cc..9372b788b89 100644
--- a/engines/ags/engine/main/main.cpp
+++ b/engines/ags/engine/main/main.cpp
@@ -204,7 +204,7 @@ int main_process_cmdline(ConfigTree &cfg, int argc, const char *argv[]) {
 			_G(loadSaveGameOnStartup) = atoi(argv[ee + 1]);
 			ee++;
 		} else if ((ags_stricmp(arg, "--enabledebugger") == 0) && (argc > ee + 1)) {
-			strcpy(_G(editor_debugger_instance_token), argv[ee + 1]);
+			snprintf(_G(editor_debugger_instance_token), sizeof(_G(editor_debugger_instance_token)), "%s", argv[ee + 1]);
 			_G(editor_debugging_enabled) = 1;
 			ee++;
 		} else if (ags_stricmp(arg, "--conf") == 0 && (argc > ee + 1)) {
diff --git a/engines/ags/engine/script/executing_script.h b/engines/ags/engine/script/executing_script.h
index 938d0213108..c7f77af13e1 100644
--- a/engines/ags/engine/script/executing_script.h
+++ b/engines/ags/engine/script/executing_script.h
@@ -40,6 +40,7 @@ enum PostScriptAction {
 
 #define MAX_QUEUED_SCRIPTS 4
 #define MAX_QUEUED_ACTIONS 5
+#define MAX_QUEUED_ACTION_DESC 100
 #define MAX_FUNCTION_NAME_LEN 60
 #define MAX_QUEUED_PARAMS  4
 
@@ -62,7 +63,7 @@ struct ExecutingScript {
 	PostScriptAction postScriptActions[MAX_QUEUED_ACTIONS];
 	const char *postScriptActionNames[MAX_QUEUED_ACTIONS];
 	ScriptPosition  postScriptActionPositions[MAX_QUEUED_ACTIONS];
-	char postScriptSaveSlotDescription[MAX_QUEUED_ACTIONS][100];
+	char postScriptSaveSlotDescription[MAX_QUEUED_ACTIONS][MAX_QUEUED_ACTION_DESC];
 	int  postScriptActionData[MAX_QUEUED_ACTIONS];
 	int  numPostScriptActions;
 	QueuedScript ScFnQueue[MAX_QUEUED_SCRIPTS];
diff --git a/engines/ags/shared/ac/character_info.cpp b/engines/ags/shared/ac/character_info.cpp
index 1f37d51c93f..c6052cdc110 100644
--- a/engines/ags/shared/ac/character_info.cpp
+++ b/engines/ags/shared/ac/character_info.cpp
@@ -157,8 +157,8 @@ void ConvertOldCharacterToNew(OldCharacterInfo *oci, CharacterInfo *ci) {
 	COPY_CHAR_VAR(actx);
 	COPY_CHAR_VAR(acty);
 	COPY_CHAR_VAR(on);
-	strcpy(ci->name, oci->name);
-	strcpy(ci->scrname, oci->scrname);
+	snprintf(ci->name, sizeof(CharacterInfo::name), "%s", oci->name);
+	snprintf(ci->scrname, sizeof(CharacterInfo::scrname), "%s", oci->scrname);
 	memcpy(&ci->inv[0], &oci->inv[0], sizeof(short) * 100);
 	// move the talking colour into the struct and remove from flags
 	ci->talkcolor = (oci->flags & OCHF_SPEECHCOL) >> OCHF_SPEECHCOLSHIFT;
diff --git a/engines/ags/shared/ac/game_setup_struct.cpp b/engines/ags/shared/ac/game_setup_struct.cpp
index ad490677641..bc7be448b4c 100644
--- a/engines/ags/shared/ac/game_setup_struct.cpp
+++ b/engines/ags/shared/ac/game_setup_struct.cpp
@@ -369,7 +369,7 @@ void GameSetupStruct::ReadFromSaveGame_v321(Stream *in, char *gswas, ccScript *c
 //=============================================================================
 
 void ConvertOldGameStruct(OldGameSetupStruct *ogss, GameSetupStruct *gss) {
-	strcpy(gss->gamename, ogss->gamename);
+	snprintf(gss->gamename, sizeof(GameSetupStruct::gamename), "%s", ogss->gamename);
 	for (int i = 0; i < 20; i++)
 		gss->options[i] = ogss->options[i];
 	memcpy(&gss->paluses[0], &ogss->paluses[0], 256);
diff --git a/engines/ags/shared/ac/words_dictionary.cpp b/engines/ags/shared/ac/words_dictionary.cpp
index 3019bd096d9..aab935661c3 100644
--- a/engines/ags/shared/ac/words_dictionary.cpp
+++ b/engines/ags/shared/ac/words_dictionary.cpp
@@ -73,9 +73,9 @@ void WordsDictionary::sort() {
 
 				wordnum[aa] = wordnum[bb];
 				wordnum[bb] = temp;
-				strcpy(tempst, word[aa]);
-				strcpy(word[aa], word[bb]);
-				strcpy(word[bb], tempst);
+				snprintf(tempst, MAX_PARSER_WORD_LENGTH, "%s", word[aa]);
+				snprintf(word[aa], MAX_PARSER_WORD_LENGTH, "%s", word[bb]);
+				snprintf(word[bb], MAX_PARSER_WORD_LENGTH, "%s", tempst);
 				bb = aa;
 			}
 		}
diff --git a/engines/ags/shared/util/string_compat.cpp b/engines/ags/shared/util/string_compat.cpp
index 55a466fd1cf..4ad775b7982 100644
--- a/engines/ags/shared/util/string_compat.cpp
+++ b/engines/ags/shared/util/string_compat.cpp
@@ -48,8 +48,9 @@ int ags_strnicmp(const char *s1, const char *s2, size_t n) {
 }
 
 char *ags_strdup(const char *s) {
-	char *result = (char *)malloc(strlen(s) + 1);
-	strcpy(result, s);
+	size_t len = strlen(s);
+	char *result = (char *)malloc(len + 1);
+	memcpy(result, s, len + 1);
 	return result;
 }
 


Commit: 9e940e436039063c2fb0248133f569447a49f3af
    https://github.com/scummvm/scummvm/commit/9e940e436039063c2fb0248133f569447a49f3af
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Put more unused code under OBSOLETE

>From upstream 0d129c9cf854fe12e277fe6b52a8dca076a51ad8

Changed paths:
    engines/ags/engine/ac/interface_button.cpp
    engines/ags/engine/ac/interface_element.cpp
    engines/ags/shared/ac/character_info.cpp
    engines/ags/shared/ac/character_info.h
    engines/ags/shared/ac/game_setup_struct.cpp
    engines/ags/shared/ac/game_setup_struct.h
    engines/ags/shared/ac/interface_button.h
    engines/ags/shared/ac/interface_element.h
    engines/ags/shared/ac/old_game_setup_struct.h


diff --git a/engines/ags/engine/ac/interface_button.cpp b/engines/ags/engine/ac/interface_button.cpp
index a8a06b52cf0..c609282eae3 100644
--- a/engines/ags/engine/ac/interface_button.cpp
+++ b/engines/ags/engine/ac/interface_button.cpp
@@ -19,6 +19,8 @@
  *
  */
 
+#if defined (OBSOLETE)
+
 #include "ags/shared/ac/interface_button.h"
 
 namespace AGS3 {
@@ -36,3 +38,5 @@ void InterfaceButton::set(int xx, int yy, int picc, int overpicc, int actionn) {
 }
 
 } // namespace AGS3
+
+#endif // OBSOLETE
diff --git a/engines/ags/engine/ac/interface_element.cpp b/engines/ags/engine/ac/interface_element.cpp
index 4bde84b60a1..5a845b9d36f 100644
--- a/engines/ags/engine/ac/interface_element.cpp
+++ b/engines/ags/engine/ac/interface_element.cpp
@@ -19,6 +19,8 @@
  *
  */
 
+#if defined (OBSOLETE)
+
 #include "common/str.h"
 #include "ags/shared/ac/interface_element.h"
 
@@ -38,3 +40,5 @@ InterfaceElement::InterfaceElement() {
 }
 
 } // namespace AGS3
+
+#endif // OBSOLETE
diff --git a/engines/ags/shared/ac/character_info.cpp b/engines/ags/shared/ac/character_info.cpp
index c6052cdc110..0b220b7814f 100644
--- a/engines/ags/shared/ac/character_info.cpp
+++ b/engines/ags/shared/ac/character_info.cpp
@@ -130,6 +130,9 @@ void CharacterInfo::WriteToFile(Stream *out) {
 	out->WriteInt8(on);
 }
 
+#if defined (OBSOLETE)
+#define COPY_CHAR_VAR(name) ci->name = oci->name
+
 void ConvertOldCharacterToNew(OldCharacterInfo *oci, CharacterInfo *ci) {
 	COPY_CHAR_VAR(defview);
 	COPY_CHAR_VAR(talkview);
@@ -164,5 +167,6 @@ void ConvertOldCharacterToNew(OldCharacterInfo *oci, CharacterInfo *ci) {
 	ci->talkcolor = (oci->flags & OCHF_SPEECHCOL) >> OCHF_SPEECHCOLSHIFT;
 	ci->flags = ci->flags & (~OCHF_SPEECHCOL);
 }
+#endif // OBSOLETE
 
 } // namespace AGS3
diff --git a/engines/ags/shared/ac/character_info.h b/engines/ags/shared/ac/character_info.h
index c2b93b34736..cc73de68bc6 100644
--- a/engines/ags/shared/ac/character_info.h
+++ b/engines/ags/shared/ac/character_info.h
@@ -136,6 +136,7 @@ struct CharacterInfo {
 };
 
 
+#if defined (OBSOLETE)
 struct OldCharacterInfo {
 	int   defview;
 	int   talkview;
@@ -160,8 +161,8 @@ struct OldCharacterInfo {
 	int8  on;
 };
 
-#define COPY_CHAR_VAR(name) ci->name = oci->name
 void ConvertOldCharacterToNew(OldCharacterInfo *oci, CharacterInfo *ci);
+#endif // OBSOLETE
 
 } // namespace AGS3
 
diff --git a/engines/ags/shared/ac/game_setup_struct.cpp b/engines/ags/shared/ac/game_setup_struct.cpp
index bc7be448b4c..19b5968eecd 100644
--- a/engines/ags/shared/ac/game_setup_struct.cpp
+++ b/engines/ags/shared/ac/game_setup_struct.cpp
@@ -367,6 +367,7 @@ void GameSetupStruct::ReadFromSaveGame_v321(Stream *in, char *gswas, ccScript *c
 }
 
 //=============================================================================
+#if defined (OBSOLETE)
 
 void ConvertOldGameStruct(OldGameSetupStruct *ogss, GameSetupStruct *gss) {
 	snprintf(gss->gamename, sizeof(GameSetupStruct::gamename), "%s", ogss->gamename);
@@ -409,6 +410,7 @@ void ConvertOldGameStruct(OldGameSetupStruct *ogss, GameSetupStruct *gss) {
 	gss->compiled_script = ogss->compiled_script;
 	gss->numcursors = 10;
 }
+#endif // OBSOLETE
 
 void GameSetupStruct::ReadFromSavegame(Stream *in) {
 	// of GameSetupStruct
diff --git a/engines/ags/shared/ac/game_setup_struct.h b/engines/ags/shared/ac/game_setup_struct.h
index cabc9eff3d3..b451ba22391 100644
--- a/engines/ags/shared/ac/game_setup_struct.h
+++ b/engines/ags/shared/ac/game_setup_struct.h
@@ -47,7 +47,6 @@ typedef std::shared_ptr<InteractionScripts> PInteractionScripts;
 using AGS::Shared::PInteraction;
 using AGS::Shared::PInteractionScripts;
 using AGS::Shared::HGameFileError;
-struct OldGameSetupStruct;
 
 
 // TODO: split GameSetupStruct into struct used to hold loaded game data, and actual runtime object
@@ -167,8 +166,11 @@ struct GameSetupStruct : public GameSetupStructBase {
 };
 
 //=============================================================================
-// TODO: find out how this function was supposed to be used
+#if defined (OBSOLETE)
+struct OldGameSetupStruct;
 void ConvertOldGameStruct(OldGameSetupStruct *ogss, GameSetupStruct *gss);
+#endif // OBSOLETE
+
 // Finds an audio clip using legacy convention index
 ScriptAudioClip *GetAudioClipForOldStyleNumber(GameSetupStruct &game, bool is_music, int num);
 
diff --git a/engines/ags/shared/ac/interface_button.h b/engines/ags/shared/ac/interface_button.h
index f7f3e97c710..96fbdfda9d3 100644
--- a/engines/ags/shared/ac/interface_button.h
+++ b/engines/ags/shared/ac/interface_button.h
@@ -24,6 +24,8 @@
 
 #include "ags/shared/core/types.h"
 
+#if defined (OBSOLETE)
+
 namespace AGS3 {
 
 #define MAXBUTTON       20
@@ -40,4 +42,6 @@ struct InterfaceButton {
 
 } // namespace AGS3
 
+#endif // OBSOLETE
+
 #endif
diff --git a/engines/ags/shared/ac/interface_element.h b/engines/ags/shared/ac/interface_element.h
index a4e0a834816..de77c34649f 100644
--- a/engines/ags/shared/ac/interface_element.h
+++ b/engines/ags/shared/ac/interface_element.h
@@ -22,6 +22,8 @@
 #ifndef AGS_SHARED_AC_INTERFACE_ELEMENT_H
 #define AGS_SHARED_AC_INTERFACE_ELEMENT_H
 
+#if defined (OBSOLETE)
+
 #include "ags/shared/ac/interface_button.h" // InterfaceButton
 
 namespace AGS3 {
@@ -44,4 +46,6 @@ struct InterfaceElement {
 
 } // namespace AGS3
 
+#endif // OBSOLETE
+
 #endif
diff --git a/engines/ags/shared/ac/old_game_setup_struct.h b/engines/ags/shared/ac/old_game_setup_struct.h
index 572facd4b2a..2018d9355f9 100644
--- a/engines/ags/shared/ac/old_game_setup_struct.h
+++ b/engines/ags/shared/ac/old_game_setup_struct.h
@@ -22,10 +22,10 @@
 #ifndef AGS_SHARED_AC_OLD_GAME_SETUP_STRUCT_H
 #define AGS_SHARED_AC_OLD_GAME_SETUP_STRUCT_H
 
-#include "ags/shared/ac/character_info.h"       // OldCharacterInfo, CharacterInfo
 #if defined (OBSOLETE)
+
+#include "ags/shared/ac/character_info.h"       // OldCharacterInfo, CharacterInfo
 #include "ags/shared/ac/event_block.h"       // EventBlock
-#endif
 #include "ags/shared/ac/interface_element.h"    // InterfaceElement
 #include "ags/shared/ac/inventory_item_info.h"   // InventoryItemInfo
 #include "ags/shared/ac/mouse_cursor.h"      // MouseCursor
@@ -84,3 +84,5 @@ struct OldGameSetupStruct : public OriGameSetupStruct2 {
 } // namespace AGS3
 
 #endif
+
+#endif


Commit: b49d3b2e9ff228b022cc9b3cc818ae0546d99f2a
    https://github.com/scummvm/scummvm/commit/b49d3b2e9ff228b022cc9b3cc818ae0546d99f2a
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: In unload_game_file() don't error if quit during the of dialog

>From upstream c7bd448dfffc197c4398a38395b444457c958d24

Changed paths:
    engines/ags/engine/ac/game.cpp


diff --git a/engines/ags/engine/ac/game.cpp b/engines/ags/engine/ac/game.cpp
index ad73c2adad4..33b153ada9e 100644
--- a/engines/ags/engine/ac/game.cpp
+++ b/engines/ags/engine/ac/game.cpp
@@ -364,7 +364,6 @@ void free_do_once_tokens() {
 
 
 // Free all the memory associated with the game
-// TODO: call this when exiting the game (currently only called in RunAGSGame)
 void unload_game_file() {
 	close_translation();
 
@@ -379,16 +378,10 @@ void unload_game_file() {
 	delete _G(gameinst);
 	_G(gameinstFork) = nullptr;
 	_G(gameinst) = nullptr;
-
 	_GP(gamescript).reset();
 
-	if ((_G(dialogScriptsInst) != nullptr) && (_G(dialogScriptsInst)->pc != 0)) {
-		quit("Error: unload_game called while dialog script still running");
-	} else if (_G(dialogScriptsInst) != nullptr) {
-		delete _G(dialogScriptsInst);
-		_G(dialogScriptsInst) = nullptr;
-	}
-
+	delete _G(dialogScriptsInst);
+	_G(dialogScriptsInst) = nullptr;
 	_GP(dialogScriptsScript).reset();
 
 	for (size_t i = 0; i < _G(numScriptModules); ++i) {


Commit: 10033d195e6570181f1e984e3a079539bc848fe6
    https://github.com/scummvm/scummvm/commit/10033d195e6570181f1e984e3a079539bc848fe6
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Store camera & viewport handles, not script ptrs, for safety

>From ustream dc1421a88cd620bd1d34f8bf856e8baa57a50904
Also includes upstream 09c27c38c3afa27194b0b5dd9276036b76267488

Changed paths:
    engines/ags/engine/ac/game_state.cpp
    engines/ags/engine/ac/game_state.h


diff --git a/engines/ags/engine/ac/game_state.cpp b/engines/ags/engine/ac/game_state.cpp
index 783dad88f79..064a48c2ec4 100644
--- a/engines/ags/engine/ac/game_state.cpp
+++ b/engines/ags/engine/ac/game_state.cpp
@@ -242,9 +242,8 @@ PViewport GameState::CreateRoomViewport() {
 	PViewport viewport(new Viewport());
 	viewport->SetID(index);
 	viewport->SetRect(_mainViewport.GetRect());
-	ScriptViewport *scv = new ScriptViewport(index);
 	_roomViewports.push_back(viewport);
-	_scViewportRefs.push_back(std::make_pair<ScriptViewport*, int32_t>(scv, 0));
+	_scViewportHandles.push_back(0);
 	_roomViewportsSorted.push_back(viewport);
 	_roomViewportZOrderChanged = true;
 	on_roomviewport_created(index);
@@ -254,32 +253,37 @@ PViewport GameState::CreateRoomViewport() {
 ScriptViewport *GameState::RegisterRoomViewport(int index, int32_t handle) {
 	if (index < 0 || (size_t)index >= _roomViewports.size())
 		return nullptr;
-	auto &scobj = _scViewportRefs[index];
+	auto scview = new ScriptViewport(index);
 	if (handle == 0) {
-		handle = ccRegisterManagedObject(scobj.first, scobj.first);
+		handle = ccRegisterManagedObject(scview, scview);
 		ccAddObjectReference(handle); // one reference for the GameState
 	} else {
-		ccRegisterUnserializedObject(handle, scobj.first, scobj.first);
+		ccRegisterUnserializedObject(handle, scview, scview);
 	}
-	scobj.second = handle;
-	return scobj.first;
+	_scViewportHandles[index] = handle; // save handle for us
+	return scview;
 }
 
 void GameState::DeleteRoomViewport(int index) {
 	// NOTE: viewport 0 can not be deleted
 	if (index <= 0 || (size_t)index >= _roomViewports.size())
 		return;
-	auto scobj = _scViewportRefs[index];
-	scobj.first->Invalidate();
-	ccReleaseObjectReference(scobj.second);
+	auto handle = _scViewportHandles[index];
+	auto scobj = const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(handle));
+	if (scobj)
+		scobj->Invalidate();
+	ccReleaseObjectReference(handle);
 	auto cam = _roomViewports[index]->GetCamera();
 	if (cam)
 		cam->UnlinkFromViewport(index);
 	_roomViewports.erase(_roomViewports.begin() + index);
-	_scViewportRefs.erase(_scViewportRefs.begin() + index);
+	_scViewportHandles.erase(_scViewportHandles.begin() + index);
 	for (size_t i = index; i < _roomViewports.size(); ++i) {
 		_roomViewports[i]->SetID(i);
-		_scViewportRefs[i].first->SetID(i);
+		handle = _scViewportHandles[index];
+		scobj = const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(handle));
+		if (scobj)
+			scobj->SetID(i);
 	}
 	for (size_t i = 0; i < _roomViewportsSorted.size(); ++i) {
 		if (_roomViewportsSorted[i]->GetID() == index) {
@@ -300,8 +304,7 @@ PCamera GameState::CreateRoomCamera() {
 	camera->SetID(index);
 	camera->SetAt(0, 0);
 	camera->SetSize(_mainViewport.GetRect().GetSize());
-	ScriptCamera *scam = new ScriptCamera(index);
-	_scCameraRefs.push_back(std::make_pair<ScriptCamera*, int32_t>(scam, 0));
+	_scCameraHandles.push_back(0);
 	_roomCameras.push_back(camera);
 	return camera;
 }
@@ -309,34 +312,39 @@ PCamera GameState::CreateRoomCamera() {
 ScriptCamera *GameState::RegisterRoomCamera(int index, int32_t handle) {
 	if (index < 0 || (size_t)index >= _roomCameras.size())
 		return nullptr;
-	auto &scobj = _scCameraRefs[index];
+	auto sccamera = new ScriptCamera(index);
 	if (handle == 0) {
-		handle = ccRegisterManagedObject(scobj.first, scobj.first);
+		handle = ccRegisterManagedObject(sccamera, sccamera);
 		ccAddObjectReference(handle); // one reference for the GameState
 	} else {
-		ccRegisterUnserializedObject(handle, scobj.first, scobj.first);
+		ccRegisterUnserializedObject(handle, sccamera, sccamera);
 	}
-	scobj.second = handle;
-	return scobj.first;
+	_scCameraHandles[index] = handle;
+	return sccamera;
 }
 
 void GameState::DeleteRoomCamera(int index) {
 	// NOTE: camera 0 can not be deleted
 	if (index <= 0 || (size_t)index >= _roomCameras.size())
 		return;
-	auto scobj = _scCameraRefs[index];
-	scobj.first->Invalidate();
-	ccReleaseObjectReference(scobj.second);
+	auto handle = _scCameraHandles[index];
+	auto scobj = const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(handle));
+	if (scobj)
+		scobj->Invalidate();
+	ccReleaseObjectReference(handle);
 	for (auto &viewref : _roomCameras[index]->GetLinkedViewports()) {
 		auto view = viewref.lock();
 		if (view)
 			view->LinkCamera(nullptr);
 	}
 	_roomCameras.erase(_roomCameras.begin() + index);
-	_scCameraRefs.erase(_scCameraRefs.begin() + index);
+	_scCameraHandles.erase(_scCameraHandles.begin() + index);
 	for (size_t i = index; i < _roomCameras.size(); ++i) {
 		_roomCameras[i]->SetID(i);
-		_scCameraRefs[i].first->SetID(i);
+		handle = _scCameraHandles[index];
+		scobj = const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(handle));
+		if (scobj)
+			scobj->SetID(i);
 	}
 }
 
@@ -347,13 +355,13 @@ int GameState::GetRoomCameraCount() const {
 ScriptViewport *GameState::GetScriptViewport(int index) {
 	if (index < 0 || (size_t)index >= _roomViewports.size())
 		return nullptr;
-	return _scViewportRefs[index].first;
+	return const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(_scViewportHandles[index]));
 }
 
 ScriptCamera *GameState::GetScriptCamera(int index) {
 	if (index < 0 || (size_t)index >= _roomCameras.size())
 		return nullptr;
-	return _scCameraRefs[index].first;
+	return const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(_scCameraHandles[index]));
 }
 
 bool GameState::IsIgnoringInput() const {
@@ -816,17 +824,21 @@ void GameState::FreeProperties() {
 void GameState::FreeViewportsAndCameras() {
 	_roomViewports.clear();
 	_roomViewportsSorted.clear();
-	for (auto &scobj : _scViewportRefs) {
-		scobj.first->Invalidate();
-		ccReleaseObjectReference(scobj.second);
+	for (auto handle : _scViewportHandles) {
+		auto scview = const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(handle));
+		if (scview)
+			scview->Invalidate();
+		ccReleaseObjectReference(handle);
 	}
-	_scViewportRefs.clear();
+	_scViewportHandles.clear();
 	_roomCameras.clear();
-	for (auto &scobj : _scCameraRefs) {
-		scobj.first->Invalidate();
-		ccReleaseObjectReference(scobj.second);
+	for (auto handle : _scCameraHandles) {
+		auto sccam = const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(handle));
+		if (sccam)
+			sccam->Invalidate();
+		ccReleaseObjectReference(handle);
 	}
-	_scCameraRefs.clear();
+	_scCameraHandles.clear();
 }
 
 void GameState::ReadCustomProperties_v340(Shared::Stream *in) {
diff --git a/engines/ags/engine/ac/game_state.h b/engines/ags/engine/ac/game_state.h
index cddf8991b65..f8db2536781 100644
--- a/engines/ags/engine/ac/game_state.h
+++ b/engines/ags/engine/ac/game_state.h
@@ -404,11 +404,10 @@ private:
 	std::vector<PViewport> _roomViewportsSorted;
 	// Cameras defines the position of a "looking eye" inside the room.
 	std::vector<PCamera> _roomCameras;
-	// Script viewports and cameras are references to real data export to
-	// user script. They became invalidated as the actual object gets
-	// destroyed, but are kept in memory to prevent script errors.
-	std::vector<std::pair<ScriptViewport *, int32_t>> _scViewportRefs;
-	std::vector<std::pair<ScriptCamera *, int32_t>> _scCameraRefs;
+	// We keep handles to the script refs to viewports and cameras, so that we
+	// could address them and invalidate as the actual object gets destroyed.
+	std::vector<int32_t> _scViewportHandles;
+	std::vector<int32_t> _scCameraHandles;
 
 	// Tells that the main viewport's position has changed since last game update
 	bool  _mainViewportHasChanged = false;


Commit: 30038fe4263e9943102e4b7d5ccae2d3ee71dcae
    https://github.com/scummvm/scummvm/commit/30038fe4263e9943102e4b7d5ccae2d3ee71dcae
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2022-06-19T20:50:24+01:00

Commit Message:
AGS: Store certain overlay handles, not script ptrs, for safety

>From upstream b73553d6b4655feec2f82ee1feaa67a6bc9fca23

Changed paths:
    engines/ags/engine/ac/game_state.cpp
    engines/ags/engine/ac/game_state.h
    engines/ags/engine/ac/overlay.cpp
    engines/ags/engine/ac/speech.cpp


diff --git a/engines/ags/engine/ac/game_state.cpp b/engines/ags/engine/ac/game_state.cpp
index 064a48c2ec4..e675b6d2314 100644
--- a/engines/ags/engine/ac/game_state.cpp
+++ b/engines/ags/engine/ac/game_state.cpp
@@ -270,9 +270,10 @@ void GameState::DeleteRoomViewport(int index) {
 		return;
 	auto handle = _scViewportHandles[index];
 	auto scobj = const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(handle));
-	if (scobj)
+	if (scobj) {
 		scobj->Invalidate();
-	ccReleaseObjectReference(handle);
+		ccReleaseObjectReference(handle);
+	}
 	auto cam = _roomViewports[index]->GetCamera();
 	if (cam)
 		cam->UnlinkFromViewport(index);
@@ -329,9 +330,10 @@ void GameState::DeleteRoomCamera(int index) {
 		return;
 	auto handle = _scCameraHandles[index];
 	auto scobj = const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(handle));
-	if (scobj)
+	if (scobj) {
 		scobj->Invalidate();
-	ccReleaseObjectReference(handle);
+		ccReleaseObjectReference(handle);
+	}
 	for (auto &viewref : _roomCameras[index]->GetLinkedViewports()) {
 		auto view = viewref.lock();
 		if (view)
@@ -826,17 +828,19 @@ void GameState::FreeViewportsAndCameras() {
 	_roomViewportsSorted.clear();
 	for (auto handle : _scViewportHandles) {
 		auto scview = const_cast<ScriptViewport*>((const ScriptViewport*)ccGetObjectAddressFromHandle(handle));
-		if (scview)
+		if (scview) {
 			scview->Invalidate();
-		ccReleaseObjectReference(handle);
+			ccReleaseObjectReference(handle);
+		}
 	}
 	_scViewportHandles.clear();
 	_roomCameras.clear();
 	for (auto handle : _scCameraHandles) {
 		auto sccam = const_cast<ScriptCamera*>((const ScriptCamera*)ccGetObjectAddressFromHandle(handle));
-		if (sccam)
+		if (sccam) {
 			sccam->Invalidate();
-		ccReleaseObjectReference(handle);
+			ccReleaseObjectReference(handle);
+		}
 	}
 	_scCameraHandles.clear();
 }
diff --git a/engines/ags/engine/ac/game_state.h b/engines/ags/engine/ac/game_state.h
index f8db2536781..1cefd30ef6d 100644
--- a/engines/ags/engine/ac/game_state.h
+++ b/engines/ags/engine/ac/game_state.h
@@ -254,12 +254,12 @@ struct GameState {
 	int  complete_overlay_on = 0;
 	// Is there a blocking text overlay on screen (contains overlay ID)
 	int  text_overlay_on = 0;
-	// Script overlay objects, because we must return same pointers
+	// Script overlay handles, because we must return same script objects
 	// whenever user script queries for them.
-	// Blocking speech overlay managed object, for accessing in scripts
-	ScriptOverlay *speech_text_scover = nullptr;
-	// Speech portrait overlay managed object
-	ScriptOverlay *speech_face_scover = nullptr;
+	// Blocking speech overlay managed handle
+	int  speech_text_schandle = 0;
+	// Speech portrait overlay managed handle
+	int  speech_face_schandle = 0;
 
 	int shake_screen_yoff = 0; // y offset of the shaking screen
 
diff --git a/engines/ags/engine/ac/overlay.cpp b/engines/ags/engine/ac/overlay.cpp
index 6c98bbddaa4..8911b7eca1d 100644
--- a/engines/ags/engine/ac/overlay.cpp
+++ b/engines/ags/engine/ac/overlay.cpp
@@ -279,27 +279,24 @@ ScriptOverlay *create_scriptoverlay(ScreenOverlay &over, bool internal_ref) {
 	ScriptOverlay *scover = new ScriptOverlay();
 	scover->overlayId = over.type;
 	int handl = ccRegisterManagedObject(scover, scover);
-	over.associatedOverlayHandle = handl;
-	if (internal_ref)
+	over.associatedOverlayHandle = handl; // save the handle for access
+	if (internal_ref) // requested additional ref
 		ccAddObjectReference(handl);
 	return scover;
 }
 
 // Invalidates existing script object to let user know that previous overlay is gone,
 // and releases engine's internal reference (script object may exist while there are user refs)
-static void invalidate_and_subref(ScreenOverlay &over, ScriptOverlay **scover) {
-	if (scover && (*scover)) {
-		(*scover)->overlayId = -1;
-		*scover = nullptr;
-	} else if (over.associatedOverlayHandle > 0) {
-		ScriptOverlay *scoverlay = const_cast<ScriptOverlay*>((const ScriptOverlay* )ccGetObjectAddressFromHandle(over.associatedOverlayHandle));
-		if (scoverlay) scoverlay->overlayId = -1;
-	}
+static void invalidate_and_subref(ScreenOverlay &over) {
+	if (over.associatedOverlayHandle <= 0)
+		return; // invalid handle
 
-	if (over.associatedOverlayHandle > 0) {
+	ScriptOverlay *scover = const_cast<ScriptOverlay*>((const ScriptOverlay*)ccGetObjectAddressFromHandle(over.associatedOverlayHandle));
+	if (scover) {
+		scover->overlayId = -1; // invalidate script object
 		ccReleaseObjectReference(over.associatedOverlayHandle);
-		over.associatedOverlayHandle = 0;
 	}
+	over.associatedOverlayHandle = 0; // reset internal handle
 }
 
 // Frees overlay resources and tell to dispose script object if there are no refs left
@@ -325,13 +322,15 @@ void remove_screen_overlay_index(size_t over_idx) {
 	if (over.type == _GP(play).complete_overlay_on) {
 		_GP(play).complete_overlay_on = 0;
 	} else if (over.type == _GP(play).text_overlay_on) { // release internal ref for speech text
-		invalidate_and_subref(over, &_GP(play).speech_text_scover);
+		invalidate_and_subref(over);
+		_GP(play).speech_text_schandle = 0;
 		_GP(play).text_overlay_on = 0;
 	} else if (over.type == OVER_PICTURE) { // release internal ref for speech face
-		invalidate_and_subref(over, &_GP(play).speech_face_scover);
+		invalidate_and_subref(over);
+		_GP(play).speech_face_schandle = 0;
 		_G(face_talking) = -1;
 	} else if (over.bgSpeechForChar >= 0) { // release internal ref for bg speech
-		invalidate_and_subref(over, nullptr);
+		invalidate_and_subref(over);
 	}
 	dispose_overlay(over);
 	_GP(screenover).erase(_GP(screenover).begin() + over_idx);
@@ -393,10 +392,13 @@ size_t add_screen_overlay_impl(bool roomlayer, int x, int y, int type, int sprnu
 		_GP(play).text_overlay_on = type;
 		// only make script object for blocking speech now, because messagebox blocks all script
 		// and therefore cannot be accessed, so no practical reason for that atm
-		if (type == OVER_TEXTSPEECH)
-			_GP(play).speech_text_scover = create_scriptoverlay(over, true);
+		if (type == OVER_TEXTSPEECH) {
+			create_scriptoverlay(over, true);
+			_GP(play).speech_text_schandle = over.associatedOverlayHandle;
+		}
 	} else if (type == OVER_PICTURE) {
-		_GP(play).speech_face_scover = create_scriptoverlay(over, true);
+		create_scriptoverlay(over, true);
+		_GP(play).speech_face_schandle = over.associatedOverlayHandle;
 	}
 	over.MarkChanged();
 	_GP(screenover).push_back(std::move(over));
diff --git a/engines/ags/engine/ac/speech.cpp b/engines/ags/engine/ac/speech.cpp
index 17efec2e829..b6f2167b53b 100644
--- a/engines/ags/engine/ac/speech.cpp
+++ b/engines/ags/engine/ac/speech.cpp
@@ -28,6 +28,7 @@
 #include "ags/engine/ac/game_state.h"
 #include "ags/engine/ac/global_audio.h"
 #include "ags/engine/ac/global_display.h"
+#include "ags/engine/ac/dynobj/cc_dynamic_object.h"
 #include "ags/shared/debugging/out.h"
 #include "ags/engine/script/script_api.h"
 #include "ags/engine/script/script_runtime.h"
@@ -149,11 +150,11 @@ String get_voice_assetpath() {
 //=============================================================================
 
 ScriptOverlay *Speech_GetTextOverlay() {
-	return _GP(play).speech_text_scover;
+	return const_cast<ScriptOverlay*>((const ScriptOverlay*)ccGetObjectAddressFromHandle(_GP(play).speech_text_schandle));
 }
 
 ScriptOverlay *Speech_GetPortraitOverlay() {
-	return _GP(play).speech_face_scover;
+	return const_cast<ScriptOverlay*>((const ScriptOverlay*)ccGetObjectAddressFromHandle(_GP(play).speech_face_schandle));
 }
 
 RuntimeScriptValue Sc_Speech_GetAnimationStopTimeMargin(const RuntimeScriptValue *params, int32_t param_count) {




More information about the Scummvm-git-logs mailing list