Engine: fixed script linking/unlinking logic, fix restoring a save

Engine/ac/game.cpp

@@ -501,8 +501,7 @@ void unload_game()
     ccUnregisterAllObjects();
     pl_stop_plugins();
 
-    FreeAllScriptInstances();
-    FreeGlobalScripts();
+    FreeAllScripts();
 
     charextra.clear();
     mls.clear();

Engine/ac/global_game.cpp

@@ -88,7 +88,7 @@ extern RGB palette[256];
 void AbortGame()
 {
     // make sure scripts stop at the next step
-    cancel_all_scripts();
+    AbortAllScripts();
 }
 
 void restart_game() {

Engine/ac/room.cpp

@@ -307,7 +307,7 @@ void unload_old_room()
     for (uint32_t ff=0;ff<croom->numobj;ff++)
         objs[ff].moving = 0;
 
-    cancel_all_scripts();
+    AbortAllScripts();
     events.clear();  // cancel any pending room events
 
     if (roomBackgroundBmp != nullptr)
@@ -319,7 +319,7 @@ void unload_old_room()
     if (croom==nullptr) ;
     else if (roomscript) {
         save_room_data_segment();
-        FreeRoomScriptInstance();
+        FreeRoomScript();
     }
     else croom->tsdatasize=0;
     memset(&play.walkable_areas_on[0],1,MAX_WALK_AREAS);

Engine/game/game_init.cpp

@@ -527,7 +527,7 @@ HGameInitError InitGameState(const LoadedGameEntities &ents, GameDataVersion dat
     for (size_t i = 0; i < ents.ScriptModules.size(); ++i)
         scriptModules.push_back(std::shared_ptr<RuntimeScript>(RuntimeScript::Create(ents.ScriptModules[i].get(), "M")));
     AllocScriptModules();
-    if (create_global_script())
+    if (!LinkGlobalScripts())
         return new GameInitError(kGameInitErr_ScriptLinkFailed, cc_get_error().ErrorString);
 
     // Apply accessibility options, must be done last, because some

Engine/game/savegame.cpp

@@ -339,7 +339,8 @@ void DoBeforeRestore(PreservedParams &pp, SaveCmpSelection select_cmp)
         pp.ScMdDataSize[i] = scriptModules[i]->GetGlobalData().size();
     }
 
-    FreeAllScriptInstances();
+    // TODO: investigate if we actually have to do this when restoring a save
+    UnlinkAllScripts();
 
     // reset saved room states
     resetRoomStatuses();
@@ -573,11 +574,10 @@ HSaveError DoAfterRestore(const PreservedParams &pp, RestoredData &r_data, SaveC
     // Re-export any missing audio channel script objects, e.g. if restoring old save
     export_missing_audiochans();
 
-    AllocScriptModules();
-    if (create_global_script())
+    if (!LinkGlobalScripts())
     {
         return new SavegameError(kSvgErr_GameObjectInitFailed,
-            String::FromFormat("Unable to recreate global script: %s",
+            String::FromFormat("Failed to link global script: %s",
                 cc_get_error().ErrorString.GetCStr()));
     }
 

Engine/script/script.cpp

@@ -156,10 +156,8 @@ int run_interaction_script(const ObjectEvent &obj_evt, const InteractionEvents *
     return 0;
 }
 
-int create_global_script()
+bool LinkGlobalScripts()
 {
-    constexpr int kscript_create_error = -3; // FIXME: use global script error code
-
     ccSetOption(SCOPT_AUTOIMPORT, 1);
 
     // NOTE: this function assumes that the module lists have their elements preallocated!
@@ -188,7 +186,7 @@ int create_global_script()
     for (auto &inst : all_insts)
     {
         if (!inst->ResolveImports(simp))
-            return kscript_create_error;
+            return false;
     }
 
     // Record addresses for 'repeatedly_execute'
@@ -210,10 +208,10 @@ int create_global_script()
         }
     }
 
-    return 0;
+    return true;
 }
 
-void cancel_all_scripts()
+void AbortAllScripts()
 {
     scriptExecutor->Abort();
     num_scripts = 0;
@@ -507,6 +505,9 @@ bool RunScriptFunctionAuto(ScriptType sc_type, const ScriptFunctionRef &fn_ref,
 
 void AllocScriptModules()
 {
+    if (!scriptExecutor)
+        scriptExecutor = std::make_unique<ScriptExecutor>();
+
     // NOTE: this preallocation possibly required to safeguard some algorithms
     scriptModules.resize(numScriptModules);
     moduleRepExecAddr.resize(numScriptModules);
@@ -526,36 +527,34 @@ void AllocScriptModules()
     }
 }
 
-void FreeAllScriptInstances()
+void UnlinkAllScripts()
 {
-    FreeRoomScriptInstance();
-
+    if (roomscript)
+        roomscript->UnRegisterExports(simp);
     if (gamescript)
         gamescript->UnRegisterExports(simp);
     if (dialogScriptsScript)
         dialogScriptsScript->UnRegisterExports(simp);
     for (auto &module : scriptModules)
         module->UnRegisterExports(simp);
-
-    gamescript.reset();
-    dialogScriptsScript.reset();
-    scriptModules.clear();
 }
 
-void FreeRoomScriptInstance()
+void FreeRoomScript()
 {
     if (roomscript)
         roomscript->UnRegisterExports(simp);
-    roomscript.reset();
+    roomscript = nullptr;
 }
 
-void FreeGlobalScripts()
+void FreeAllScripts()
 {
-    numScriptModules = 0;
+    UnlinkAllScripts();
 
-    gamescript.reset();
+    roomscript = nullptr;
+    gamescript = nullptr;
+    dialogScriptsScript = nullptr;
     scriptModules.clear();
-    dialogScriptsScript.reset();
+    numScriptModules = 0;
 
     repExecAlways.ModuleHasFunction.clear();
     lateRepExecAlways.ModuleHasFunction.clear();
@@ -567,6 +566,8 @@ void FreeGlobalScripts()
     runDialogOptionTextInputHandlerFunc.ModuleHasFunction.clear();
     runDialogOptionRepExecFunc.ModuleHasFunction.clear();
     runDialogOptionCloseFunc.ModuleHasFunction.clear();
+
+    scriptExecutor = nullptr;
 }
 
 //=============================================================================
@@ -626,14 +627,14 @@ void post_script_cleanup()
                 curscript->QueueAction(PostScriptAction(ePSANewRoom, data1, "NewRoom"));
             break;
         case ePSARestoreGame:
-            cancel_all_scripts();
+            AbortAllScripts();
             try_restore_save(data1);
             return;
         case ePSARestoreGameDialog:
             restore_game_dialog2(data1 & 0xFFFF, (data1 >> 16));
             return;
         case ePSARunAGSGame:
-            cancel_all_scripts();
+            AbortAllScripts();
             load_new_game = data1;
             return;
         case ePSARunDialog:
@@ -650,7 +651,7 @@ void post_script_cleanup()
             set_dialog_result_stop();
             break;
         case ePSARestartGame:
-            cancel_all_scripts();
+            AbortAllScripts();
             restart_game();
             return;
         case ePSASaveGame:

Engine/script/script.h

@@ -99,9 +99,6 @@ void    run_function_on_non_blocking_thread(NonBlockingScriptFunction* funcToRun
 int     run_interaction_script(const ObjectEvent &obj_evt, const InteractionEvents *nint, int evnt, int chkAny = -1);
 void    run_unhandled_event(const ObjectEvent &obj_evt, int evnt);
 
-int     create_global_script();
-void    cancel_all_scripts();
-
 enum RunScFuncResult
 {
     kScFnRes_Done = 0,
@@ -142,13 +139,18 @@ bool   RunScriptFunctionAuto(ScriptType sc_type, const ScriptFunctionRef &fn_ref
 
 // Preallocates script module instances
 void    AllocScriptModules();
-// Delete all the script instance objects
-void    FreeAllScriptInstances();
+// Link all script modules into a single program,
+// registers script exports in a global symbol table
+bool    LinkGlobalScripts();
+// Cancel any running scripts
+void    AbortAllScripts();
+// Unlinks scripts, unregister script exports
+void    UnlinkAllScripts();
 // Delete only the current room script instance
-void    FreeRoomScriptInstance();
+void    FreeRoomScript();
 // Deletes all the global scripts and modules;
 // this frees all of the bytecode and runtime script memory.
-void    FreeGlobalScripts();
+void    FreeAllScripts();
 
 //=============================================================================