Browse Source

Trade: print a message when named thing is not found.

Otherwise it's hard to distinguish whether it's a particular importer
fault (failing without a message) or if the name is not found.
pull/432/head
Vladimír Vondruš 6 years ago
parent
commit
c3299ad3c5
  1. 60
      src/Magnum/Trade/AbstractImporter.cpp
  2. 83
      src/Magnum/Trade/AbstractImporter.h
  3. 105
      src/Magnum/Trade/Test/AbstractImporterTest.cpp

60
src/Magnum/Trade/AbstractImporter.cpp

@ -251,7 +251,10 @@ Containers::Optional<SceneData> AbstractImporter::doScene(UnsignedInt) {
Containers::Optional<SceneData> AbstractImporter::scene(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::scene(): no file opened", {});
const Int id = doSceneForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::scene(): scene" << name << "not found";
return {};
}
return scene(id); /* not doScene(), so we get the range checks also */
}
@ -295,7 +298,10 @@ Containers::Optional<AnimationData> AbstractImporter::doAnimation(UnsignedInt) {
Containers::Optional<AnimationData> AbstractImporter::animation(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::animation(): no file opened", {});
const Int id = doAnimationForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::animation(): animation" << name << "not found";
return {};
}
return animation(id); /* not doAnimation(), so we get the checks also */
}
@ -334,7 +340,10 @@ Containers::Optional<LightData> AbstractImporter::doLight(UnsignedInt) {
Containers::Optional<LightData> AbstractImporter::light(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::light(): no file opened", {});
const Int id = doLightForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::light(): light" << name << "not found";
return {};
}
return light(id); /* not doLight(), so we get the range checks also */
}
@ -373,7 +382,10 @@ Containers::Optional<CameraData> AbstractImporter::doCamera(UnsignedInt) {
Containers::Optional<CameraData> AbstractImporter::camera(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::camera(): no file opened", {});
const Int id = doCameraForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::camera(): camera" << name << "not found";
return {};
}
return camera(id); /* not doCamera(), so we get the range checks also */
}
@ -412,7 +424,10 @@ Containers::Pointer<ObjectData2D> AbstractImporter::doObject2D(UnsignedInt) {
Containers::Pointer<ObjectData2D> AbstractImporter::object2D(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::object2D(): no file opened", {});
const Int id = doObject2DForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::object2D(): object" << name << "not found";
return {};
}
return object2D(id); /* not doObject2D(), so we get the range checks also */
}
@ -451,7 +466,10 @@ Containers::Pointer<ObjectData3D> AbstractImporter::doObject3D(UnsignedInt) {
Containers::Pointer<ObjectData3D> AbstractImporter::object3D(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::object3D(): no file opened", {});
const Int id = doObject3DForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::object3D(): object" << name << "not found";
return {};
}
return object3D(id); /* not doObject3D(), so we get the range checks also */
}
@ -517,7 +535,10 @@ Containers::Optional<MeshData> AbstractImporter::doMesh(UnsignedInt, UnsignedInt
Containers::Optional<MeshData> AbstractImporter::mesh(const std::string& name, const UnsignedInt level) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::mesh(): no file opened", {});
const Int id = doMeshForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::mesh(): mesh" << name << "not found";
return {};
}
return mesh(id, level); /* not doMesh(), so we get the checks also */
}
@ -665,7 +686,10 @@ Containers::Pointer<AbstractMaterialData> AbstractImporter::doMaterial(UnsignedI
Containers::Pointer<AbstractMaterialData> AbstractImporter::material(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::material(): no file opened", {});
const Int id = doMaterialForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::material(): material" << name << "not found";
return {};
}
return material(id); /* not doMaterial(), so we get the range checks also */
}
@ -704,7 +728,10 @@ Containers::Optional<TextureData> AbstractImporter::doTexture(UnsignedInt) {
Containers::Optional<TextureData> AbstractImporter::texture(const std::string& name) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::texture(): no file opened", {});
const Int id = doTextureForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::texture(): texture" << name << "not found";
return {};
}
return texture(id); /* not doTexture(), so we get the range checks also */
}
@ -767,7 +794,10 @@ Containers::Optional<ImageData1D> AbstractImporter::doImage1D(UnsignedInt, Unsig
Containers::Optional<ImageData1D> AbstractImporter::image1D(const std::string& name, const UnsignedInt level) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image1D(): no file opened", {});
const Int id = doImage1DForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::image1D(): image" << name << "not found";
return {};
}
/* not doImage1D(), so we get the range checks also */
return image1D(id, level);
}
@ -831,7 +861,10 @@ Containers::Optional<ImageData2D> AbstractImporter::doImage2D(UnsignedInt, Unsig
Containers::Optional<ImageData2D> AbstractImporter::image2D(const std::string& name, const UnsignedInt level) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image2D(): no file opened", {});
const Int id = doImage2DForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::image2D(): image" << name << "not found";
return {};
}
/* not doImage2D(), so we get the range checks also */
return image2D(id, level);
}
@ -895,7 +928,10 @@ Containers::Optional<ImageData3D> AbstractImporter::doImage3D(UnsignedInt, Unsig
Containers::Optional<ImageData3D> AbstractImporter::image3D(const std::string& name, const UnsignedInt level) {
CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::image3D(): no file opened", {});
const Int id = doImage3DForName(name);
if(id == -1) return {};
if(id == -1) {
Error{} << "Trade::AbstractImporter::image3D(): image" << name << "not found";
return {};
}
/* not doImage3D(), so we get the range checks also */
return image3D(id, level);
}

83
src/Magnum/Trade/AbstractImporter.h

@ -519,8 +519,9 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref sceneForName() and
* @ref scene(UnsignedInt). Returns @ref Containers::NullOpt either
* if @ref sceneForName() returns @cpp -1 @ce or if importing fails.
* @ref scene(UnsignedInt). If @ref sceneForName() returns @cpp -1 @ce,
* prints an error message and returns @ref Containers::NullOpt,
* otherwise propagates the result from @ref scene(UnsignedInt).
* Expects that a file is opened.
*/
Containers::Optional<SceneData> scene(const std::string& name);
@ -565,9 +566,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref animationForName() and
* @ref animation(UnsignedInt). Returns @ref Containers::NullOpt either
* if @ref animationForName() returns @cpp -1 @ce or if importing
* fails. Expects that a file is opened.
* @ref animation(UnsignedInt). If @ref animationForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref animation(UnsignedInt). Expects that a file is opened.
*/
Containers::Optional<AnimationData> animation(const std::string& name);
@ -611,8 +613,9 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref lightForName() and
* @ref light(UnsignedInt). Returns @ref Containers::NullOpt either if
* @ref lightForName() returns @cpp -1 @ce or if importing fails.
* @ref light(UnsignedInt). If @ref lightForName() returns @cpp -1 @ce,
* prints an error message and returns @ref Containers::NullOpt,
* otherwise propagates the result from @ref light(UnsignedInt).
* Expects that a file is opened.
*/
Containers::Optional<LightData> light(const std::string& name);
@ -657,9 +660,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref cameraForName() and
* @ref camera(UnsignedInt). Returns @ref Containers::NullOpt either if
* @ref cameraForName() returns @cpp -1 @ce or if importing fails.
* Expects that a file is opened.
* @ref camera(UnsignedInt). If @ref cameraForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref camera(UnsignedInt). Expects that a file is opened.
*/
Containers::Optional<CameraData> camera(const std::string& name);
@ -703,9 +707,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref object2DForName() and
* @ref object2D(UnsignedInt). Returns @cpp nullptr @ce either if
* @ref object2DForName() returns @cpp -1 @ce or if importing fails.
* Expects that a file is opened.
* @ref object2D(UnsignedInt). If @ref object2DForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref object2D(UnsignedInt). Expects that a file is opened.
*/
Containers::Pointer<ObjectData2D> object2D(const std::string& name);
@ -749,9 +754,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref object3DForName() and
* @ref object3D(UnsignedInt). Returns @cpp nullptr @ce either if
* @ref object3DForName() returns @cpp -1 @ce or if importing fails.
* Expects that a file is opened.
* @ref object3D(UnsignedInt). If @ref object3DForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref object3D(UnsignedInt). Expects that a file is opened.
*/
Containers::Pointer<ObjectData3D> object3D(const std::string& name);
@ -816,9 +822,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref meshForName() and
* @ref mesh(UnsignedInt, UnsignedInt). Returns
* @ref Containers::NullOpt either if @ref meshForName() returns
* @cpp -1 @ce or if importing fails. Expects that a file is opened.
* @ref mesh(UnsignedInt, UnsignedInt). If @ref meshForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref mesh(UnsignedInt, UnsignedInt). Expects that a file is opened.
*/
Containers::Optional<MeshData> mesh(const std::string& name, UnsignedInt level = 0);
@ -972,9 +979,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref materialForName() and
* @ref material(UnsignedInt). Returns @ref Containers::NullOpt either
* if @ref materialForName() returns @cpp -1 @ce or if importing fails.
* Expects that a file is opened.
* @ref material(UnsignedInt). If @ref materialForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref material(UnsignedInt). Expects that a file is opened.
*/
Containers::Pointer<AbstractMaterialData> material(const std::string& name);
@ -1018,9 +1026,10 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref textureForName() and
* @ref texture(UnsignedInt). Returns @ref Containers::NullOpt either
* if @ref textureForName() returns @cpp -1 @ce or if importing fails.
* Expects that a file is opened.
* @ref texture(UnsignedInt). If @ref textureForName() returns
* @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref texture(UnsignedInt). Expects that a file is opened.
*/
Containers::Optional<TextureData> texture(const std::string& name);
@ -1076,9 +1085,11 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref image1DForName() and
* @ref image1D(UnsignedInt, UnsignedInt). Returns
* @ref Containers::NullOpt either if @ref image1DForName() returns
* @cpp -1 @ce or if importing fails. Expects that a file is opened.
* @ref image1D(UnsignedInt, UnsignedInt). If @ref image1DForName()
* returns @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref image1D(UnsignedInt, UnsignedInt). Expects that a file is
* opened.
*/
Containers::Optional<ImageData1D> image1D(const std::string& name, UnsignedInt level = 0);
@ -1134,9 +1145,11 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref image2DForName() and
* @ref image2D(UnsignedInt, UnsignedInt). Returns
* @ref Containers::NullOpt either if @ref image2DForName() returns
* @cpp -1 @ce or if importing fails. Expects that a file is opened.
* @ref image2D(UnsignedInt, UnsignedInt). If @ref image2DForName()
* returns @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref image2D(UnsignedInt, UnsignedInt). Expects that a file is
* opened.
*/
Containers::Optional<ImageData2D> image2D(const std::string& name, UnsignedInt level = 0);
@ -1192,9 +1205,11 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi
* @m_since_latest
*
* A convenience API combining @ref image3DForName() and
* @ref image3D(UnsignedInt, UnsignedInt). Returns
* @ref Containers::NullOpt either if @ref image3DForName() returns
* @cpp -1 @ce or if importing fails. Expects that a file is opened.
* @ref image3D(UnsignedInt, UnsignedInt). If @ref image3DForName()
* returns @cpp -1 @ce, prints an error message and returns
* @ref Containers::NullOpt, otherwise propagates the result from
* @ref image3D(UnsignedInt, UnsignedInt). Expects that a file is
* opened.
*/
Containers::Optional<ImageData3D> image3D(const std::string& name, UnsignedInt level = 0);

105
src/Magnum/Trade/Test/AbstractImporterTest.cpp

@ -89,6 +89,7 @@ struct AbstractImporterTest: TestSuite::Tester {
void thingCountNoFile();
void thingForNameNotImplemented();
void thingForNameNoFile();
void thingByNameNotFound();
void thingNameNoFile();
void thingNoFile();
@ -242,6 +243,14 @@ struct AbstractImporterTest: TestSuite::Tester {
void debugFeatures();
};
constexpr struct {
const char* name;
bool checkMessage;
} ThingByNameData[] {
{"check it's not an assert", false},
{"verify the message", true}
};
AbstractImporterTest::AbstractImporterTest() {
addTests({&AbstractImporterTest::construct,
&AbstractImporterTest::constructWithPluginManagerReference,
@ -272,8 +281,12 @@ AbstractImporterTest::AbstractImporterTest() {
&AbstractImporterTest::thingCountNotImplemented,
&AbstractImporterTest::thingCountNoFile,
&AbstractImporterTest::thingForNameNotImplemented,
&AbstractImporterTest::thingForNameNoFile,
&AbstractImporterTest::thingNameNoFile,
&AbstractImporterTest::thingForNameNoFile});
addInstancedTests({&AbstractImporterTest::thingByNameNotFound},
Containers::arraySize(ThingByNameData));
addTests({&AbstractImporterTest::thingNameNoFile,
&AbstractImporterTest::thingNoFile,
&AbstractImporterTest::defaultScene,
@ -1047,6 +1060,58 @@ void AbstractImporterTest::thingForNameNoFile() {
"Trade::AbstractImporter::image3DForName(): no file opened\n");
}
void AbstractImporterTest::thingByNameNotFound() {
auto&& data = ThingByNameData[testCaseInstanceId()];
setTestCaseDescription(data.name);
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
bool doIsOpened() const override { return true; }
void doClose() override {}
} importer;
std::ostringstream out;
{
Containers::Optional<Error> redirectError;
if(data.checkMessage) redirectError.emplace(&out);
CORRADE_VERIFY(!importer.scene("foobar"));
CORRADE_VERIFY(!importer.animation("foobar"));
CORRADE_VERIFY(!importer.light("foobar"));
CORRADE_VERIFY(!importer.camera("foobar"));
CORRADE_VERIFY(!importer.object2D("foobar"));
CORRADE_VERIFY(!importer.object3D("foobar"));
CORRADE_VERIFY(!importer.mesh("foobar"));
CORRADE_VERIFY(!importer.material("foobar"));
CORRADE_VERIFY(!importer.texture("foobar"));
CORRADE_VERIFY(!importer.image1D("foobar"));
CORRADE_VERIFY(!importer.image2D("foobar"));
CORRADE_VERIFY(!importer.image3D("foobar"));
}
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::object2D(): object foobar not found\n"
"Trade::AbstractImporter::object3D(): object foobar not found\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::image1D(): image foobar not found\n"
"Trade::AbstractImporter::image2D(): image foobar not found\n"
"Trade::AbstractImporter::image3D(): image foobar not found\n");
}
}
void AbstractImporterTest::thingNameNoFile() {
struct: AbstractImporter {
ImporterFeatures doFeatures() const override { return {}; }
@ -1216,9 +1281,6 @@ void AbstractImporterTest::scene() {
auto data = importer.scene("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.scene("foo"));
}
}
@ -1320,9 +1382,6 @@ void AbstractImporterTest::animation() {
auto data = importer.animation("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.animation("foo"));
}
}
@ -1506,9 +1565,6 @@ void AbstractImporterTest::light() {
auto data = importer.light("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.light("foo"));
}
}
@ -1605,9 +1661,6 @@ void AbstractImporterTest::camera() {
auto data = importer.camera("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.camera("foo"));
}
}
@ -1704,9 +1757,6 @@ void AbstractImporterTest::object2D() {
auto data = importer.object2D("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.object2D("foo"));
}
}
@ -1803,9 +1853,6 @@ void AbstractImporterTest::object3D() {
auto data = importer.object3D("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.object3D("foo"));
}
}
@ -1908,9 +1955,6 @@ void AbstractImporterTest::mesh() {
auto data = importer.mesh("eighth", 2);
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.mesh("foo"));
}
}
@ -2683,9 +2727,6 @@ void AbstractImporterTest::material() {
auto data = importer.material("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.material("foo"));
}
}
void AbstractImporterTest::materialNameNotImplemented() {
@ -2781,9 +2822,6 @@ void AbstractImporterTest::texture() {
auto data = importer.texture("eighth");
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.texture("foo"));
}
}
@ -2885,9 +2923,6 @@ void AbstractImporterTest::image1D() {
auto data = importer.image1D("eighth", 2);
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.image1D("foo"));
}
}
@ -3121,9 +3156,6 @@ void AbstractImporterTest::image2D() {
auto data = importer.image2D("eighth", 2);
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.image2D("foo"));
}
}
@ -3357,9 +3389,6 @@ void AbstractImporterTest::image3D() {
auto data = importer.image3D("eighth", 2);
CORRADE_VERIFY(data);
CORRADE_COMPARE(data->importerState(), &state);
} {
/* This should fail gracefully, not assert */
CORRADE_VERIFY(!importer.image3D("foo"));
}
}

Loading…
Cancel
Save