Integrate all changes made during A Highland Song development. See description for more details.

joethephish · joethephish · commit 4864a8667c5f · 2024-05-22T12:48:12.000+01:00
- Choices were storing thread references in a non-safe way, meaning that it could cause corruption during a background save. Not sure why we didn't see this issue on Heaven's Vault!
 - Move variable observation events to be sent out at the last second, since they may trigger new ink to be run (this was primarily to prevent ForceCompleteAsync not to start running new stuff unnecessarily)
 - More resilience when loading saves. Specifically, if content has shifted around, then a container at a specific indexed address may no longer be a container, which caused ink to crash
 - Support for using Continue() to completely a previously active ContinueAsync(). Not sure how this was ever working before on HV 🤪
 - Other minor improvements and convenience stuff
diff --git a/ink-engine-runtime/CallStack.cs b/ink-engine-runtime/CallStack.cs
@@ -342,6 +342,7 @@ public void Pop(PushPopType? type = null)
         // Get variable value, dereferencing a variable pointer if necessary
         public Runtime.Object GetTemporaryVariableWithName(string name, int contextIndex = -1)
         {
+            // contextIndex 0 means global, so index is actually 1-based
             if (contextIndex == -1)
                 contextIndex = currentElementIndex+1;
             
diff --git a/ink-engine-runtime/Choice.cs b/ink-engine-runtime/Choice.cs
@@ -51,6 +51,18 @@ public string pathStringOnChoice {
         public Choice()
         {
         }
+
+        public Choice Clone() {
+            var copy = new Choice();
+            copy.text = text;
+            copy.sourcePath = sourcePath;
+            copy.index = index;
+            copy.targetPath = targetPath;
+            copy.originalThreadIndex = originalThreadIndex;
+            copy.isInvisibleDefault = isInvisibleDefault;
+            if( threadAtGeneration != null ) copy.threadAtGeneration = threadAtGeneration.Copy();
+            return copy;
+        }
 	}
 }
 
diff --git a/ink-engine-runtime/Container.cs b/ink-engine-runtime/Container.cs
@@ -254,8 +254,18 @@ public SearchResult ContentAtPath(Path path, int partialPathStart = 0, int parti
                     break;
                 } 
 
+                // Are we about to loop into another container?
+                // Is the object a container as expected? It might
+                // no longer be if the content has shuffled around, so what
+                // was originally a container no longer is.
+                var nextContainer = foundObj as Container;
+                if( i < partialPathLength-1 && nextContainer == null ) {
+                    result.approximate = true;
+                    break;
+                }
+
                 currentObj = foundObj;
-                currentContainer = foundObj as Container;
+                currentContainer = nextContainer;
             }
 
             result.obj = currentObj;
diff --git a/ink-engine-runtime/InkList.cs b/ink-engine-runtime/InkList.cs
@@ -75,15 +75,24 @@ public override string ToString ()
         /// </summary>
         public override bool Equals (object obj)
         {
-            if (obj is InkListItem) {
-                var otherItem = (InkListItem)obj;
-                return otherItem.itemName   == itemName 
-                    && otherItem.originName == originName;
-            }
-
+            if (obj is InkListItem) 
+                return Equals((InkListItem)obj);
             return false;
         }
 
+        public bool Equals (InkListItem otherItem)
+        {
+            return otherItem.itemName == itemName && otherItem.originName == originName;
+        }
+
+        public static bool operator == (InkListItem left, InkListItem right) {
+            return left.Equals(right);
+        }
+    
+        public static bool operator != (InkListItem left, InkListItem right) {
+            return !(left == right);
+        }
+
         /// <summary>
         /// Get the hashcode for an item.
         /// </summary>
