Merge pull request #195 from SHADE-DP/SP3-6-c-scripting

Fixed crash on launch due to script rebuilds and support undo-ing add/remove of scripts

Also restricted access to internal types that SHADE_Scripting should not be accessing.
Redo wonkiness has also been fixed

SHADE_Engine/src/Scripting/SHScriptEngine.cpp

@@ -17,6 +17,7 @@ of DigiPen Institute of Technology is prohibited.
 #include <fstream>      // std::fstream
 #include <filesystem>   // std::filesystem::canonical, std::filesystem::remove
 #include <memory>       // std::shared_ptr
+#include <thread>       // std::this_thread::sleep_for
 // Project Headers
 #include "Tools/SHLogger.h"
 #include "Tools/SHStringUtils.h"
@@ -25,7 +26,6 @@ of DigiPen Institute of Technology is prohibited.
 #include "Events/SHEventReceiver.h"
 #include "Events/SHEventManager.hpp"
 #include "Physics/SHPhysicsSystem.h"
-
 #include "Assets/SHAssetMacros.h"
 
 namespace SHADE
@@ -177,10 +177,10 @@ namespace SHADE
     }
 
     // Prepare directory (delete useless files)
-    deleteFolder(CSPROJ_DIR + "\\net5.0");
+    deleteFolder(CSPROJ_DIR + "/net5.0");
-    deleteFolder(CSPROJ_DIR + "\\ref");
+    deleteFolder(CSPROJ_DIR + "/ref");
-    deleteFolder(CSPROJ_DIR + "\\obj");
+    deleteFolder(CSPROJ_DIR + "/obj");
-    deleteFolder(CSPROJ_DIR + "\\bin");
+    deleteFolder(CSPROJ_DIR + "/bin");
 
     // Attempt to build the assembly
     std::ostringstream oss;
@@ -214,7 +214,10 @@ namespace SHADE
 
     // Clean up built files
     deleteFolder("./tmp");
-    deleteFolder(CSPROJ_DIR + "\\obj");
+    deleteFolder(CSPROJ_DIR + "/bin");
+    using namespace std::chrono_literals;
+    std::this_thread::sleep_for(50ms); // Not sure why this works but it prevents the folders from respawning
+    deleteFolder(CSPROJ_DIR + "/obj");
 
     // Read the build log and output to the console
     dumpBuildLog(BUILD_LOG_PATH);

SHADE_Managed/src/Components/Component.hxx

@@ -110,7 +110,7 @@ namespace SHADE
         /// <param name="c">Component to check.</param>
         static operator bool(BaseComponent^ c);
 
-      protected:
+      internal:
         /*-----------------------------------------------------------------------------*/
         /* Constructors                                                                */
         /*-----------------------------------------------------------------------------*/
@@ -193,7 +193,6 @@ namespace SHADE
         /// </exception>
         NativeComponent* GetNativeComponent();
 
-      protected:
         /*-----------------------------------------------------------------------------*/
         /* Constructors                                                                */
         /*-----------------------------------------------------------------------------*/

SHADE_Managed/src/Editor/Editor.cxx

@@ -79,7 +79,10 @@ namespace SHADE
                 if (SHEditorUI::Selectable(Convert::ToNative(type->Name)))
                 {
                     // Add the script
-                    ScriptStore::AddScriptViaName(entity, type->Name);
+                    Script^ script;
+                    ScriptStore::AddScriptViaNameWithRef(entity, type->Name, script);
+                    registerUndoScriptAddAction(entity, script);
+                    break;
                 }
             }
 
@@ -120,7 +123,7 @@ namespace SHADE
             SHEditorUI::Indent();
             {
                 // Right Click Menu
-                renderScriptContextMenu(entity, script);
+                renderScriptContextMenu(entity, script, index);
 
                 // Go through all fields and output them
                 auto fields = ReflectionUtilities::GetInstanceFields(script);
@@ -143,7 +146,7 @@ namespace SHADE
         }
         else
         {
-            renderScriptContextMenu(entity, script);
+            renderScriptContextMenu(entity, script, index);
         }
         SHEditorUI::PopID();
     }
@@ -336,7 +339,7 @@ namespace SHADE
         return false;
     }
 
