[Scummvm-git-logs] scummvm master -> 9be6d156df81657f3ea56557f202bf9f88268ab8

dreammaster noreply at scummvm.org
Wed Mar 30 04:49:05 UTC 2022


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

Summary:
fb52856d7e AGS: Reorder function linking steps
36c4da51f3 AGS: Fixed room script linking errors were not reported fully
9be6d156df AGS: Updated build version (3.6.0.12)


Commit: fb52856d7e979938c3aa101f2ce454ca5fa1d46f
    https://github.com/scummvm/scummvm/commit/fb52856d7e979938c3aa101f2ce454ca5fa1d46f
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2022-03-29T21:43:59-07:00

Commit Message:
AGS: Reorder function linking steps

>From upstream 0f977a56f0b1cf293f711e8380bb9ddaa54d2573

Changed paths:
    engines/ags/engine/ac/room.cpp
    engines/ags/engine/script/cc_instance.cpp
    engines/ags/engine/script/cc_instance.h
    engines/ags/engine/script/script.cpp


diff --git a/engines/ags/engine/ac/room.cpp b/engines/ags/engine/ac/room.cpp
index 364de3c6676..b7fa01aea4a 100644
--- a/engines/ags/engine/ac/room.cpp
+++ b/engines/ags/engine/ac/room.cpp
@@ -997,6 +997,12 @@ void compile_room_script() {
 		quitprintf("Unable to create local script: %s", _G(ccErrorString).GetCStr());
 	}
 
+	if (!_G(roominst)->ResolveScriptImports(_G(roominst)->instanceof.get()))
+		quitprintf("Unable to resolve imports in room script");
+
+	if (!_G(roominst)->ResolveImportFixups(_G(roominst)->instanceof.get()))
+		quitprintf("Unable to resolve import fixups in room script");
+
 	_G(roominstFork) = _G(roominst)->Fork();
 	if (_G(roominstFork) == nullptr)
 		quitprintf("Unable to create forked room instance: %s", _G(ccErrorString).GetCStr());
diff --git a/engines/ags/engine/script/cc_instance.cpp b/engines/ags/engine/script/cc_instance.cpp
index 1901d8658ad..4af04790a20 100644
--- a/engines/ags/engine/script/cc_instance.cpp
+++ b/engines/ags/engine/script/cc_instance.cpp
@@ -1321,7 +1321,6 @@ bool ccInstance::IsBeingRun() const {
 }
 
 bool ccInstance::_Create(PScript scri, ccInstance *joined) {
-	int i;
 	_G(currentline) = -1;
 	if ((scri == nullptr) && (joined != nullptr))
 		scri = joined->instanceof;
@@ -1355,7 +1354,7 @@ bool ccInstance::_Create(PScript scri, ccInstance *joined) {
 			code = (intptr_t *)malloc(codesize * sizeof(intptr_t));
 			// 64 bit: Read code into 8 byte array, necessary for being able to perform
 			// relocations on the references.
-			for (i = 0; i < codesize; ++i)
+			for (int i = 0; i < codesize; ++i)
 				code[i] = scri->code[i];
 		}
 	}
@@ -1376,7 +1375,7 @@ bool ccInstance::_Create(PScript scri, ccInstance *joined) {
 	}
 
 	// find a LoadedInstance slot for it
