Make use of std::filesystem::path and cleanup specification error messages

include/openspace/documentation/documentation.h

@@ -66,6 +66,8 @@ struct TestResult {
         std::string offender;
         /// The Reason that caused this offense
         Reason reason;
+        /// An optional explanation for when a verification fails
+        std::string explanation;
     };
 
     /**
@@ -79,7 +81,7 @@ struct TestResult {
          * The reason for the warning
          */
         enum class Reason {
-            Deprecated          ///< The value is marked as deprecated and should not used
+            Deprecated ///< The value is marked as deprecated and should not used
         };
 
         /// The offending key that caused the Warning. In the case of a nested table,

modules/space/rendering/renderablestars.cpp

@@ -308,10 +308,10 @@ namespace {
 
     struct [[codegen::Dictionary(RenderableStars)]] Parameters {
         // The path to the SPECK file containing information about the stars being rendered
-        std::string speckFile [[codegen::key("File")]];
+        std::filesystem::path speckFile [[codegen::key("File")]];
 
         // [[codegen::verbatim(ColorTextureInfo.description)]]
-        std::string colorMap;
+        std::filesystem::path colorMap;
 
         enum class ColorOption {
             Color,
@@ -350,7 +350,7 @@ namespace {
         std::string renderMethod;
 
         // [[codegen::verbatim(PsfTextureInfo.description)]]
-        std::string texture;
+        std::filesystem::path texture;
 
         // [[codegen::verbatim(SizeCompositionOptionInfo.description)]]
         std::optional<std::string> sizeComposition;
@@ -434,11 +434,11 @@ RenderableStars::RenderableStars(const ghoul::Dictionary& dictionary)
     addProperty(_opacity);
     registerUpdateRenderBinFromOpacity();
 
-    _speckFile = absPath(p.speckFile);
+    _speckFile = p.speckFile.string();
     _speckFile.onChange([&]() { _speckFileIsDirty = true; });
     addProperty(_speckFile);
 
-    _colorTexturePath = absPath(p.colorMap);
+    _colorTexturePath = p.colorMap.string();
     _colorTextureFile = std::make_unique<File>(_colorTexturePath);
 
     /*_shapeTexturePath = absPath(dictionary.value<std::string>(
@@ -525,7 +525,7 @@ RenderableStars::RenderableStars(const ghoul::Dictionary& dictionary)
         _renderingMethodOption = RenderOptionTexture;
     }
 
-    _pointSpreadFunctionTexturePath = absPath(p.texture);
+    _pointSpreadFunctionTexturePath = absPath(p.texture.string());
     _pointSpreadFunctionFile = std::make_unique<File>(_pointSpreadFunctionTexturePath);
     _pointSpreadFunctionTexturePath.onChange([&]() {
         _pointSpreadFunctionTextureIsDirty = true;
@@ -1237,13 +1237,13 @@ void RenderableStars::loadShapeTexture() {
 */
 
 void RenderableStars::loadData() {
-    std::string _file = _speckFile;
-    if (!FileSys.fileExists(absPath(_file))) {
+    std::string file = absPath(_speckFile);
+    if (!FileSys.fileExists(file)) {
         return;
     }
 
     std::string cachedFile = FileSys.cacheManager()->cachedFilename(
-        _file,
+        file,
         ghoul::filesystem::CacheManager::Persistent::Yes
     );
 
@@ -1255,7 +1255,7 @@ void RenderableStars::loadData() {
     bool hasCachedFile = FileSys.fileExists(cachedFile);
     if (hasCachedFile) {
         LINFO(fmt::format("Cached file '{}' used for Speck file '{}'",
-            cachedFile, _file
+            cachedFile, file
         ));
 
         bool success = loadCachedFile(cachedFile);
@@ -1263,15 +1263,15 @@ void RenderableStars::loadData() {
             return;
         }
         else {
-            FileSys.cacheManager()->removeCacheFile(_file);
+            FileSys.cacheManager()->removeCacheFile(file);
             // Intentional fall-through to the 'else' computation to generate the cache
             // file for the next run
         }
     }
     else {
-        LINFO(fmt::format("Cache for Speck file '{}' not found", _file));
+        LINFO(fmt::format("Cache for Speck file '{}' not found", file));
     }
-    LINFO(fmt::format("Loading Speck file '{}'", _file));
+    LINFO(fmt::format("Loading Speck file '{}'", file));
 
     readSpeckFile();
 

src/documentation/documentation.cpp

@@ -71,14 +71,18 @@ std::string to_string(const openspace::documentation::TestResult& value) {
     }
     else {
         std::stringstream stream;
-        stream << "Failure." << '\n';
+        stream << "Specification Failure. ";
 
         for (const TestResult::Offense& offense : value.offenses) {
-            stream << "  " << ghoul::to_string(offense) << '\n';
+            stream << fmt::format(" {}", ghoul::to_string(offense));
+            if (!offense.explanation.empty()) {
+                stream << fmt::format(" ({})", offense.explanation);
+            }
+            stream << '\n';
         }
 
         for (const TestResult::Warning& warning : value.warnings) {
-            stream << "  " << ghoul::to_string(warning) << '\n';
+            stream << fmt::format(" {}\n", ghoul::to_string(warning));
         }
 
         return stream.str();
@@ -129,17 +133,23 @@ namespace openspace::documentation {
 
 const std::string DocumentationEntry::Wildcard = "*";
 
-std::string concatenate(const std::vector<TestResult::Offense>& offenses) {
-    std::string result = "Error in specification (";
-    for (const TestResult::Offense& o : offenses) {
-        result += o.offender + ',';
-    }
-    result.back() = ')';
-    return result;
-}
+//std::string concatenate(const std::vector<TestResult::Offense>& offenses) {
+//    std::string result = "Error in specification (";
+//    for (const TestResult::Offense& o : offenses) {
+//        if (o.explanation.empty()) {
+//            result += fmt::format("{} ({}), ", o.offender, ghoul::to_string(o.reason));
+//        }
+//        else {
+//            result += fmt::format("{} ({}: {}), ", o.offender, ghoul::to_string(o.reason), o.explanation);
+//        }
+//    }
+//    result.pop_back();
+//    result.back() = ')';
+//    return result;
+//}
 
 SpecificationError::SpecificationError(TestResult res, std::string comp)
-    : ghoul::RuntimeError(concatenate(res.offenses), std::move(comp))
+    : ghoul::RuntimeError("Error in specification", std::move(comp))
     , result(std::move(res))
 {
     ghoul_assert(!result.success, "Result's success must be false");

src/documentation/verifier.cpp

@@ -189,7 +189,11 @@ TestResult FileVerifier::operator()(const ghoul::Dictionary& dict,
     std::string file = dict.value<std::string>(key);
     if (!std::filesystem::exists(file) || !std::filesystem::is_regular_file(file)) {
         res.success = false;
-        res.offenses.push_back({ key, TestResult::Offense::Reason::Verification });
+        TestResult::Offense off;
+        off.offender = key;
+        off.reason = TestResult::Offense::Reason::Verification;
+        off.explanation = "File did not exist";
+        res.offenses.push_back(off);
     }
     return res;
 }
@@ -209,7 +213,11 @@ TestResult DirectoryVerifier::operator()(const ghoul::Dictionary& dict,
     std::string dir = dict.value<std::string>(key);
     if (!std::filesystem::exists(dir) || !std::filesystem::is_directory(dir)) {
         res.success = false;
-        res.offenses.push_back({ key, TestResult::Offense::Reason::Verification });
+        TestResult::Offense off;
+        off.offender = key;
+        off.reason = TestResult::Offense::Reason::Verification;
+        off.explanation = "Directory did not exist";
+        res.offenses.push_back(off);
     }
     return res;
 }

src/scene/asset.cpp

@@ -520,9 +520,8 @@ bool Asset::initialize() {
         loader()->callOnInitialize(this);
     }
     catch (const ghoul::lua::LuaRuntimeException& e) {
-        LERROR(fmt::format(
-            "Failed to initialize asset {}; {}: {}", id(), e.component, e.message
-        ));
+        LERROR(fmt::format("Failed to initialize asset {}", id()));
+        LERROR(fmt::format("{}: {}", e.component, e.message));
         // TODO: rollback;
         setState(State::InitializationFailed);
         return false;

src/scene/scene_lua.inl

@@ -528,10 +528,10 @@ int addSceneGraphNode(lua_State* L) {
         global::renderEngine->scene()->initializeNode(node);
     }
     catch (const documentation::SpecificationError& e) {
+        LERRORC("Scene", ghoul::to_string(e.result));
         return ghoul::lua::luaError(
             L,
-            fmt::format("Error loading scene graph node: {}: {}",
-                e.what(), ghoul::to_string(e.result))
+            fmt::format("Error loading scene graph node: {}", e.what())
         );
     }
     catch (const ghoul::RuntimeError& e) {