Fix to allow variable observers to run new ink more robustly by havin…

…g the callback fire later

src/engine/NativeFunctionCall.ts

@@ -94,7 +94,9 @@ export class NativeFunctionCall extends InkObject {
     for (let p of parameters) {
       if (p instanceof Void)
         throw new StoryException(
-          'Attempting to perform operation on a void value. Did you forget to "return" a value from a function you called here?'
+          "Attempting to perform " +
+            this.name +
+            ' on a void value. Did you forget to "return" a value from a function you called here?'
         );
       if (p instanceof ListValue) hasList = true;
     }

src/engine/Story.ts

@@ -370,7 +370,9 @@ export class Story extends InkObject {
       this._state.ResetOutput();
 
       if (this._recursiveContinueCount == 1)
-        this._state.variablesState.batchObservingVariableChanges = true;
+        this._state.variablesState.StartVariableObservation();
+    } else if (this._asyncContinueActive && !isAsyncTimeLimited) {
+      this._asyncContinueActive = false;
     }
 
     let durationStopwatch = new Stopwatch();
@@ -400,6 +402,8 @@ export class Story extends InkObject {
 
     durationStopwatch.Stop();
 
+    let changedVariablesToObserve: Map<string, any> | null = null;
+
     if (outputStreamEndsInNewline || !this.canContinue) {
       if (this._stateSnapshotAtLastNewline !== null) {
         this.RestoreStateSnapshot();
@@ -439,7 +443,7 @@ export class Story extends InkObject {
       this._sawLookaheadUnsafeFunctionAfterNewline = false;
 
       if (this._recursiveContinueCount == 1)
-        this._state.variablesState.batchObservingVariableChanges = false;
+        changedVariablesToObserve = this._state.variablesState.CompleteVariableObservation();
 
       this._asyncContinueActive = false;
       if (this.onDidContinue !== null) this.onDidContinue();
@@ -494,6 +498,12 @@ export class Story extends InkObject {
         throw new StoryException(sb.toString());
       }
     }
+    if (
+      changedVariablesToObserve != null &&
+      Object.keys(changedVariablesToObserve).length > 0
+    ) {
+      this._state.variablesState.NotifyObservers(changedVariablesToObserve);
+    }
   }
 
   public ContinueSingleStep() {
@@ -1836,6 +1846,20 @@ export class Story extends InkObject {
 
     let foundExternal = typeof funcDef !== "undefined";
 
+    if (
+      foundExternal &&
+      !funcDef!.lookAheadSafe &&
+      this._state.inStringEvaluation
+    ) {
+      this.Error(
+        "External function " +
+          funcName +
+          ' could not be called because 1) it wasn\'t marked as lookaheadSafe when BindExternalFunction was called and 2) the story is in the middle of string generation, either because choice text is being generated, or because you have ink like "hello {func()}". You can work around this by generating the result of your function into a temporary variable before the string or choice gets generated: ~ temp x = ' +
+          funcName +
+          "()"
+      );
+    }
+
     if (
       foundExternal &&
       !funcDef!.lookAheadSafe &&

src/engine/StoryState.ts

@@ -227,6 +227,18 @@ export class StoryState {
     }
   }
 
+  get previousPathString() {
+    let pointer = this.previousPointer;
+    if (pointer.isNull) {
+      return null;
+    } else {
+      if (pointer.path === null) {
+        return throwNullException("previousPointer.path");
+      }
+      return pointer.path.toString();
+    }
+  }
+
   get currentPointer() {
     return this.callStack.currentElement.currentPointer.copy();
   }

src/engine/VariablesState.ts

@@ -47,27 +47,35 @@ export class VariablesState extends VariablesStateAccessor<
 
   public patch: StatePatch | null = null;
 
-  get batchObservingVariableChanges() {
-    return this._batchObservingVariableChanges;
+  public StartVariableObservation() {
+    this._batchObservingVariableChanges = true;
+    this._changedVariablesForBatchObs = new Set();
   }
-  set batchObservingVariableChanges(value: boolean) {
-    this._batchObservingVariableChanges = value;
-    if (value) {
-      this._changedVariablesForBatchObs = new Set();
-    } else {
-      if (this._changedVariablesForBatchObs != null) {
-        for (let variableName of this._changedVariablesForBatchObs) {
-          let currentValue = this._globalVariables.get(variableName);
-          if (!currentValue) {
-            throwNullException("currentValue");
-          } else {
-            this.variableChangedEvent(variableName, currentValue);
-          }
-        }
 
-        this._changedVariablesForBatchObs = null;
+  public CompleteVariableObservation(): Map<string, any> {
+    this._batchObservingVariableChanges = false;
+    let changedVars = new Map<string, any>();
+    if (this._changedVariablesForBatchObs != null) {
+      for (let variableName of this._changedVariablesForBatchObs) {
+        let currentValue = this._globalVariables.get(variableName) as InkObject;
+        this.variableChangedEvent(variableName, currentValue);
+      }
+    }
+    // Patch may still be active - e.g. if we were in the middle of a background save
+    if (this.patch != null) {
+      for (let variableName of this.patch.changedVariables) {
+        let patchedVal = this.patch.TryGetGlobal(variableName, null);
+        if (patchedVal.exists) changedVars.set(variableName, patchedVal);
       }
     }
+    this._changedVariablesForBatchObs = null;
+    return changedVars;
+  }
+
+  public NotifyObservers(changedVars: Map<string, any>){
+    for (const [key, value] of changedVars) {
+      this.variableChangedEvent(key, value);
+    }
   }
 
   get callStack() {
@@ -77,8 +85,6 @@ export class VariablesState extends VariablesStateAccessor<
     this._callStack = callStack;
   }
 
-  private _batchObservingVariableChanges: boolean = false;
-
   // the original code uses a magic getter and setter for global variables,
   // allowing things like variableState['varname]. This is not quite possible
   // in js without a Proxy, so it is replaced with this $ function.
@@ -429,7 +435,7 @@ export class VariablesState extends VariablesStateAccessor<
       oldValue !== null &&
       value !== oldValue.result
     ) {
-      if (this.batchObservingVariableChanges) {
+      if (this._batchObservingVariableChanges) {
         if (this._changedVariablesForBatchObs === null) {
           return throwNullException("this._changedVariablesForBatchObs");
         }
@@ -495,4 +501,6 @@ export class VariablesState extends VariablesStateAccessor<
   private _callStack: CallStack;
   private _changedVariablesForBatchObs: Set<string> | null = new Set();
   private _listDefsOrigin: ListDefinitionsOrigin | null;
+
+  private _batchObservingVariableChanges: boolean = false;
 }

src/tests/inkfiles/original/lists/list_range.ink

@@ -6,6 +6,7 @@ VAR all = ()
 {all}
 {LIST_RANGE(all, 2, 3)}
 {LIST_RANGE(LIST_ALL(Numbers), Two, Six)}
+{LIST_RANGE(LIST_ALL(Numbers), Currency, Three)}
 {LIST_RANGE(LIST_ALL(Numbers), 2, Four)} // mix int and list
 {LIST_RANGE(LIST_ALL(Numbers), Two, 5)} // mix list and int
 {LIST_RANGE((Pizza, Pasta), -1, 100)} // allow out of range

src/tests/specs/ink/Lists.spec.ts

@@ -68,6 +68,7 @@ describe("Lists", () => {
       `Pound, Pizza, Euro, Pasta, Dollar, Curry, Paella
 Euro, Pasta, Dollar, Curry
 Two, Three, Four, Five, Six
+One, Two, Three
 Two, Three, Four
 Two, Three, Four, Five
 Pizza, Pasta