-	for (i = 0; i < MAX_LOADED_INSTANCES; i++) {
+	for (int i = 0; i < MAX_LOADED_INSTANCES; i++) {
 		if (_G(loadedInstances)[i] == nullptr) {
 			_G(loadedInstances)[i] = this;
 			loadedInstanceId = i;
@@ -1392,9 +1391,6 @@ bool ccInstance::_Create(PScript scri, ccInstance *joined) {
 		resolved_imports = joined->resolved_imports;
 		code_fixups = joined->code_fixups;
 	} else {
-		if (!ResolveScriptImports(scri.get())) {
-			return false;
-		}
 		if (!CreateGlobalVars(scri.get())) {
 			return false;
 		}
@@ -1406,7 +1402,7 @@ bool ccInstance::_Create(PScript scri, ccInstance *joined) {
 	exports = new RuntimeScriptValue[scri->numexports];
 
 	// find the real address of the exports
-	for (i = 0; i < scri->numexports; i++) {
+	for (int i = 0; i < scri->numexports; i++) {
 		int32_t etype = (scri->export_addr[i] >> 24L) & 0x000ff;
 		int32_t eaddr = (scri->export_addr[i] & 0x00ffffff);
 		if (etype == EXPORT_FUNCTION) {
@@ -1435,7 +1431,7 @@ bool ccInstance::_Create(PScript scri, ccInstance *joined) {
 
 	if ((scri->instances == 1) && (ccGetOption(SCOPT_AUTOIMPORT) != 0)) {
 		// import all the exported stuff from this script
-		for (i = 0; i < scri->numexports; i++) {
+		for (int i = 0; i < scri->numexports; i++) {
 			if (!ccAddExternalScriptSymbol(scri->exports[i], exports[i], this)) {
 				cc_error("Export table overflow at '%s'", scri->exports[i]);
 				return false;
@@ -1482,35 +1478,35 @@ void ccInstance::Free() {
 }
 
 bool ccInstance::ResolveScriptImports(const ccScript *scri) {
-	// When the import is referenced in code, it's being addressed
-	// by it's index in the script imports array. That index is
-	// NOT unique and relative to script only.
-	// Script keeps information of used imports as an array of
-	// names.
-	// To allow real-time import use we should put resolved imports
-	// to the array keeping the order of their names in script's
-	// array of names.
-
-	// resolve all imports referenced in the script
+	// Script keeps the information of what imports are used as an array of names.
+	// When an import is referenced in the code, it's addressed by its index in this
+	// array. Different scripts have differing arrays of imports; indexes
+	// into 'imports[]' are NOT unique and relative to the respective script only.
+	// To allow real-time import use, the sequence of imports in 'imports[]'
+	// and 'resolved_imports[]' should not be modified.
+
 	numimports = scri->numimports;
 	if (numimports == 0) {
+		// [PGB] AFAICS there's nothing wrong with not having any imports, and
+		// it doesn't lead to trouble. However, if it turns out that we do need
+		// to return 'false' here, we should also report why with a 'Debug::Printf()' call.
 		resolved_imports = nullptr;
-		return false;
+		return true;
 	}
 
 	resolved_imports = new int[numimports];
-	int errors = 0, last_err_idx = 0;
-	for (int i = 0; i < scri->numimports; ++i) {
-		if (scri->imports[i] == nullptr) {
-			resolved_imports[i] = -1;
+	int errors = 0, last_err_idx;
+	for (int import_idx = 0; import_idx < scri->numimports; ++import_idx) {
+		if (scri->imports[import_idx] == nullptr) {
+			resolved_imports[import_idx] = -1;
 			continue;
 		}
 
-		resolved_imports[i] = _GP(simp).get_index_of(scri->imports[i]);
-		if (resolved_imports[i] < 0) {
-			Debug::Printf(kDbgMsg_Error, "unresolved import '%s' in '%s'", scri->imports[i], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
+		resolved_imports[import_idx] = _GP(simp).get_index_of(scri->imports[import_idx]);
+		if (resolved_imports[import_idx] < 0) {
+			Debug::Printf(kDbgMsg_Error, "unresolved import '%s' in '%s'", scri->imports[import_idx], scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>");
 			errors++;
-			last_err_idx = i;
+			last_err_idx = import_idx;
 		}
 	}
 
@@ -1519,6 +1515,7 @@ bool ccInstance::ResolveScriptImports(const ccScript *scri) {
 			scri->numSections > 0 ? scri->sectionNames[0] : "<unknown>",
 			errors,
 			scri->imports[last_err_idx]);
+
 	return errors == 0;
 }
 
@@ -1654,26 +1651,8 @@ bool ccInstance::CreateRuntimeCodeFixups(const ccScript *scri) {
 		case FIXUP_FUNCTION:
 		case FIXUP_STRING:
 		case FIXUP_STACK:
-			break; // do nothing yet
 		case FIXUP_IMPORT:
-			// we do not need to save import's address now when we have
-			// resolved imports kept so far as instance exists, but we
-			// must fixup the following instruction in certain case
-		{
-			int import_index = resolved_imports[code[fixup]];
-			const ScriptImport *import = _GP(simp).getByIndex(import_index);
-			if (!import) {
-				cc_error_fixups(scri, fixup, "cannot resolve import (bytecode pos %d, key %d)", fixup, import_index);
-				return false;
-			}
-			code[fixup] = import_index;
-			// If the call is to another script function next CALLEXT
-			// must be replaced with CALLAS
-			if (import->InstancePtr != nullptr && (code[fixup + 1] & INSTANCE_ID_REMOVEMASK) == SCMD_CALLEXT) {
-				code[fixup + 1] = SCMD_CALLAS | (import->InstancePtr->loadedInstanceId << INSTANCE_ID_SHIFT);
-			}
-		}
-		break;
+			break; // do nothing yet
 		default:
 			cc_error_fixups(scri, (size_t)-1, "unknown fixup type: %d (fixup num %d)", scri->fixuptypes[i], i);
 			return false;
@@ -1683,6 +1662,27 @@ bool ccInstance::CreateRuntimeCodeFixups(const ccScript *scri) {
 	return true;
 }
 
+bool ccInstance::ResolveImportFixups(const ccScript *scri) {
+	for (int fixup_idx = 0; fixup_idx < scri->numfixups; ++fixup_idx) {
+		if (scri->fixuptypes[fixup_idx] != FIXUP_IMPORT)
+			continue;
+
+		int32_t const fixup = scri->fixups[fixup_idx];
+		int const import_index = resolved_imports[code[fixup]];
+		ScriptImport const *import = _GP(simp).getByIndex(import_index);
+		if (!import) {
+			cc_error_fixups(scri, fixup, "cannot resolve import (bytecode pos %d, key %d)", fixup, import_index);
+			return false;
+		}
+		code[fixup] = import_index;
+		// If the call is to another script function next CALLEXT
+		// must be replaced with CALLAS
+		if (import->InstancePtr != nullptr && (code[fixup + 1] & INSTANCE_ID_REMOVEMASK) == SCMD_CALLEXT)
+			code[fixup + 1] = SCMD_CALLAS | (import->InstancePtr->loadedInstanceId << INSTANCE_ID_SHIFT);
+	}
+	return true;
+}
+
 /*
 bool ccInstance::ReadOperation(ScriptOperation &op, int32_t at_pc)
 {
diff --git a/engines/ags/engine/script/cc_instance.h b/engines/ags/engine/script/cc_instance.h
index 8cc6b2fd1dd..3e8c24d6980 100644
--- a/engines/ags/engine/script/cc_instance.h
+++ b/engines/ags/engine/script/cc_instance.h
@@ -174,12 +174,19 @@ public:
 	// Tells whether this instance is in the process of executing the byte-code
 	bool    IsBeingRun() const;
 
+	// For each import, find the instance that corresponds to it and save it
+	// in resolved_imports[]. Return whether the function is successful
+	bool    ResolveScriptImports(const ccScript *scri);
+
+	// Using resolved_imports[], resolve the IMPORT fixups
+	// Also change CALLEXT op-codes to CALLAS when they pertain to a script instance 
+	bool    ResolveImportFixups(const ccScript *scri);
+
 protected:
 	bool    _Create(PScript scri, ccInstance *joined);
 	// free the memory associated with the instance
 	void    Free();
 
-	bool    ResolveScriptImports(const ccScript *scri);
 	bool    CreateGlobalVars(const ccScript *scri);
 	bool    AddGlobalVar(const ScriptVariable &glvar);
 	ScriptVariable *FindGlobalVar(int32_t var_addr);
diff --git a/engines/ags/engine/script/script.cpp b/engines/ags/engine/script/script.cpp
index bf30c1454f8..751970e98bf 100644
--- a/engines/ags/engine/script/script.cpp
+++ b/engines/ags/engine/script/script.cpp
@@ -196,32 +196,53 @@ int run_interaction_script(InteractionScripts *nint, int evnt, int chkAny, int i
 }
 
 int create_global_script() {
+	constexpr int kscript_create_error = -3;
+
 	ccSetOption(SCOPT_AUTOIMPORT, 1);
+
+	std::vector<ccInstance *> instances_for_resolving;
 	for (int kk = 0; kk < _G(numScriptModules); kk++) {
 		_GP(moduleInst)[kk] = ccInstance::CreateFromScript(_GP(scriptModules)[kk]);
 		if (_GP(moduleInst)[kk] == nullptr)
-			return -3;
-		// create a forked instance for rep_exec_always
-		_GP(moduleInstFork)[kk] = _GP(moduleInst)[kk]->Fork();
-		if (_GP(moduleInstFork)[kk] == nullptr)
-			return -3;
-
-		_GP(moduleRepExecAddr)[kk] = _GP(moduleInst)[kk]->GetSymbolAddress(REP_EXEC_NAME);
+			return kscript_create_error;
+		instances_for_resolving.push_back(_GP(moduleInst)[kk]);
 	}
+
 	_G(gameinst) = ccInstance::CreateFromScript(_GP(gamescript));
 	if (_G(gameinst) == nullptr)
-		return -3;
-	// create a forked instance for rep_exec_always
-	_G(gameinstFork) = _G(gameinst)->Fork();
-	if (_G(gameinstFork) == nullptr)
-		return -3;
+		return kscript_create_error;
+	instances_for_resolving.push_back(_G(gameinst));
 
 	if (_GP(dialogScriptsScript) != nullptr) {
 		_G(dialogScriptsInst) = ccInstance::CreateFromScript(_GP(dialogScriptsScript));
 		if (_G(dialogScriptsInst) == nullptr)
-			return -3;
+			return kscript_create_error;
+		instances_for_resolving.push_back(_G(dialogScriptsInst));
 	}
 
+	// Resolve the script imports after all the scripts have been loaded 
+	for (size_t instance_idx = 0; instance_idx < instances_for_resolving.size(); instance_idx++) {
+		auto inst = instances_for_resolving[instance_idx];
+		if (!inst->ResolveScriptImports(inst->instanceof.get()))
+			return kscript_create_error;
+		if (!inst->ResolveImportFixups(inst->instanceof.get()))
+			return kscript_create_error;
+	}
+
+	// Create the forks for 'repeatedly_execute_always' after resolving
+	// because they copy their respective originals including the resolve information
+	for (int module_idx = 0; module_idx < _G(numScriptModules); module_idx++) {
+		_GP(moduleInstFork)[module_idx] = _GP(moduleInst)[module_idx]->Fork();
+		if (_GP(moduleInstFork)[module_idx] == nullptr)
+			return kscript_create_error;
+
+		_GP(moduleRepExecAddr)[module_idx] = _GP(moduleInst)[module_idx]->GetSymbolAddress(REP_EXEC_NAME);
+	}
+
+	_G(gameinstFork) = _G(gameinst)->Fork();
+	if (_G(gameinstFork) == nullptr)
+		return kscript_create_error;
+
 	ccSetOption(SCOPT_AUTOIMPORT, 0);
 	return 0;
 }


Commit: 36c4da51f3f9207bedf9b464a9d3897a4629c54c
    https://github.com/scummvm/scummvm/commit/36c4da51f3f9207bedf9b464a9d3897a4629c54c
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2022-03-29T21:44:09-07:00

Commit Message:
AGS: Fixed room script linking errors were not reported fully

>From upstream c858b18f1121ab777cb4bb4d7122821ca54b6e63

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


diff --git a/engines/ags/engine/ac/room.cpp b/engines/ags/engine/ac/room.cpp
index b7fa01aea4a..5fee3073380 100644
--- a/engines/ags/engine/ac/room.cpp
+++ b/engines/ags/engine/ac/room.cpp
@@ -992,20 +992,19 @@ void compile_room_script() {
 	_G(ccError) = 0;
 
 	_G(roominst) = ccInstance::CreateFromScript(_GP(thisroom).CompiledScript);
-
 	if ((_G(ccError) != 0) || (_G(roominst) == nullptr)) {
-		quitprintf("Unable to create local script: %s", _G(ccErrorString).GetCStr());
+		quitprintf("Unable to create local script:\n%s", _G(ccErrorString).GetCStr());
 	}
 
 	if (!_G(roominst)->ResolveScriptImports(_G(roominst)->instanceof.get()))
-		quitprintf("Unable to resolve imports in room script");
+		quitprintf("Unable to resolve imports in room script:\n%s", _G(ccErrorString).GetCStr());
 
 	if (!_G(roominst)->ResolveImportFixups(_G(roominst)->instanceof.get()))
-		quitprintf("Unable to resolve import fixups in room script");
+		quitprintf("Unable to resolve import fixups in room script:\n%s", _G(ccErrorString).GetCStr());
 
 	_G(roominstFork) = _G(roominst)->Fork();
 	if (_G(roominstFork) == nullptr)
-		quitprintf("Unable to create forked room instance: %s", _G(ccErrorString).GetCStr());
+		quitprintf("Unable to create forked room instance:\n%s", _G(ccErrorString).GetCStr());
 
 	_GP(repExecAlways).roomHasFunction = true;
 	_GP(lateRepExecAlways).roomHasFunction = true;


Commit: 9be6d156df81657f3ea56557f202bf9f88268ab8
    https://github.com/scummvm/scummvm/commit/9be6d156df81657f3ea56557f202bf9f88268ab8
Author: Paul Gilbert (dreammaster at scummvm.org)
Date: 2022-03-29T21:48:18-07:00

Commit Message:
AGS: Updated build version (3.6.0.12)

>From upstream 0fe6c546c39a84eeee99bd8a7f8f07fd63449821

Changed paths:
    engines/ags/shared/core/def_version.h


diff --git a/engines/ags/shared/core/def_version.h b/engines/ags/shared/core/def_version.h
index 048b4e44617..418939c1a5b 100644
--- a/engines/ags/shared/core/def_version.h
+++ b/engines/ags/shared/core/def_version.h
@@ -22,9 +22,9 @@
 #ifndef AGS_SHARED_CORE_DEFVERSION_H
 #define AGS_SHARED_CORE_DEFVERSION_H
 
-#define ACI_VERSION_STR      "3.6.0.11"
+#define ACI_VERSION_STR      "3.6.0.12"
 #if defined (RC_INVOKED) // for MSVC resource compiler
-#define ACI_VERSION_MSRC_DEF  3.6.0.11
+#define ACI_VERSION_MSRC_DEF  3.6.0.12
 #endif
 
 #define SPECIAL_VERSION ""




More information about the Scummvm-git-logs mailing list