-    void Editor::renderScriptContextMenu(Entity entity, Script^ script)
+    void Editor::renderScriptContextMenu(Entity entity, Script^ script, int scriptIndex)
     {
         // Right Click Menu
         if (SHEditorUI::BeginPopupContextItem("scriptContextMenu"))
@@ -345,6 +348,7 @@ namespace SHADE
             {
                 // Mark script for removal
                 ScriptStore::RemoveScript(entity, script);
+                registerUndoScriptRemoveAction(entity, script, scriptIndex);
             }
             SHEditorUI::EndPopup();
         }
@@ -392,6 +396,28 @@ namespace SHADE
         SHCommandManager::RegisterCommand(std::reinterpret_pointer_cast<SHBaseCommand>(std::make_shared<SHCLICommand>()));
     }
 
+    void Editor::registerUndoScriptAddAction(EntityID id, Script^ script)
+    {
+        if (script == nullptr)
+            return;
+
+        actionStack.Add(gcnew ScriptAddCommand(id, script));
+
+        // Inform the C++ Undo-Redo stack
+        SHCommandManager::RegisterCommand(std::reinterpret_pointer_cast<SHBaseCommand>(std::make_shared<SHCLICommand>()));
+    }
+
+    void Editor::registerUndoScriptRemoveAction(EntityID id, Script^ script, int originalIndex)
+    {
+        if (script == nullptr)
+            return;
+
+        actionStack.Add(gcnew ScriptRemoveCommand(id, script, originalIndex));
+
+        // Inform the C++ Undo-Redo stack
+        SHCommandManager::RegisterCommand(std::reinterpret_pointer_cast<SHBaseCommand>(std::make_shared<SHCLICommand>()));
+    }
+
     generic<typename Attribute>
     Attribute Editor::hasAttribute(System::Reflection::FieldInfo^ field)
     {

SHADE_Managed/src/Editor/Editor.hxx

@@ -69,7 +69,7 @@ namespace SHADE
         static UndoRedoStack actionStack;
 
         /*-----------------------------------------------------------------------------*/
-        /* Helper Functions                                                            */
+        /* Helper Functions - Inspector Rendering                                      */
         /*-----------------------------------------------------------------------------*/
         /// <summary>
         /// Renders a single specified Script's inspector.
@@ -170,25 +170,25 @@ namespace SHADE
         /// <returns>True if the field is modified.</returns>
         template<typename NativeType, typename ManagedType>
         static bool renderFieldEditorInternal(const std::string& fieldName, interior_ptr<ManagedType> managedValPtr, EditorFieldFunc<NativeType> fieldEditor, bool* isHovered, RangeAttribute^ rangeAttrib);
-
         /// <summary>
         /// Renders a context menu when right clicked for the scripts
         /// </summary>
         /// <param name="entity">The Entity to render the Scripts of.</param>
         /// <param name="script">The Script to render the inspector for.</param>
-        static void renderScriptContextMenu(Entity entity, Script^ script);
+        /// <param name="scriptIndex">Index at which the Script is stored.</param>
-        /// <summary>
+        static void renderScriptContextMenu(Entity entity, Script^ script, int scriptIndex);
-        /// Adds changes to a variable as an undo-able/redo-able action on the Undo-Redo
+        /*-----------------------------------------------------------------------------*/
-        /// stack.
+        /* Helper Functions - Undo                                                     */
-        /// </summary>
+        /*-----------------------------------------------------------------------------*/
-        /// <param name="object">The object that changes are applied to.</param>
-        /// <param name="field">The field that was changed.</param>
-        /// <param name="newData">New data to set.</param>
-        /// <param name="oldData">Data that was overriden.</param>
         static void registerUndoAction(System::Object^ object, System::Reflection::FieldInfo^ field, System::Object^ newData, System::Object^ oldData);
         static void registerUndoListChangeAction(System::Type^ type, System::Collections::IList^ list, int index, System::Object^ newData, System::Object^ oldData);
         static void registerUndoListAddAction(System::Type^ type, System::Collections::IList^ list, int index, System::Object^ data);
         static void registerUndoListRemoveAction(System::Type^ type, System::Collections::IList^ list, int index, System::Object^ data);
+        static void registerUndoScriptAddAction(EntityID id, Script^ script);
+        static void registerUndoScriptRemoveAction(EntityID id, Script^ script, int originalIndex);
+        /*-----------------------------------------------------------------------------*/
+        /* Helper Functions - Others                                                   */
+        /*-----------------------------------------------------------------------------*/
         /// <summary>
         /// Checks if a specific field has the specified attribute
         /// </summary>

SHADE_Managed/src/Editor/UndoRedoStack.cxx

@@ -21,6 +21,7 @@ of DigiPen Institute of Technology is prohibited.
 // Project Headers
 #include "Utility/Debug.hxx"
 #include "Utility/Convert.hxx"
+#include "Scripts/ScriptStore.hxx"
 
 namespace SHADE
 {
@@ -34,7 +35,8 @@ namespace SHADE
 
     bool UndoRedoStack::RedoActionPresent::get()
     {
-        return latestActionIndex >= 0 && latestActionIndex < commandStack->Count - 1;
+        const int REDO_ACTION_INDEX = latestActionIndex + 1;
+        return REDO_ACTION_INDEX >= 0 && REDO_ACTION_INDEX < commandStack->Count;
     }
     
     /*---------------------------------------------------------------------------------*/
@@ -69,8 +71,9 @@ namespace SHADE
     {
         if (!RedoActionPresent)
             return;
-
+        
-        ICommand^ cmd = commandStack[latestActionIndex];
+        const int REDO_ACTION_INDEX = latestActionIndex + 1;
+        ICommand^ cmd = commandStack[REDO_ACTION_INDEX];
         cmd->Execute();
         ++latestActionIndex;
     }
@@ -180,7 +183,7 @@ namespace SHADE
     }    
     
     /*---------------------------------------------------------------------------------*/
-    /* ListElementAddCommand - ICommand Functions                                      */
+    /* ListElementAddCommand - Constructor                                             */
     /*---------------------------------------------------------------------------------*/
     ListElementAddCommand::ListElementAddCommand(System::Collections::IList^ list, int addIndex, System::Object^ data)
     : list     { list     }
@@ -220,7 +223,7 @@ namespace SHADE
     }
 
     /*---------------------------------------------------------------------------------*/
