Rewrite ISystem::listItemsInDirectory

remove deprecated `ISystem::loadBuiltinData` function

Author: devshgraphicsprogramming

include/nbl/system/ISystem.h

@@ -111,17 +111,16 @@ class ISystem : public core::IReferenceCounted
                 }
         };
 
-        // [[deprecated]] use `createFile` instead
-        core::smart_refctd_ptr<IFile> loadBuiltinData(const std::string& builtinPath) const;
-
         //! Compile time resource ID
         template<typename StringUniqueType>
         inline core::smart_refctd_ptr<IFile> loadBuiltinData() const
         {
         #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
             return impl_loadEmbeddedBuiltinData(StringUniqueType::value,nbl::builtin::get_resource<StringUniqueType>());
         #else
-            return loadBuiltinData(StringUniqueType::value);
+            future_t<core::smart_refctd_ptr<IFile>> future;
+            createFile(future,StringUniqueType::value,core::bitflag(IFileBase::ECF_READ)|IFileBase::ECF_MAPPABLE);
+            return future.get();
         #endif
         }
 
@@ -177,7 +176,7 @@ class ISystem : public core::IReferenceCounted
         //
         void createFile(
             future_t<core::smart_refctd_ptr<IFile>>& future, // creation may happen on a dedicated thread, so its async
-            std::filesystem::path filename, // absolute path within our virtual filesystem
+            path filename, // absolute path within our virtual filesystem
             const core::bitflag<IFileBase::E_CREATE_FLAGS> flags, // access flags (IMPORTANT: files from most archives wont open with ECF_WRITE bit)
             const std::string_view& accessToken="" // usually password for archives, but should be SSH key for URL downloads
         );

src/nbl/asset/utils/IGLSLEmbeddedIncludeLoader.h

@@ -50,8 +50,10 @@ class IGLSLEmbeddedIncludeLoader : public IBuiltinIncludeLoader
 		//
 		inline std::string getFromDiskOrEmbedding(const std::string& _name) const
 		{
+			system::ISystem::future_t<core::smart_refctd_ptr<system::IFile>> future;
 			auto path = "nbl/builtin/" + _name;
-			auto data = s->loadBuiltinData(path);
+			s->createFile(future,path,core::bitflag(system::IFileBase::ECF_READ)|system::IFileBase::ECF_MAPPABLE);
+			auto data = future.get();
 			if (!data)
 				return "";
 			auto begin = reinterpret_cast<const char*>(data->getMappedPointer());

src/nbl/system/ISystem.cpp

@@ -16,40 +16,15 @@ ISystem::ISystem(core::smart_refctd_ptr<ISystem::ICaller>&& caller) : m_dispatch
     //addArchiveLoader(core::make_smart_refctd_ptr<CArchiveLoaderTar>(nullptr));
 }
 
