diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index cda75437e..ec15b7ef2 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -589,7 +589,11 @@ Int AbstractImporter::object2DForName(const std::string& name) { Int AbstractImporter::doObject2DForName(const std::string& name) { /* Alias to the new interface. If it returns an ID that's larger than reported 2D object count, then it's probably for a 3D object instead - -- ignore it in that case. */ + -- ignore it in that case. Ideally this would be solved by checking if + the ID is actually present in a 2D scene (and same in doObject2DName()) + but that's a lot of extra code for just a backwards compatibility + feature that almost nobody needs. The only pre-existing 2D importer is + PrimitiveImporter, which is rarely used. */ const Long id = doObjectForName(name); CORRADE_IGNORE_DEPRECATED_PUSH return id < doObject2DCount() ? id : -1; @@ -763,7 +767,11 @@ Int AbstractImporter::object3DForName(const std::string& name) { Int AbstractImporter::doObject3DForName(const std::string& name) { /* Alias to the new interface. If it returns an ID that's larger than reported 3D object count, then it's probably for a 2D object instead - -- ignore it in that case. */ + -- ignore it in that case. Ideally this would be solved by checking if + the ID is actually present in a 3D scene (and same in doObject3DName()) + but that's a lot of extra code for just a backwards compatibility + feature that almost nobody needs. The only pre-existing 2D importer is + PrimitiveImporter, which is rarely used. */ const Long id = doObjectForName(name); CORRADE_IGNORE_DEPRECATED_PUSH return id < doObject3DCount() ? id : -1; diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 627ba15a9..d549cf7db 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -128,6 +128,7 @@ struct AbstractImporterTest: TestSuite::Tester { void sceneDeprecatedFallbackMultiFunctionObjects3D(); void sceneDeprecatedFallbackObjectCountNoScenes(); void sceneDeprecatedFallbackObjectCountAllSceneImportFailed(); + void sceneDeprecatedFallbackBoth2DAnd3DScene(); #endif void sceneNameNotImplemented(); void objectNameNotImplemented(); @@ -414,6 +415,7 @@ AbstractImporterTest::AbstractImporterTest() { &AbstractImporterTest::sceneDeprecatedFallbackMultiFunctionObjects3D, &AbstractImporterTest::sceneDeprecatedFallbackObjectCountNoScenes, &AbstractImporterTest::sceneDeprecatedFallbackObjectCountAllSceneImportFailed, + &AbstractImporterTest::sceneDeprecatedFallbackBoth2DAnd3DScene, #endif &AbstractImporterTest::sceneForNameOutOfRange, &AbstractImporterTest::objectForNameOutOfRange, @@ -3262,6 +3264,60 @@ void AbstractImporterTest::sceneDeprecatedFallbackObjectCountAllSceneImportFaile CORRADE_COMPARE(importer.object3DCount(), 27); CORRADE_IGNORE_DEPRECATED_POP } + +void AbstractImporterTest::sceneDeprecatedFallbackBoth2DAnd3DScene() { + struct: AbstractImporter { + ImporterFeatures doFeatures() const override { return {}; } + bool doIsOpened() const override { return true; } + void doClose() override {} + + UnsignedInt doSceneCount() const override { return 2; } + UnsignedLong doObjectCount() const override { return 7; } + Long doObjectForName(const std::string& name) override { + if(name == "sixth") return 5; + return -1; + } + std::string doObjectName(UnsignedLong id) override { + if(id == 5) return "sixth"; + return {}; + } + Containers::Optional doScene(UnsignedInt id) override { + if(id == 0) return SceneData{SceneObjectType::UnsignedInt, 7, nullptr, { + SceneFieldData{SceneField::Parent, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Int, nullptr}, + SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Vector2, nullptr}, + }}; + if(id == 1) return SceneData{SceneObjectType::UnsignedInt, 7, nullptr, { + SceneFieldData{SceneField::Parent, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Int, nullptr}, + SceneFieldData{SceneField::Translation, SceneObjectType::UnsignedInt, nullptr, SceneFieldType::Vector3, nullptr}, + }}; + CORRADE_INTERNAL_ASSERT_UNREACHABLE(); + } + } importer; + + CORRADE_IGNORE_DEPRECATED_PUSH + CORRADE_COMPARE(importer.object2DCount(), 7); + CORRADE_COMPARE(importer.object3DCount(), 7); + CORRADE_IGNORE_DEPRECATED_POP + + { + CORRADE_EXPECT_FAIL("No check for whether given object is 2D or 3D is done, so the names are reported for both 2D and 3D objects."); + CORRADE_IGNORE_DEPRECATED_PUSH + CORRADE_COMPARE(importer.object2DForName("sixth"), -1); + CORRADE_COMPARE(importer.object2DName(5), ""); + CORRADE_COMPARE(importer.object3DForName("sixth"), -1); + CORRADE_COMPARE(importer.object3DName(5), ""); + CORRADE_IGNORE_DEPRECATED_POP + } { + /* Just to be sure, verify that the names get really reported for both + instead of some other weird shit happening */ + CORRADE_IGNORE_DEPRECATED_PUSH + CORRADE_COMPARE(importer.object2DForName("sixth"), 5); + CORRADE_COMPARE(importer.object2DName(5), "sixth"); + CORRADE_COMPARE(importer.object3DForName("sixth"), 5); + CORRADE_COMPARE(importer.object3DName(5), "sixth"); + CORRADE_IGNORE_DEPRECATED_POP + } +} #endif void AbstractImporterTest::sceneNameNotImplemented() {