-    /* ListElementRemoveCommand - ICommand Functions                                   */
+    /* ListElementRemoveCommand - Constructor                                          */
     /*---------------------------------------------------------------------------------*/
     ListElementRemoveCommand::ListElementRemoveCommand(System::Collections::IList^ list, int removeIndex, System::Object^ data)
     : list        { list        }
@@ -258,4 +261,59 @@ namespace SHADE
         // Not allowed
         return false;
     }
+    
+    /*---------------------------------------------------------------------------------*/
+    /* ScriptAddCommand - Constructor                                                  */
+    /*---------------------------------------------------------------------------------*/
+    ScriptAddCommand::ScriptAddCommand(EntityID id, Script^ script)
+    : entity      { id     }
+    , addedScript { script }
+    {}
+    
+    /*---------------------------------------------------------------------------------*/
+    /* ScriptAddCommand - ICommand Functions                                           */
+    /*---------------------------------------------------------------------------------*/
+    bool ScriptAddCommand::Execute()
+    {
+        return ScriptStore::AddScript(entity, addedScript) != nullptr;
+    }
+    
+    bool ScriptAddCommand::Unexceute()
+    {
+        return ScriptStore::RemoveScript(entity, addedScript);
+    }
+    
+    bool ScriptAddCommand::Merge(ICommand^)
+    {
+        // Not allowed
+        return false;
+    }
+    
+    /*---------------------------------------------------------------------------------*/
+    /* ScriptRemoveCommand - Constructor                                               */
+    /*---------------------------------------------------------------------------------*/
+    ScriptRemoveCommand::ScriptRemoveCommand(EntityID id, Script^ script, int index)
+    : entity        { id     }
+    , removedScript { script }
+    , originalIndex { index  }
+    {}
+    
+    /*---------------------------------------------------------------------------------*/
+    /* ScriptRemoveCommand - ICommand Functions                                        */
+    /*---------------------------------------------------------------------------------*/
+    bool ScriptRemoveCommand::Execute()
+    {
+        return ScriptStore::RemoveScript(entity, removedScript);
+    }
+    
+    bool ScriptRemoveCommand::Unexceute()
+    {
+        return ScriptStore::AddScript(entity, removedScript, originalIndex) != nullptr;
+    }
+    
+    bool ScriptRemoveCommand::Merge(ICommand^)
+    {
+        // Not allowed
+        return false;
+    }
 }

SHADE_Managed/src/Editor/UndoRedoStack.hxx

@@ -12,6 +12,7 @@ Reproduction or disclosure of this file or its contents without the prior writte
 of DigiPen Institute of Technology is prohibited.
 *//*************************************************************************************/
 #pragma once
+#include "Scripts/Script.hxx"
 
 namespace SHADE
 {
@@ -102,6 +103,35 @@ namespace SHADE
         System::Object^ data;
     };
 