-core::smart_refctd_ptr<IFile> ISystem::loadBuiltinData(const std::string& builtinPath) const
-{
-#ifdef _NBL_EMBED_BUILTIN_RESOURCES_
-    return impl_loadEmbeddedBuiltinData(builtinPath,nbl::builtin::get_resource_runtime(builtinPath));
-#else
-    constexpr auto pathPrefix = "nbl/builtin/";
-    auto pos = builtinPath.find(pathPrefix);
-    std::string path;
-    if (pos != std::string::npos)
-        path = builtinResourceDirectory + builtinPath.substr(pos + strlen(pathPrefix));
-    else
-        path = builtinResourceDirectory + builtinPath;
-
-    future_t<core::smart_refctd_ptr<IFile>> fut;
-    createFile(future, path.c_str(), core::bitflag<IFile::E_CREATE_FLAGS>(IFile::ECF_READ) :: IFile::ECF_MAPPABLE);
-    auto file = fut.get();
-    if (file.get())
-    { 
-        return file;
-    }
-    return nullptr;
-#endif
-}
-
 core::smart_refctd_ptr<IFile> ISystem::impl_loadEmbeddedBuiltinData(const std::string& builtinPath, const std::pair<const uint8_t*,size_t>& found) const
 {
 #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
     if (found.first && found.second)
     {
         auto fileView = core::make_smart_refctd_ptr<CFileView<CNullAllocator>>(
-            core::smart_refctd_ptr<ISystem>(this),
             builtinPath, 
             IFile::ECF_READ,
-            found.first,
+            const_cast<uint8_t*>(found.first),
             found.second
         );
         return fileView;
@@ -58,14 +33,21 @@ core::smart_refctd_ptr<IFile> ISystem::impl_loadEmbeddedBuiltinData(const std::s
     return nullptr;
 }
 
+constexpr const char* builtinPathPrefix = "nbl/builtin/";
 bool ISystem::exists(const system::path& filename, const core::bitflag<IFile::E_CREATE_FLAGS> flags) const
 {
     const bool writeUsage = flags.value&IFile::ECF_WRITE;
-#ifdef _NBL_EMBED_BUILTIN_RESOURCES_
-    std::pair<const uint8_t*, size_t> found = nbl::builtin::get_resource_runtime(filename.string());
-    if (!writeUsage && found.first && found.second)
-        return true;
-#endif
+    if (!writeUsage && filename.string().find(builtinPathPrefix)==0)
+    {
+        #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
+            std::pair<const uint8_t*, size_t> found = nbl::builtin::get_resource_runtime(filename.string());
+            if (found.first && found.second)
+                return true;
+        #else
+            if (exists(builtinResourceDirectory/filename))
+                return true;
+        #endif
+    }
     // filename too long
     if (filename.string().size() >= sizeof(SRequestParams_CREATE_FILE::filename))
         return false;
@@ -78,17 +60,14 @@ bool ISystem::exists(const system::path& filename, const core::bitflag<IFile::E_
 
 bool ISystem::isPathReadOnly(const system::path& p) const
 {
+    // first check if its a builtin path
+    if (p.string().find(builtinPathPrefix)==0)
+        return true;
+
     // check all parent subpaths
     auto curPath = p;
     while (!curPath.empty() && curPath.parent_path() != curPath)
     {
-        // first check if its a builtin path
-    #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
-        std::pair<const uint8_t*,size_t> found = nbl::builtin::get_resource_runtime(curPath.string());
-        if (found.first && found.second)
-            return true;
-    #endif
-
         // then check for presence in an archive
         auto archives = m_cachedArchiveFiles.findRange(curPath);
         if (!archives.empty())
@@ -103,33 +82,55 @@ bool ISystem::isPathReadOnly(const system::path& p) const
 core::vector<system::path> ISystem::listItemsInDirectory(const system::path& p) const
 {
     core::vector<system::path> res;
-    res.reserve(128u);
+    res.reserve(512u);
 
-    // TODO: check for path being a builtin
-    if (isPathReadOnly(p)) // TODO: better check for archives
+    auto addArchiveItems = [this,&res](const path& archPath) -> void
     {
-        auto curPath = p;
-        while (!curPath.empty() && curPath.parent_path() != curPath)
+        const auto archives = m_cachedArchiveFiles.findRange(archPath);
+        for (auto& arch : archives)
         {
-            auto archives = m_cachedArchiveFiles.findRange(curPath);
-            for (auto& arch : archives)
-            {
-                auto rel = std::filesystem::relative(p, arch.first);
-                auto res =  arch.second->listAssets(rel.generic_string().c_str());
-                std::for_each(res.begin(), res.end(), [&arch](system::path& p) {p = arch.first / p; });
-                return res;
-            }
-
-            curPath = curPath.parent_path().generic_string();
+            const auto assets = arch.second->listAssets();
+            for (auto& item : assets)
+                res.push_back(archPath/item.pathRelativeToArchive);
         }
+    };
+
+    std::error_code err;
+    const auto directories = std::filesystem::recursive_directory_iterator(p,err);
+    if (!err)
+    for (auto entry : directories)
+    {
+        res.push_back(entry.path());
+        // entry could have been an archive
+        addArchiveItems(entry.path());
     }
     else
     {
-        uint32_t fileCount = std::distance(std::filesystem::recursive_directory_iterator(p), std::filesystem::recursive_directory_iterator{});
-        for (auto entry : std::filesystem::recursive_directory_iterator(p))
-            res.push_back(entry.path());
+        #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
+        // TODO: Change the python generator and make builtinResourceData.cpp's `resourcesByFilename` a nicely named `extern` variable
+        // then in ISystem precompute a [unordered] multimap with keys being builtin subpaths and values being full paths they contain
+        // find all the values here and iterate over them, adding to `res
+        #else
+        err.clear();
+        // check root path is prefixed with "nbl/builtin/"
+        if (p.string().find(builtinPathPrefix) == 0)
+        {
+            const auto subdirs = std::filesystem::recursive_directory_iterator(builtinResourceDirectory/p,err);
+            if (!err)
+            for (auto entry : subdirs) // there are never any archives inside builtins
+                res.push_back(entry.path());
+        }
+        #endif
+        // check for part of subpath being an archive
+        system::path path = std::filesystem::exists(p) ? std::filesystem::canonical(p.parent_path()):p.parent_path();
+        // going up the directory tree
+        while (!path.empty() && path.parent_path()!=path)
+        {
+            path = std::filesystem::exists(path) ? std::filesystem::canonical(path):path;
+            addArchiveItems(path);
+            path = path.parent_path();
+        }
     }
-    // TODO: recurse into any archives which could have been found!
     return res;
 }
 
@@ -204,18 +205,26 @@ bool ISystem::copy(const system::path& from, const system::path& to)
 
 void ISystem::createFile(future_t<core::smart_refctd_ptr<IFile>>& future, std::filesystem::path filename, const core::bitflag<IFileBase::E_CREATE_FLAGS> flags, const std::string_view& accessToken)
 {
+    // canonicalize
+    if (std::filesystem::exists(filename))
+        filename = std::filesystem::canonical(filename);
     // try builtins
-    if (!(flags.value&IFile::ECF_WRITE))
+    if (!(flags.value&IFile::ECF_WRITE) && filename.string().find(builtinPathPrefix)==0)
     {
-        auto file = loadBuiltinData(filename.string());
-        if (file)
-        {
-            future.notify(std::move(file));
+        #ifdef _NBL_EMBED_BUILTIN_RESOURCES_
+            auto file = impl_loadEmbeddedBuiltinData(filename.string(),nbl::builtin::get_resource_runtime(filename.string()));
+            if (file)
+            {
+                future.notify(std::move(file));
+                return;
+            }
+        #else
+            createFile(future,builtinResourceDirectory/filename,flags);
             return;
-        }
+        #endif
     }
-    // try archives
-    if (flags.value&IFile::ECF_READ)
+    // try archives (readonly, for now)
+    if (!(flags.value&IFile::ECF_WRITE))
     {
         const auto found = findFileInArchive(filename);
         if (found.archive)