From 08afdfd4bdcc7388e964922532bb3bf226171b75 Mon Sep 17 00:00:00 2001
From: Clark Kromenaker <kromenak@gmail.com>
Date: Wed, 10 Aug 2022 15:55:02 -0700
Subject: [PATCH] Fixed Mac/Clang/Release crash related to std::function
 callback during SheepScript loading.

While testing the v0.0.6 release, I noticed the game crashing at a strange spot while loading SheepScript files. The crash appears to be related to destructing std::function objects.

Converting the offending code to use a simple function pointer appears to fix the problem. So, I'll do this for now. However, seems strange to me that std::function would not work correctly here...
---
 Source/Assets/AssetManager.cpp | 32 ++++++++++++++++++--------------
 Source/Assets/AssetManager.h   |  2 +-
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/Source/Assets/AssetManager.cpp b/Source/Assets/AssetManager.cpp
index 0f208f33..d341aba5 100644
--- a/Source/Assets/AssetManager.cpp
+++ b/Source/Assets/AssetManager.cpp
@@ -298,22 +298,26 @@ BSPLightmap* AssetManager::LoadBSPLightmap(const std::string& name)
     return LoadAsset<BSPLightmap>(SanitizeAssetName(name, ".MUL"), &mLoadedBSPLightmaps);
 }
 
-SheepScript* AssetManager::LoadSheep(const std::string& name)
+//TODO: For some reason, on Mac/Clang, when compiling in Release mode, using this as a Lambda with std::function causes a malloc error and crash.
+//TODO: I've converted it to a normal function with a function pointer for now...which seems to work. But why?
+SheepScript* LoadSheepFunc(const std::string& assetName, char* buffer, unsigned int bufferSize)
 {
-    // Sheep assets need more complex/custom creation login, provided in the create callback.
-    return LoadAsset<SheepScript>(SanitizeAssetName(name, ".SHP"), &mLoadedSheeps, [](const std::string& name, char* buffer, unsigned int bufferSize) {
+    // Determine whether this is a binary sheep asset.
+    if(SheepScript::IsSheepDataCompiled(buffer, bufferSize))
+    {
+        return new SheepScript(assetName, buffer, bufferSize);
+    }
 
-        // Determine whether this is a binary sheep asset.
-        if(SheepScript::IsSheepDataCompiled(buffer, bufferSize))
-        {
-            return new SheepScript(name, buffer, bufferSize);
-        }
+    // This doesn't appear to be a binary sheep file, so it might be a text sheep file.
+    // Let's try compiling it on-the-fly!
+    imstream stream(buffer, bufferSize);
+    return Services::GetSheep()->Compile(assetName, stream);
+}
 
-        // This doesn't appear to be a binary sheep file, so it might be a text sheep file.
-        // Let's try compiling it on-the-fly!
-        imstream stream(buffer, bufferSize);
-        return Services::GetSheep()->Compile(name, stream);
-    });
+SheepScript* AssetManager::LoadSheep(const std::string& name)
+{
+    // Sheep assets need more complex/custom creation login, provided in the create callback.
+    return LoadAsset<SheepScript>(SanitizeAssetName(name, ".SHP"), &mLoadedSheeps, &LoadSheepFunc);
 }
 
 Cursor* AssetManager::LoadCursor(const std::string& name)
@@ -428,7 +432,7 @@ std::string AssetManager::SanitizeAssetName(const std::string& assetName, const
 }
 
 template<class T>
-T* AssetManager::LoadAsset(const std::string& assetName, std::unordered_map_ci<std::string, T*>* cache, std::function<T* (const std::string&, char*, unsigned int)> createFunc, bool deleteBuffer)
+T* AssetManager::LoadAsset(const std::string& assetName, std::unordered_map_ci<std::string, T*>* cache, T*(*createFunc)(const std::string&, char*, unsigned int), bool deleteBuffer)
 {
     // If already present in cache, return existing asset right away.
     if(cache != nullptr)
diff --git a/Source/Assets/AssetManager.h b/Source/Assets/AssetManager.h
index e352124b..ef8ea8ad 100644
--- a/Source/Assets/AssetManager.h
+++ b/Source/Assets/AssetManager.h
@@ -151,7 +151,7 @@ class AssetManager
     // The first uses a single constructor (name, data, size).
     // The second uses a constructor (name) and a separate load function (data, size).
     // The latter is necessary if two assets can potentially attempt to load one another (circular dependency).
-    template<class T> T* LoadAsset(const std::string& assetName, std::unordered_map_ci<std::string, T*>* cache, std::function<T* (const std::string&, char*, unsigned int)> createFunc = nullptr, bool deleteBuffer = true);
+    template<class T> T* LoadAsset(const std::string& assetName, std::unordered_map_ci<std::string, T*>* cache, T*(*createFunc)(const std::string&, char*, unsigned int) = nullptr, bool deleteBuffer = true);
     template<class T> T* LoadAsset_SeparateLoadFunc(const std::string& assetName, std::unordered_map_ci<std::string, T*>* cache, bool deleteBuffer = true);
     
     char* CreateAssetBuffer(const std::string& assetName, unsigned int& outBufferSize);