+    private ref class ScriptAddCommand sealed : public ICommand
+    {
+      public:
+        ScriptAddCommand(EntityID id, Script^ script);
+
+        bool Execute() override;
+        bool Unexceute() override;
+        bool Merge(ICommand^ command) override;
+
+      private:
+        EntityID entity;
+        Script^  addedScript;
+    };
+
+    private ref class ScriptRemoveCommand sealed : public ICommand
+    {
+      public:
+        ScriptRemoveCommand(EntityID id, Script^ script, int index);
+
+        bool Execute() override;
+        bool Unexceute() override;
+        bool Merge(ICommand^ command) override;
+
+      private:
+        EntityID entity;
+        Script^  removedScript;
+        int      originalIndex;
+    };
+
     /// <summary>
     /// Class that is able to store a stack of actions that can be done and redone.
     /// </summary>

SHADE_Managed/src/Engine/GenericHandle.hxx

@@ -21,7 +21,7 @@ namespace SHADE
     /// <summary>
     /// Managed version of the generic Handle<void>.
     /// </summary>
-    public value struct GenericHandle
+    private value struct GenericHandle
     {
       public:
         /*-----------------------------------------------------------------------------*/

SHADE_Managed/src/Scripts/ScriptStore.cxx

@@ -38,6 +38,20 @@ namespace SHADE
     /*---------------------------------------------------------------------------------*/
     generic<typename T>
     T ScriptStore::AddScript(Entity entity)
+    {
+        // Create the script and add it in        
+        array<System::Object^>^ params = gcnew array<System::Object^>{GameObject(entity)};
+        Script^ script = safe_cast<Script^>(System::Activator::CreateInstance(T::typeid, params));
+
+        return safe_cast<T>(AddScript(entity, script));
+    }
+
+    Script^ ScriptStore::AddScript(Entity entity, Script^ script)
+    {
+        return AddScript(entity, script, System::Int32::MaxValue);
+    }
+
+    Script^ ScriptStore::AddScript(Entity entity, Script^ script, int index)
     {
         // Check if entity exists
         if (!EntityUtils::IsValid(entity))
@@ -57,15 +71,13 @@ namespace SHADE
             entityScriptList = scripts[entity];
         }
 
-        // Create the script and add it in        
+        // Add the script in
-        array<Object^>^ params = gcnew array<Object^>{GameObject(entity)};
+        entityScriptList->Insert(System::Math::Clamp(index, 0, entityScriptList->Count), script);
-        Script^ script = safe_cast<Script^>(System::Activator::CreateInstance(T::typeid, params));
-        entityScriptList->Add(script);
         awakeList.Add(script);
         startList.Add(script);
         script->OnAttached();
 
-        return safe_cast<T>(script);
+        return script;
     }
 
     bool ScriptStore::AddScriptViaName(Entity entity, System::String^ scriptName)
@@ -364,7 +376,10 @@ namespace SHADE
             }
         }
         startList.Clear();
-        startList.AddRange(%inactiveStartList);
+        for each (Script ^ script in startList)
+        {
+            startList.Add(script);
+        }
         inactiveStartList.Clear();
 
         SAFE_NATIVE_CALL_END_N("SHADE_Managed.ScriptStore")
