From b1686170341a0555db33ed4a1108b418dd62a9c0 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Thu, 27 Oct 2022 18:14:30 -0500 Subject: [PATCH 1/5] correct the moduleDependencySort callback --- Engine/source/module/moduleManager.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/Engine/source/module/moduleManager.cpp b/Engine/source/module/moduleManager.cpp index 08db0da6c..04411640a 100644 --- a/Engine/source/module/moduleManager.cpp +++ b/Engine/source/module/moduleManager.cpp @@ -80,17 +80,25 @@ S32 QSORT_CALLBACK moduleDependencySort(const void* a, const void* b) ModuleDefinition* pDefinition1 = *(ModuleDefinition * *)a; ModuleDefinition* pDefinition2 = *(ModuleDefinition * *)b; - // Fetch version Ids. + // if A depends on B move A down the list ModuleDefinition::typeModuleDependencyVector moduleDependencies = pDefinition1->getDependencies(); - bool foundDependant = false; for (ModuleDefinition::typeModuleDependencyVector::const_iterator dependencyItr = moduleDependencies.begin(); dependencyItr != moduleDependencies.end(); ++dependencyItr) { if (String::compare(dependencyItr->mModuleId, pDefinition2->getModuleId()) && (dependencyItr->mVersionId == pDefinition2->getVersionId())) - foundDependant = true; + return -1; } - return foundDependant ? 1 : -1; + //If B depends on A, move A up the list + ModuleDefinition::typeModuleDependencyVector moduleDependencies2 = pDefinition2->getDependencies(); + for (ModuleDefinition::typeModuleDependencyVector::const_iterator dependencyItr2 = moduleDependencies2.begin(); dependencyItr2 != moduleDependencies2.end(); ++dependencyItr2) + { + if (String::compare(dependencyItr2->mModuleId, pDefinition1->getModuleId()) + && (dependencyItr2->mVersionId == pDefinition1->getVersionId())) + return 1; + } + //if neither depend on the other leave the order alone + return 0; } //----------------------------------------------------------------------------- From b48d462fbe0d7cca09d078565d05f108547ae172 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Sat, 29 Oct 2022 12:11:54 -0500 Subject: [PATCH 2/5] debug option for checking module order --- Templates/BaseGame/game/core/utility/scripts/module.tscript | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Templates/BaseGame/game/core/utility/scripts/module.tscript b/Templates/BaseGame/game/core/utility/scripts/module.tscript index 137cf6835..fe5189cf9 100644 --- a/Templates/BaseGame/game/core/utility/scripts/module.tscript +++ b/Templates/BaseGame/game/core/utility/scripts/module.tscript @@ -1,3 +1,4 @@ +$reportModuleOrder = false; $traceModuleCalls=false; $reportModuleFileConflicts=true; if (!isObject(ExecFilesList)) @@ -10,10 +11,11 @@ function callOnModules(%functionName, %moduleGroup, %var0, %var1, %var2, %var3, ExecFilesList.push_back(%execArray); //Get our modules so we can exec any specific client-side loading/handling %modulesList = ModuleDatabase.findModules(false); + %modlist = "modlist:"; for(%i=0; %i < getWordCount(%modulesList); %i++) { %module = getWord(%modulesList, %i); - + %modlist = %modlist SPC %module.ModuleId; if(%moduleGroup !$= "") { if(%module.group !$= %moduleGroup) @@ -23,6 +25,8 @@ function callOnModules(%functionName, %moduleGroup, %var0, %var1, %var2, %var3, if(isObject(%module.scopeSet) && %module.scopeSet.isMethod(%functionName)) %module.scopeSet.call(%functionName, %var0, %var1, %var2, %var3, %var4, %var5, %var6); } + if ($reportModuleOrder) + warn(%modlist); %execFilecount = %execArray.count(); From ebcee97d5ca160499fa60c43d7ccf0409fdb4852 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Sun, 30 Oct 2022 14:02:30 -0500 Subject: [PATCH 3/5] leverage the pre-existing vector.sort(&method) hooks for better self containment. also sort by dependency count, and above all else, skip sorting entirely if we've already populated mModulesLoaded since that already takes dependencies into account when generating the vector also defualt to using that one for the general findmodules case --- Engine/source/module/moduleManager.cpp | 84 ++++++++++--------- Engine/source/module/moduleManager.h | 7 +- .../module/moduleManager_ScriptBinding.h | 2 +- 3 files changed, 51 insertions(+), 42 deletions(-) diff --git a/Engine/source/module/moduleManager.cpp b/Engine/source/module/moduleManager.cpp index 04411640a..cdfd90f94 100644 --- a/Engine/source/module/moduleManager.cpp +++ b/Engine/source/module/moduleManager.cpp @@ -74,30 +74,28 @@ S32 QSORT_CALLBACK moduleDefinitionVersionIdSort( const void* a, const void* b ) return versionId1 > versionId2 ? -1 : versionId1 < versionId2 ? 1 : 0; } -S32 QSORT_CALLBACK moduleDependencySort(const void* a, const void* b) +S32 ModuleManager::moduleDependencySort(ModuleDefinition* const* a, ModuleDefinition* const* b) { - // Fetch module definitions. - ModuleDefinition* pDefinition1 = *(ModuleDefinition * *)a; - ModuleDefinition* pDefinition2 = *(ModuleDefinition * *)b; - // if A depends on B move A down the list - ModuleDefinition::typeModuleDependencyVector moduleDependencies = pDefinition1->getDependencies(); + ModuleDefinition::typeModuleDependencyVector moduleDependencies = (*a)->getDependencies(); for (ModuleDefinition::typeModuleDependencyVector::const_iterator dependencyItr = moduleDependencies.begin(); dependencyItr != moduleDependencies.end(); ++dependencyItr) { - if (String::compare(dependencyItr->mModuleId, pDefinition2->getModuleId()) - && (dependencyItr->mVersionId == pDefinition2->getVersionId())) - return -1; + if ((String::compare(dependencyItr->mModuleId, (*b)->getModuleId()) == 0) + && (dependencyItr->mVersionId == (*b)->getVersionId())) + return 1; } //If B depends on A, move A up the list - ModuleDefinition::typeModuleDependencyVector moduleDependencies2 = pDefinition2->getDependencies(); + ModuleDefinition::typeModuleDependencyVector moduleDependencies2 = (*b)->getDependencies(); for (ModuleDefinition::typeModuleDependencyVector::const_iterator dependencyItr2 = moduleDependencies2.begin(); dependencyItr2 != moduleDependencies2.end(); ++dependencyItr2) { - if (String::compare(dependencyItr2->mModuleId, pDefinition1->getModuleId()) - && (dependencyItr2->mVersionId == pDefinition1->getVersionId())) - return 1; + if ((String::compare(dependencyItr2->mModuleId, (*a)->getModuleId()) == 0) + && (dependencyItr2->mVersionId == (*a)->getVersionId())) + return -1; } - //if neither depend on the other leave the order alone + //didn't find any explicit dependencies between the two, so sort by which has more + if (moduleDependencies.size() > moduleDependencies2.size()) return 1; + if (moduleDependencies.size() < moduleDependencies2.size()) return -1; return 0; } @@ -1074,39 +1072,49 @@ ModuleDefinition* ModuleManager::findLoadedModule( const char* pModuleId ) //----------------------------------------------------------------------------- -void ModuleManager::findModules( const bool loadedOnly, typeConstModuleDefinitionVector& moduleDefinitions ) +void ModuleManager::findModules(const bool loadedOnly, typeModuleDefinitionVector& moduleDefinitions) { - // Iterate module Ids. - for( typeModuleIdDatabaseHash::iterator moduleIdItr = mModuleIdDatabase.begin(); moduleIdItr != mModuleIdDatabase.end(); ++moduleIdItr ) - { - // Fetch module definition entry. - ModuleDefinitionEntry* pModuleDefinitionEntry = moduleIdItr->value; + if (loadedOnly) + { + for (U32 i = 0; i < mModulesLoaded.size(); i++) + { + moduleDefinitions.push_back(mModulesLoaded[i].mpModuleDefinition); + } + } + else + { + // Iterate module Ids. + for (typeModuleIdDatabaseHash::iterator moduleIdItr = mModuleIdDatabase.begin(); moduleIdItr != mModuleIdDatabase.end(); ++moduleIdItr) + { + // Fetch module definition entry. + ModuleDefinitionEntry* pModuleDefinitionEntry = moduleIdItr->value; - // Iterate module definitions. - for ( typeModuleDefinitionVector::iterator moduleDefinitionItr = pModuleDefinitionEntry->begin(); moduleDefinitionItr != pModuleDefinitionEntry->end(); ++moduleDefinitionItr ) - { + // Iterate module definitions. + for (typeModuleDefinitionVector::iterator moduleDefinitionItr = pModuleDefinitionEntry->begin(); moduleDefinitionItr != pModuleDefinitionEntry->end(); ++moduleDefinitionItr) + { // Fetch module definition. ModuleDefinition* pModuleDefinition = *moduleDefinitionItr; // Are we searching for loaded modules only? - if ( loadedOnly ) + if (loadedOnly) { - // Yes, so skip if the module is not loaded. - if ( pModuleDefinition->getLoadCount() == 0 ) - continue; + // Yes, so skip if the module is not loaded. + if (pModuleDefinition->getLoadCount() == 0) + continue; - // Use module definition. - moduleDefinitions.push_back( pModuleDefinition ); + // Use module definition. + moduleDefinitions.push_back(pModuleDefinition); - // Finish iterating module definitions as only a single module in this entry can be loaded concurrently. - break; + // Finish iterating module definitions as only a single module in this entry can be loaded concurrently. + break; } // use module definition. - moduleDefinitions.push_back( pModuleDefinition ); - } - } - dQsort(moduleDefinitions.address(), moduleDefinitions.size(), sizeof(ModuleDefinition*), moduleDependencySort); + moduleDefinitions.push_back(pModuleDefinition); + } + } + moduleDefinitions.sort(&moduleDependencySort); + } } //----------------------------------------------------------------------------- @@ -1579,7 +1587,7 @@ bool ModuleManager::synchronizeDependencies( ModuleDefinition* pRootModuleDefini } // Find any target modules left, These are orphaned modules not depended upon by any other module. - typeConstModuleDefinitionVector orphanedTargetModules; + typeModuleDefinitionVector orphanedTargetModules; targetModuleManager.findModules( false, orphanedTargetModules ); // Iterate module definitions. @@ -1627,7 +1635,7 @@ bool ModuleManager::canMergeModules( const char* pMergeSourcePath ) mergeModuleManager.scanModules( pMergeSourcePath ); // Find all the merge modules. - typeConstModuleDefinitionVector mergeModules; + typeModuleDefinitionVector mergeModules; mergeModuleManager.findModules( false, mergeModules ); // Iterate found merge module definitions. @@ -1716,7 +1724,7 @@ bool ModuleManager::mergeModules( const char* pMergeTargetPath, const bool remov sourceModuleManager.scanModules( mergeSourcePath ); // Find all the source modules. - typeConstModuleDefinitionVector sourceModules; + typeModuleDefinitionVector sourceModules; sourceModuleManager.findModules( false, sourceModules ); // Iterate found merge module definitions. diff --git a/Engine/source/module/moduleManager.h b/Engine/source/module/moduleManager.h index 8673b561c..00f30c59f 100644 --- a/Engine/source/module/moduleManager.h +++ b/Engine/source/module/moduleManager.h @@ -61,7 +61,8 @@ public: /// Module definitions. typedef Vector typeModuleDefinitionVector; typedef Vector typeConstModuleDefinitionVector; - +protected: + static S32 moduleDependencySort(ModuleDefinition* const* a, ModuleDefinition* const* b); private: /// Database locking. struct LockDatabase @@ -175,7 +176,7 @@ public: ModuleDefinition* findModule( const char* pModuleId, const U32 versionId ); ModuleDefinition* findModuleByFilePath(StringTableEntry filePath); ModuleDefinition* findLoadedModule( const char* pModuleId ); - void findModules( const bool loadedOnly, typeConstModuleDefinitionVector& moduleDefinitions ); + void findModules( const bool loadedOnly, typeModuleDefinitionVector& moduleDefinitions ); void findModuleTypes( const char* pModuleType, const bool loadedOnly, typeConstModuleDefinitionVector& moduleDefinitions ); /// Module synchronization. @@ -219,4 +220,4 @@ private: extern ModuleManager ModuleDatabase; -#endif // _MODULE_MANAGER_H \ No newline at end of file +#endif // _MODULE_MANAGER_H diff --git a/Engine/source/module/moduleManager_ScriptBinding.h b/Engine/source/module/moduleManager_ScriptBinding.h index 042683fad..61d70a819 100644 --- a/Engine/source/module/moduleManager_ScriptBinding.h +++ b/Engine/source/module/moduleManager_ScriptBinding.h @@ -157,7 +157,7 @@ DefineEngineMethod(ModuleManager, findModules, String, (bool loadedOnly), (false "@return A list of space - separated module definition object Ids.\n") { // Find module type definitions. - Vector moduleDefinitions; + Vector moduleDefinitions; // Find modules. object->findModules( loadedOnly, moduleDefinitions ); From 9f5824ca3e30168db52ee3cbc90068996d8130e4 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Sun, 30 Oct 2022 14:07:33 -0500 Subject: [PATCH 4/5] use default (rue) case for findModules in callonmodules --- Templates/BaseGame/game/core/utility/scripts/module.tscript | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Templates/BaseGame/game/core/utility/scripts/module.tscript b/Templates/BaseGame/game/core/utility/scripts/module.tscript index fe5189cf9..7cdc57423 100644 --- a/Templates/BaseGame/game/core/utility/scripts/module.tscript +++ b/Templates/BaseGame/game/core/utility/scripts/module.tscript @@ -10,7 +10,7 @@ function callOnModules(%functionName, %moduleGroup, %var0, %var1, %var2, %var3, %execArray = new ArrayObject("callOn" @ %functionName @ "_" @ %moduleGroup); ExecFilesList.push_back(%execArray); //Get our modules so we can exec any specific client-side loading/handling - %modulesList = ModuleDatabase.findModules(false); + %modulesList = ModuleDatabase.findModules(); %modlist = "modlist:"; for(%i=0; %i < getWordCount(%modulesList); %i++) { From ac1cbe8198ad04cdff2112e0b294f2ff8a270694 Mon Sep 17 00:00:00 2001 From: AzaezelX Date: Sun, 30 Oct 2022 14:10:36 -0500 Subject: [PATCH 5/5] set default (true) case for findModules --- Engine/source/module/moduleManager_ScriptBinding.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Engine/source/module/moduleManager_ScriptBinding.h b/Engine/source/module/moduleManager_ScriptBinding.h index 61d70a819..b0f67548b 100644 --- a/Engine/source/module/moduleManager_ScriptBinding.h +++ b/Engine/source/module/moduleManager_ScriptBinding.h @@ -151,7 +151,7 @@ DefineEngineMethod(ModuleManager, findModuleByFilePath, String, (const char* fil //----------------------------------------------------------------------------- -DefineEngineMethod(ModuleManager, findModules, String, (bool loadedOnly), (false), +DefineEngineMethod(ModuleManager, findModules, String, (bool loadedOnly), (true), "Find all the modules registered with the specified loaded state.\n" "@param loadedOnly Whether to return only modules that are loaded or not.\n" "@return A list of space - separated module definition object Ids.\n")