@@ -154,6 +163,8 @@ public InkList (KeyValuePair<InkListItem, int> singleElement)
 		/// <param name="itemKey">Item key.</param>
 		/// <param name="originStory">Origin story.</param>
 		public static InkList FromString(string myListItem, Story originStory) {
+            if (string.IsNullOrEmpty(myListItem))
+                return new InkList();
 			var listValue = originStory.listDefinitions.FindSingleItemListWithName (myListItem);
 			if (listValue)
 				return new InkList (listValue.value);
@@ -193,30 +204,44 @@ public void AddItem (InkListItem item)
         /// <summary>
         /// Adds the given item to the ink list, attempting to find the origin list definition that it belongs to.
         /// The item must therefore come from a list definition that is already "known" to this list, so that the
-        /// item's value can be looked up. By "known", we mean that it already has items in it from that source, or
+        /// item's value can be looked up.
+        /// By "known", we mean that it already has items in it from that source, or
         /// it did at one point - it can't be a completely fresh empty list, or a list that only contains items from
         /// a different list definition.
+        /// You can also provide the Story object, so in the case of an unknown element, it can be created fresh
         /// </summary>
-        public void AddItem (string itemName)
+        public void AddItem(string itemName, Story storyObject = null)
         {
             ListDefinition foundListDef = null;
 
-            foreach (var origin in origins) {
-                if (origin.ContainsItemWithName (itemName)) {
-                    if (foundListDef != null) {
-                        throw new System.Exception ("Could not add the item " + itemName + " to this list because it could come from either " + origin.name + " or " + foundListDef.name);
-                    } else {
-                        foundListDef = origin;
+            if (origins != null) { 
+                foreach (var origin in origins) {
+                    if (origin.ContainsItemWithName(itemName)) {
+                        if (foundListDef != null) {
+                            throw new System.Exception("Could not add the item " + itemName + " to this list because it could come from either " + origin.name + " or " + foundListDef.name);
+                        } else {
+                            foundListDef = origin;
+                        }
                     }
                 }
             }
 
             if (foundListDef == null)
-                throw new System.Exception ("Could not add the item " + itemName + " to this list because it isn't known to any list definitions previously associated with this list.");
-
-            var item = new InkListItem (foundListDef.name, itemName);
-            var itemVal = foundListDef.ValueForItem(item);
-            this [item] = itemVal;
+            {
+                if (storyObject == null)
+                    throw new System.Exception("Could not add the item " + itemName + " to this list because it isn't known to any list definitions previously associated with this list, and no ink Story object was provided to create it from.");
+                else
+                {
+                    var newItem = FromString(itemName, storyObject).orderedItems[0];
+                    this[newItem.Key] = newItem.Value;
+                }
+            }
+            else
+            {
+                var item = new InkListItem(foundListDef.name, itemName);
+                var itemVal = foundListDef.ValueForItem(item);
+                this[item] = itemVal;
+            }
         }
 
         /// <summary>
@@ -591,6 +616,17 @@ List<KeyValuePair<InkListItem, int>> orderedItems {
             }
         }
 
+        /// <summary>
+        /// If you have an InkList that's known to have one single item, this is a convenient way to get it.
+        /// </summary>
+        public InkListItem singleItem {
+            get {
+                foreach(var item in this)
+                    return item.Key;
+                return default;
+            }
+        }
+
         /// <summary>
         /// Returns a string in the form "a, b, c" with the names of the items in the list, without
         /// the origin list definition names. Equivalent to writing {list} in ink.
diff --git a/ink-engine-runtime/ListDefinitionsOrigin.cs b/ink-engine-runtime/ListDefinitionsOrigin.cs
@@ -43,7 +43,8 @@ public bool TryListGetDefinition (string name, out ListDefinition def)
         public ListValue FindSingleItemListWithName (string name)
         {
 			ListValue val = null;
-			_allUnambiguousListValueCache.TryGetValue(name, out val);
+            if (!string.IsNullOrWhiteSpace(name))
+			    _allUnambiguousListValueCache.TryGetValue(name, out val);
 			return val;
         }
 
diff --git a/ink-engine-runtime/NativeFunctionCall.cs b/ink-engine-runtime/NativeFunctionCall.cs
@@ -96,7 +96,7 @@ public Runtime.Object Call(List<Runtime.Object> parameters)
             bool hasList = false;
             foreach (var p in parameters) {
                 if (p is 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?");
+                    throw new StoryException ("Attempting to perform "+this.name+" on a void value. Did you forget to 'return' a value from a function you called here?");
                 if (p is ListValue)
                     hasList = true;
             }
diff --git a/ink-engine-runtime/SimpleJson.cs b/ink-engine-runtime/SimpleJson.cs
@@ -314,6 +314,16 @@ public Writer(Stream stream)
                 _writer = new System.IO.StreamWriter(stream, Encoding.UTF8);
             }
 
+            public void Clear()
+            {
+                var stringWriter = _writer as StringWriter;
+                if( stringWriter == null ) {
+                    throw new NotSupportedException("Writer.Clear() is only supported for the StringWriter variant, not for streams");
+                }
+                
+                stringWriter.GetStringBuilder().Clear();
+            }
+
             public void WriteObject(Action<Writer> inner)
             {
                 WriteObjectStart();
diff --git a/ink-engine-runtime/Story.cs b/ink-engine-runtime/Story.cs
@@ -447,7 +447,12 @@ void ContinueInternal (float millisecsLimitAsync = 0)
                 // In this case, we only want to batch observe variable changes
                 // for the outermost call.
                 if (_recursiveContinueCount == 1)
-                    _state.variablesState.batchObservingVariableChanges = true;
+                    _state.variablesState.StartVariableObservation();
+            }
+
+            // Async was previously active, but now we want to finish synchronously
+            else if( _asyncContinueActive && !isAsyncTimeLimited ) {
+                _asyncContinueActive = false;
             }
 
             // Start timing
@@ -477,6 +482,8 @@ void ContinueInternal (float millisecsLimitAsync = 0)
 
             durationStopwatch.Stop ();
 
+            Dictionary<string, Object> changedVariablesToObserve = null;
+
             // 4 outcomes:
             //  - got newline (so finished this line of text)
             //  - can't continue (e.g. choices or ending)
@@ -512,7 +519,7 @@ void ContinueInternal (float millisecsLimitAsync = 0)
                 _sawLookaheadUnsafeFunctionAfterNewline = false;
 
                 if (_recursiveContinueCount == 1)
-                    _state.variablesState.batchObservingVariableChanges = false;
+                    changedVariablesToObserve = _state.variablesState.CompleteVariableObservation();
 
                 _asyncContinueActive = false;
                 if(onDidContinue != null) onDidContinue();
@@ -573,6 +580,11 @@ void ContinueInternal (float millisecsLimitAsync = 0)
                     throw new StoryException(sb.ToString());
                 }
             }
+
+            // Send out variable observation events at the last second, since it might trigger new ink to be run
+            if( changedVariablesToObserve != null && changedVariablesToObserve.Count > 0 ) {
+                _state.variablesState.NotifyObservers(changedVariablesToObserve);
+            }
         }
 
         bool ContinueSingleStep ()
