From ddb718541744864fb92cfb6698a1e2b225807d15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Tue, 1 Sep 2020 00:14:47 +0200 Subject: [PATCH] Trade: make AbstractImporter::defaultScene() const. Makes more sense as the function isn't expected to fail (and thus any kind of lazy population is not possible as it would be too late for error checks anyway). *Not* updating interface strings even though this is an ABI break because we're doing that right after the skin import interface bump. --- doc/changelog.dox | 6 ++++++ src/Magnum/Trade/AbstractImporter.cpp | 4 ++-- src/Magnum/Trade/AbstractImporter.h | 12 ++++++------ src/Magnum/Trade/Test/AbstractImporterTest.cpp | 2 +- .../AnySceneImporter/AnySceneImporter.cpp | 2 +- .../AnySceneImporter/AnySceneImporter.h | 2 +- 6 files changed, 17 insertions(+), 11 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 43753dc45..0c543693c 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -248,6 +248,12 @@ See also: @cpp Trade::AbstractMaterialData @ce aliases to, doesn't have a @cpp virtual @ce destructor as subclasses with extra data members aren't a desired use case anymore. +- @ref Trade::AbstractImporter::doDefaultScene() is now @cpp const @ce --- + since the function is not expected to fail, no kind of complex lazy + population can be done anyway. This is now consistent with `do*Count()` + interfaces, which are also @cpp const @ce and can't fail. Documentation of + each function was expanded to suggest a recommended place for potential + error handling. @section changelog-2020-06 2020.06 diff --git a/src/Magnum/Trade/AbstractImporter.cpp b/src/Magnum/Trade/AbstractImporter.cpp index 28dae34b1..2e2c819f5 100644 --- a/src/Magnum/Trade/AbstractImporter.cpp +++ b/src/Magnum/Trade/AbstractImporter.cpp @@ -212,12 +212,12 @@ void AbstractImporter::close() { } } -Int AbstractImporter::defaultScene() { +Int AbstractImporter::defaultScene() const { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::defaultScene(): no file opened", -1); return doDefaultScene(); } -Int AbstractImporter::doDefaultScene() { return -1; } +Int AbstractImporter::doDefaultScene() const { return -1; } UnsignedInt AbstractImporter::sceneCount() const { CORRADE_ASSERT(isOpened(), "Trade::AbstractImporter::sceneCount(): no file opened", 0); diff --git a/src/Magnum/Trade/AbstractImporter.h b/src/Magnum/Trade/AbstractImporter.h index 6e572cb2d..481b286b4 100644 --- a/src/Magnum/Trade/AbstractImporter.h +++ b/src/Magnum/Trade/AbstractImporter.h @@ -537,11 +537,8 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi * When there is more than one scene, returns ID of the default one. * If there is no default scene, returns @cpp -1 @ce. Expects that a * file is opened. - * - * @note The function is not const, because the value will probably - * be lazy-populated. */ - Int defaultScene(); + Int defaultScene() const; /** * @brief Scene count @@ -1475,9 +1472,12 @@ class MAGNUM_TRADE_EXPORT AbstractImporter: public PluginManager::AbstractManagi /** * @brief Implementation for @ref defaultScene() * - * Default implementation returns @cpp -1 @ce. + * Default implementation returns @cpp -1 @ce. This function isn't + * expected to fail --- if an import error occus (for example because + * the default scene index is out of bounds), it should be handled + * already during file opening. */ - virtual Int doDefaultScene(); + virtual Int doDefaultScene() const; /** * @brief Implementation for @ref sceneCount() diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index 84a755b08..92cd8c6be 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -1431,7 +1431,7 @@ void AbstractImporterTest::defaultScene() { bool doIsOpened() const override { return true; } void doClose() override {} - Int doDefaultScene() override { return 42; } + Int doDefaultScene() const override { return 42; } } importer; CORRADE_COMPARE(importer.defaultScene(), 42); diff --git a/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.cpp b/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.cpp index b3d23b755..872659d0c 100644 --- a/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.cpp +++ b/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.cpp @@ -165,7 +165,7 @@ Int AnySceneImporter::doAnimationForName(const std::string& name) { return _in-> std::string AnySceneImporter::doAnimationName(const UnsignedInt id) { return _in->animationName(id); } Containers::Optional AnySceneImporter::doAnimation(const UnsignedInt id) { return _in->animation(id); } -Int AnySceneImporter::doDefaultScene() { return _in->defaultScene(); } +Int AnySceneImporter::doDefaultScene() const { return _in->defaultScene(); } UnsignedInt AnySceneImporter::doSceneCount() const { return _in->sceneCount(); } Int AnySceneImporter::doSceneForName(const std::string& name) { return _in->sceneForName(name); } diff --git a/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.h b/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.h index 6a68182a6..0de8e0163 100644 --- a/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.h +++ b/src/MagnumPlugins/AnySceneImporter/AnySceneImporter.h @@ -149,7 +149,7 @@ class MAGNUM_ANYSCENEIMPORTER_EXPORT AnySceneImporter: public AbstractImporter { MAGNUM_ANYSCENEIMPORTER_LOCAL Int doAnimationForName(const std::string& name) override; MAGNUM_ANYSCENEIMPORTER_LOCAL Containers::Optional doAnimation(UnsignedInt id) override; - MAGNUM_ANYSCENEIMPORTER_LOCAL Int doDefaultScene() override; + MAGNUM_ANYSCENEIMPORTER_LOCAL Int doDefaultScene() const override; MAGNUM_ANYSCENEIMPORTER_LOCAL UnsignedInt doSceneCount() const override; MAGNUM_ANYSCENEIMPORTER_LOCAL Int doSceneForName(const std::string& name) override;