From aa386f66e5eb65707eaf6954156de4740c794b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 15 Nov 2021 19:41:41 +0100 Subject: [PATCH] Trade: make importer by-name-lookup failure messages a bit more helpful. For example if we're looking for a 2D image named "cubemap", the importer will tell us there's 0 2D images, making a potential mistake (2D vs 3D) more obvious. --- src/Magnum/Trade/AbstractImporter.cpp | 28 +++++------ .../Trade/Test/AbstractImporterTest.cpp | 47 +++++++++++++------ 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index fb6a96c08..3d7d95f7e 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -300,7 +300,7 @@ Containers::Optional AbstractImporter::scene(const std::string& name) CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::scene(): no file opened", {}); const Int id = doSceneForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::scene(): scene" << name << "not found"; + Error{} << "Trade::AbstractImporter::scene(): scene" << name << "not found among" << doSceneCount() << "entries"; return {}; } return scene(id); /* not doScene(), so we get the range checks also */ @@ -350,7 +350,7 @@ Containers::Optional AbstractImporter::animation(const std::strin CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {}); const Int id = doAnimationForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::animation(): animation" << name << "not found"; + Error{} << "Trade::AbstractImporter::animation(): animation" << name << "not found among" << doAnimationCount() << "entries"; return {}; } return animation(id); /* not doAnimation(), so we get the checks also */ @@ -395,7 +395,7 @@ Containers::Optional AbstractImporter::light(const std::string& name) CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::light(): no file opened", {}); const Int id = doLightForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::light(): light" << name << "not found"; + Error{} << "Trade::AbstractImporter::light(): light" << name << "not found among" << doLightCount() << "entries"; return {}; } return light(id); /* not doLight(), so we get the range checks also */ @@ -440,7 +440,7 @@ Containers::Optional AbstractImporter::camera(const std::string& nam CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::camera(): no file opened", {}); const Int id = doCameraForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::camera(): camera" << name << "not found"; + Error{} << "Trade::AbstractImporter::camera(): camera" << name << "not found among" << doCameraCount() << "entries"; return {}; } return camera(id); /* not doCamera(), so we get the range checks also */ @@ -485,7 +485,7 @@ Containers::Pointer AbstractImporter::object2D(const std::string& CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::object2D(): no file opened", {}); const Int id = doObject2DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::object2D(): object" << name << "not found"; + Error{} << "Trade::AbstractImporter::object2D(): object" << name << "not found among" << doObject2DCount() << "entries"; return {}; } return object2D(id); /* not doObject2D(), so we get the range checks also */ @@ -530,7 +530,7 @@ Containers::Pointer AbstractImporter::object3D(const std::string& CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::object3D(): no file opened", {}); const Int id = doObject3DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::object3D(): object" << name << "not found"; + Error{} << "Trade::AbstractImporter::object3D(): object" << name << "not found among" << doObject3DCount() << "entries"; return {}; } return object3D(id); /* not doObject3D(), so we get the range checks also */ @@ -580,7 +580,7 @@ Containers::Optional AbstractImporter::skin2D(const std::string& nam CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::skin2D(): no file opened", {}); const Int id = doSkin2DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::skin2D(): skin" << name << "not found"; + Error{} << "Trade::AbstractImporter::skin2D(): skin" << name << "not found among" << doSkin2DCount() << "entries"; return {}; } return skin2D(id); /* not doSkin2D(), so we get the range checks also */ @@ -630,7 +630,7 @@ Containers::Optional AbstractImporter::skin3D(const std::string& nam CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::skin3D(): no file opened", {}); const Int id = doSkin3DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::skin3D(): skin" << name << "not found"; + Error{} << "Trade::AbstractImporter::skin3D(): skin" << name << "not found among" << doSkin3DCount() << "entries"; return {}; } return skin3D(id); /* not doSkin3D(), so we get the range checks also */ @@ -702,7 +702,7 @@ Containers::Optional AbstractImporter::mesh(const std::string& name, c CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::mesh(): no file opened", {}); const Int id = doMeshForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::mesh(): mesh" << name << "not found"; + Error{} << "Trade::AbstractImporter::mesh(): mesh" << name << "not found among" << doMeshCount() << "entries"; return {}; } return mesh(id, level); /* not doMesh(), so we get the checks also */ @@ -878,7 +878,7 @@ AbstractImporter::material(const std::string& name) { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::material(): no file opened", {}); const Int id = doMaterialForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::material(): material" << name << "not found"; + Error{} << "Trade::AbstractImporter::material(): material" << name << "not found among" << doMaterialCount() << "entries"; return {}; } return material(id); /* not doMaterial(), so we get the range checks also */ @@ -923,7 +923,7 @@ Containers::Optional AbstractImporter::texture(const std::string& n CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::texture(): no file opened", {}); const Int id = doTextureForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::texture(): texture" << name << "not found"; + Error{} << "Trade::AbstractImporter::texture(): texture" << name << "not found among" << doTextureCount() << "entries"; return {}; } return texture(id); /* not doTexture(), so we get the range checks also */ @@ -992,7 +992,7 @@ Containers::Optional AbstractImporter::image1D(const std::string& n CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image1D(): no file opened", {}); const Int id = doImage1DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::image1D(): image" << name << "not found"; + Error{} << "Trade::AbstractImporter::image1D(): image" << name << "not found among" << doImage1DCount() << "entries"; return {}; } /* not doImage1D(), so we get the range checks also */ @@ -1062,7 +1062,7 @@ Containers::Optional AbstractImporter::image2D(const std::string& n CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image2D(): no file opened", {}); const Int id = doImage2DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::image2D(): image" << name << "not found"; + Error{} << "Trade::AbstractImporter::image2D(): image" << name << "not found among" << doImage2DCount() << "entries"; return {}; } /* not doImage2D(), so we get the range checks also */ @@ -1132,7 +1132,7 @@ Containers::Optional AbstractImporter::image3D(const std::string& n CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image3D(): no file opened", {}); const Int id = doImage3DForName(name); if(id == -1) { - Error{} << "Trade::AbstractImporter::image3D(): image" << name << "not found"; + Error{} << "Trade::AbstractImporter::image3D(): image" << name << "not found among" << doImage3DCount() << "entries"; return {}; } /* not doImage3D(), so we get the range checks also */ diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 55fd40800..671aec909 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -1372,6 +1372,25 @@ void AbstractImporterTest::thingByNameNotFound() { ImporterFeatures doFeatures() const override { return {}; } bool doIsOpened() const override { return true; } void doClose() override {} + + UnsignedInt doSceneCount() const override { return 1; } + UnsignedInt doAnimationCount() const override { return 2; } + UnsignedInt doLightCount() const override { return 3; } + UnsignedInt doCameraCount() const override { return 4; } + + UnsignedInt doObject2DCount() const override { return 5; } + UnsignedInt doObject3DCount() const override { return 6; } + + UnsignedInt doSkin2DCount() const override { return 7; } + UnsignedInt doSkin3DCount() const override { return 8; } + + UnsignedInt doMeshCount() const override { return 9; } + UnsignedInt doMaterialCount() const override { return 10; } + UnsignedInt doTextureCount() const override { return 11; } + + UnsignedInt doImage1DCount() const override { return 12; } + UnsignedInt doImage2DCount() const override { return 13; } + UnsignedInt doImage3DCount() const override { return 14; } } importer; std::ostringstream out; @@ -1401,24 +1420,24 @@ void AbstractImporterTest::thingByNameNotFound() { if(data.checkMessage) { CORRADE_COMPARE(out.str(), - "Trade::AbstractImporter::scene(): scene foobar not found\n" - "Trade::AbstractImporter::animation(): animation foobar not found\n" - "Trade::AbstractImporter::light(): light foobar not found\n" - "Trade::AbstractImporter::camera(): camera foobar not found\n" + "Trade::AbstractImporter::scene(): scene foobar not found among 1 entries\n" + "Trade::AbstractImporter::animation(): animation foobar not found among 2 entries\n" + "Trade::AbstractImporter::light(): light foobar not found among 3 entries\n" + "Trade::AbstractImporter::camera(): camera foobar not found among 4 entries\n" - "Trade::AbstractImporter::object2D(): object foobar not found\n" - "Trade::AbstractImporter::object3D(): object foobar not found\n" + "Trade::AbstractImporter::object2D(): object foobar not found among 5 entries\n" + "Trade::AbstractImporter::object3D(): object foobar not found among 6 entries\n" - "Trade::AbstractImporter::skin2D(): skin foobar not found\n" - "Trade::AbstractImporter::skin3D(): skin foobar not found\n" + "Trade::AbstractImporter::skin2D(): skin foobar not found among 7 entries\n" + "Trade::AbstractImporter::skin3D(): skin foobar not found among 8 entries\n" - "Trade::AbstractImporter::mesh(): mesh foobar not found\n" - "Trade::AbstractImporter::material(): material foobar not found\n" - "Trade::AbstractImporter::texture(): texture foobar not found\n" + "Trade::AbstractImporter::mesh(): mesh foobar not found among 9 entries\n" + "Trade::AbstractImporter::material(): material foobar not found among 10 entries\n" + "Trade::AbstractImporter::texture(): texture foobar not found among 11 entries\n" - "Trade::AbstractImporter::image1D(): image foobar not found\n" - "Trade::AbstractImporter::image2D(): image foobar not found\n" - "Trade::AbstractImporter::image3D(): image foobar not found\n"); + "Trade::AbstractImporter::image1D(): image foobar not found among 12 entries\n" + "Trade::AbstractImporter::image2D(): image foobar not found among 13 entries\n" + "Trade::AbstractImporter::image3D(): image foobar not found among 14 entries\n"); } }