Skip to content

Commit

Permalink
Fixed Mac/Clang/Release crash related to std::function callback durin…
Browse files Browse the repository at this point in the history
…g 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...
  • Loading branch information
kromenak committed Aug 10, 2022
1 parent eba7081 commit 08afdfd
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 15 deletions.
32 changes: 18 additions & 14 deletions Source/Assets/AssetManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Source/Assets/AssetManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 08afdfd

Please sign in to comment.