@@ -373,9 +388,8 @@ namespace SHADE
     {
         SAFE_NATIVE_CALL_BEGIN
         // Clear the queue
-        while (disposalQueue.Count > 0)
+        for each (Script^ script in disposalQueue)
-        {
+        {;            
-            Script^ script = disposalQueue.Dequeue();            
             if (Application::IsPlaying)
             {
                 script->OnDestroy();
@@ -388,6 +402,7 @@ namespace SHADE
                 scripts.Remove(entity);
             }
         }
+        disposalQueue.Clear();
         SAFE_NATIVE_CALL_END_N("SHADE_Managed.ScriptStore")
     }
     void ScriptStore::Exit()
@@ -677,9 +692,9 @@ namespace SHADE
     void ScriptStore::removeScript(Script^ script)
     {
         // Prepare for disposal
-        disposalQueue.Enqueue(script);
+        disposalQueue.Add(script);
 
-        // Also remove it fromm awake and start queues if they were created but not initialised
+        // Also remove it from awake and start queues if they were created but not initialised
         awakeList.Remove(script);
         startList.Remove(script);
         script->OnDetached();
@@ -749,7 +764,8 @@ namespace SHADE
 
     void ScriptStore::getGenericMethods()
     {
-        addScriptMethod = ScriptStore::typeid->GetMethod("AddScript");
+        array<System::Type^>^ paramTypes = gcnew array<System::Type^>{ Entity::typeid };
+        addScriptMethod = ScriptStore::typeid->GetMethod("AddScript", paramTypes);
         if  (addScriptMethod == nullptr)
         {
             Debug::LogError("[ScriptStore] Failed to get MethodInfo of \"AddScript<T>()\". Adding of scripts from native code will fail.");

SHADE_Managed/src/Scripts/ScriptStore.hxx

@@ -25,7 +25,7 @@ namespace SHADE
     /// Responsible for managing all scripts attached to Entities as well as executing 
     /// all lifecycle functions of scripts.
     /// </summary>
-    public ref class ScriptStore abstract sealed
+    private ref class ScriptStore abstract sealed
     {
       public:
         /*-----------------------------------------------------------------------------*/
@@ -46,6 +46,27 @@ namespace SHADE
         generic<typename T> where T : ref class, Script
         static T AddScript(Entity entity);
         /// <summary>
+        /// Adds a specified pre-constructed Script to a specified Entity.
+        /// </summary>
+        /// <param name="entity">The entity to add a script to.</param>
+        /// <param name="script">The pre-constructed Script to add.</param>
+        /// <returns>Reference to the script added.</returns>
+        /// <exception cref="ArgumentException">
+        /// If the specified Entity is invalid.
+        /// </exception>
+        static Script^ AddScript(Entity entity, Script^ script);
+        /// <summary>
+        /// Adds a specified pre-constructed Script to a specified Entity.
+        /// </summary>
+        /// <param name="entity">The entity to add a script to.</param>
+        /// <param name="script">The pre-constructed Script to add.</param>
+        /// <param name="index">Location in the script list to add.</param>
+        /// <returns>Reference to the script added.</returns>
+        /// <exception cref="ArgumentException">
+        /// If the specified Entity is invalid.
+        /// </exception>
+        static Script^ AddScript(Entity entity, Script^ script, int index);
+        /// <summary>
         /// Adds a Script to a specified Entity.
         /// <br/>
         /// This function is meant for consumption from native code. If you are writing
@@ -281,16 +302,16 @@ namespace SHADE
         /*-----------------------------------------------------------------------------*/
         using ScriptList       = System::Collections::Generic::List<Script^>;
         using ScriptDictionary = System::Collections::Generic::Dictionary<Entity, ScriptList^>;
-        using ScriptQueue      = System::Collections::Generic::Queue<Script^>;
+        using ScriptSet        = System::Collections::Generic::HashSet<Script^>;
 
         /*-----------------------------------------------------------------------------*/
         /* Static Data Members                                                         */
         /*-----------------------------------------------------------------------------*/
         static ScriptDictionary                                          scripts;
-        static ScriptList                                                awakeList;
+        static ScriptSet                                                 awakeList;
-        static ScriptList                                                startList;
+        static ScriptSet                                                 startList;
-        static ScriptList                                                inactiveStartList;
+        static ScriptSet                                                 inactiveStartList;
-        static ScriptQueue                                               disposalQueue;
+        static ScriptSet                                                 disposalQueue;
         static System::Collections::Generic::IEnumerable<System::Type^>^ scriptTypeList;
         static System::Reflection::MethodInfo^                           addScriptMethod;
 

SHADE_Managed/src/Serialisation/SerialisationUtilities.h++

@@ -28,7 +28,6 @@ namespace SHADE
     template<typename FieldType>
     bool SerialisationUtilities::fieldInsertYaml(System::Reflection::FieldInfo^ fieldInfo, System::Object^ object, YAML::Node& fieldNode)
     {
-        Debug::Log(FieldType::typeid->Name);
         return varInsertYamlInternal<FieldType>(fieldInfo->GetValue(object), fieldNode);
     }
     template<typename FieldType>
@@ -123,16 +122,6 @@ namespace SHADE
                 return true;
             }
         }
-        else if constexpr (std::is_same_v<FieldType, System::Collections::IList>)
-        {
-            if (ReflectionUtilities::FieldIsList(fieldInfo))
-            {
-                System::Collections::IList^ iList = safe_cast<System::Collections::IList^>(object);
-                object = gcnew 
-                if (node.IsSequence() )
-
-            }
-        }
         else
         {
             if (object->GetType() == FieldType::typeid)