@@ -776,7 +788,7 @@ public Pointer PointerAtPath (Path path)
         void StateSnapshot()
         {
             _stateSnapshotAtLastNewline = _state;
-            _state = _state.CopyAndStartPatching();
+            _state = _state.CopyAndStartPatching(forBackgroundSave:false);
         }
 
         void RestoreStateSnapshot()
@@ -828,7 +840,7 @@ public StoryState CopyStateForBackgroundThreadSave()
             IfAsyncWeCant("start saving on a background thread");
             if (_asyncSaving) throw new System.Exception("Story is already in background saving mode, can't call CopyStateForBackgroundThreadSave again!");
             var stateToSave = _state;
-            _state = _state.CopyAndStartPatching();
+            _state = _state.CopyAndStartPatching(forBackgroundSave:true);
             _asyncSaving = true;
             return stateToSave;
         }
@@ -1912,6 +1924,10 @@ public Runtime.Object EvaluateExpression(Runtime.Container exprContainer)
         /// </summary>
         public bool allowExternalFunctionFallbacks { get; set; }
         
+        /// <summary>
+        /// Try to get a reference to an external function. This was originally exposed for the
+        /// Ink-Unity integration plugin so that the inspector can list them.
+        /// </summary>
         public bool TryGetExternalFunction(string functionName, out ExternalFunction externalFunction) {
             ExternalFunctionDef externalFunctionDef;
             if(_externals.TryGetValue (functionName, out externalFunctionDef)) {
@@ -2528,7 +2544,6 @@ void VariableStateDidChangeEvent(string variableName, Runtime.Object newValueObj
             
             VariableObserver observers = null;
             if (_variableObservers.TryGetValue (variableName, out observers)) {
-
                 if (!(newValueObj is Value)) {
                     throw new System.Exception ("Tried to get the value of a variable that isn't a standard type");
                 }
diff --git a/ink-engine-runtime/StoryState.cs b/ink-engine-runtime/StoryState.cs
@@ -231,6 +231,21 @@ public string currentPathString {
             }
         }
 
+        /// <summary>
+        /// Get the previous state of currentPathString, which can be helpful
+        /// for finding out where the story was before it ended (when the path
+        /// string becomes null)
+        /// </summary>
+        public string previousPathString {
+            get {
+                var pointer = previousPointer;
+                if (pointer.isNull)
+                    return null;
+                else
+                    return pointer.path.ToString();
+            }
+        }
+
         public Runtime.Pointer currentPointer {
             get {
                 return callStack.currentElement.currentPointer;
@@ -530,7 +545,7 @@ internal void RemoveFlow_Internal(string flowName)
         // Runtime.Objects are treated as immutable after they've been set up.
         // (e.g. we don't edit a Runtime.StringValue after it's been created an added.)
         // I wonder if there's a sensible way to enforce that..??
-        public StoryState CopyAndStartPatching()
+        public StoryState CopyAndStartPatching(bool forBackgroundSave)
         {
             var copy = new StoryState(story);
 
@@ -540,10 +555,21 @@ public StoryState CopyAndStartPatching()
             // If the patch is applied, then this new flow will replace the old one in _namedFlows
             copy._currentFlow.name = _currentFlow.name;
             copy._currentFlow.callStack = new CallStack (_currentFlow.callStack);
-            copy._currentFlow.currentChoices.AddRange(_currentFlow.currentChoices);
             copy._currentFlow.outputStream.AddRange(_currentFlow.outputStream);
             copy.OutputStreamDirty();
 
+            // When background saving we need to make copies of choices since they each have
+            // a snapshot of the thread at the time of generation since the game could progress
+            // significantly and threads modified during the save process.
+            // However, when doing internal saving and restoring of snapshots this isn't an issue,
+            // and we can simply ref-copy the choices with their existing threads.
+            if( forBackgroundSave ) {
+                foreach(var choice in _currentFlow.currentChoices)
+                    copy._currentFlow.currentChoices.Add(choice.Clone());
+            } else {
+                copy._currentFlow.currentChoices.AddRange(_currentFlow.currentChoices);
+            }
+
             // The copy of the state has its own copy of the named flows dictionary,
             // except with the current flow replaced with the copy above
             // (Assuming we're in multi-flow mode at all. If we're not then
diff --git a/ink-engine-runtime/VariablesState.cs b/ink-engine-runtime/VariablesState.cs

Original file line number	Diff line number	Diff line change
`@@ -342,6 +342,7 @@ public void Pop(PushPopType? type = null)`
`342`	`342`	`// Get variable value, dereferencing a variable pointer if necessary`
`343`	`343`	`public Runtime.Object GetTemporaryVariableWithName(string name, int contextIndex = -1)`
`344`	`344`	`{`
	`345`	`+ // contextIndex 0 means global, so index is actually 1-based`
`345`	`346`	`if (contextIndex == -1)`
`346`	`347`	`contextIndex = currentElementIndex+1;`
`347`	`348`
Original file line number	Diff line number	Diff line change
`@@ -51,6 +51,18 @@ public string pathStringOnChoice {`
`51`	`51`	`public Choice()`
`52`	`52`	`{`
`53`	`53`	`}`
	`54`	`+`
	`55`	`+ public Choice Clone() {`
	`56`	`+ var copy = new Choice();`
	`57`	`+ copy.text = text;`
	`58`	`+ copy.sourcePath = sourcePath;`
	`59`	`+ copy.index = index;`
	`60`	`+ copy.targetPath = targetPath;`
	`61`	`+ copy.originalThreadIndex = originalThreadIndex;`
	`62`	`+ copy.isInvisibleDefault = isInvisibleDefault;`
	`63`	`+ if( threadAtGeneration != null ) copy.threadAtGeneration = threadAtGeneration.Copy();`
	`64`	`+ return copy;`
	`65`	`+ }`
`54`	`66`	`}`
`55`	`67`	`}`
`56`	`68`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,8 @@ public bool TryListGetDefinition (string name, out ListDefinition def)`
`43`	`43`	`public ListValue FindSingleItemListWithName (string name)`
`44`	`44`	`{`
`45`	`45`	`ListValue val = null;`
`46`		`- _allUnambiguousListValueCache.TryGetValue(name, out val);`
	`46`	`+ if (!string.IsNullOrWhiteSpace(name))`
	`47`	`+ _allUnambiguousListValueCache.TryGetValue(name, out val);`
`47`	`48`	`return val;`
`48`	`49`	`}`
`49`	`50`
Original file line number	Diff line number	Diff line change
`@@ -96,7 +96,7 @@ public Runtime.Object Call(List<Runtime.Object> parameters)`
`96`	`96`	`bool hasList = false;`
`97`	`97`	`foreach (var p in parameters) {`
`98`	`98`	`if (p is Void)`
`99`		`- throw new StoryException ("Attempting to perform operation on a void value. Did you forget to 'return' a value from a function you called here?");`
	`99`	`+ throw new StoryException ("Attempting to perform "+this.name+" on a void value. Did you forget to 'return' a value from a function you called here?");`
`100`	`100`	`if (p is ListValue)`
`101`	`101`	`hasList = true;`
`102`	`102`	`}`