Browse Source

Trade: another case for the deprecated path not being 100% compatible.

Honestly I don't care much, this is just that the original
PrimitiveImporter tests started to fail because they expected
object3DForName() to return -1 for a 2D name, but that's no longer the
case. It would be possible to fix this, but I doubt anyone ever relied
on such behavior for 2D scenes, so just add a test that acknowledges
this new behavior.
pull/525/head
Vladimír Vondruš 5 years ago
parent
commit
e0893f87ac
  1. 12
      src/Magnum/Trade/AbstractImporter.cpp
  2. 56
      src/Magnum/Trade/Test/AbstractImporterTest.cpp

12
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;

56
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<SceneData> 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() {

Loading…
Cancel
Save