From 93e6dc2c544ac1945de47dfb9a5b545b7c919f70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Thu, 21 Nov 2019 21:51:47 +0100 Subject: [PATCH] Trade: make AnimationTrackData constructors explicit. There's a ton of parameters and it's just unreadable without. --- doc/changelog.dox | 2 + src/Magnum/Trade/AnimationData.h | 8 +- .../Trade/Test/AbstractImporterTest.cpp | 4 +- src/Magnum/Trade/Test/AnimationDataTest.cpp | 116 +++++++++--------- 4 files changed, 66 insertions(+), 64 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index b6fca800d..0e293d4b0 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -412,6 +412,8 @@ See also: - The @ref Magnum/Math/FunctionsBatch.h header is no longer included from @ref Magnum/Math/Functions.h for backwards compatibility in order to speed up compile times. +- @ref Trade::AnimationTrackData constructors are now explicit as that + enforces better readability in long initializer expressions - Non-const @ref Trade::ImageData::data() and @ref Trade::ImageData::pixels() were renamed to @ref Trade::ImageData::mutableData() and @ref Trade::ImageData::mutablePixels() to follow the new diff --git a/src/Magnum/Trade/AnimationData.h b/src/Magnum/Trade/AnimationData.h index d8bfd5688..50f7b51de 100644 --- a/src/Magnum/Trade/AnimationData.h +++ b/src/Magnum/Trade/AnimationData.h @@ -228,7 +228,7 @@ class AnimationTrackData { * initialization of the track array for @ref AnimationData, expected * to be replaced with concrete values later. */ - /*implicit*/ AnimationTrackData() noexcept: _type{}, _resultType{}, _targetType{}, _target{}, _view{} {} + explicit AnimationTrackData() noexcept: _type{}, _resultType{}, _targetType{}, _target{}, _view{} {} /** * @brief Type-erased constructor @@ -238,14 +238,14 @@ class AnimationTrackData { * @param target Track target * @param view Type-erased @ref Animation::TrackView instance */ - /*implicit*/ AnimationTrackData(AnimationTrackType type, AnimationTrackType resultType, AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackViewStorage view) noexcept: _type{type}, _resultType{resultType}, _targetType{targetType}, _target{target}, _view{view} {} + explicit AnimationTrackData(AnimationTrackType type, AnimationTrackType resultType, AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackViewStorage view) noexcept: _type{type}, _resultType{resultType}, _targetType{targetType}, _target{target}, _view{view} {} /** @overload * * Equivalent to the above with @p type used as both value type and * result type. */ - /*implicit*/ AnimationTrackData(AnimationTrackType type, AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackViewStorage view) noexcept: _type{type}, _resultType{type}, _targetType{targetType}, _target{target}, _view{view} {} + explicit AnimationTrackData(AnimationTrackType type, AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackViewStorage view) noexcept: _type{type}, _resultType{type}, _targetType{targetType}, _target{target}, _view{view} {} /** * @brief Constructor @@ -257,7 +257,7 @@ class AnimationTrackData { * Detects @ref AnimationTrackType from @p view type and delegates to * @ref AnimationTrackData(AnimationTrackType, AnimationTrackType, AnimationTrackTargetType, UnsignedInt, Animation::TrackViewStorage). */ - template /*implicit*/ AnimationTrackData(AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackView view) noexcept; + template explicit AnimationTrackData(AnimationTrackTargetType targetType, UnsignedInt target, Animation::TrackView view) noexcept; private: friend AnimationData; diff --git a/src/Magnum/Trade/Test/AbstractImporterTest.cpp b/src/Magnum/Trade/Test/AbstractImporterTest.cpp index f714854ce..f3b97bade 100644 --- a/src/Magnum/Trade/Test/AbstractImporterTest.cpp +++ b/src/Magnum/Trade/Test/AbstractImporterTest.cpp @@ -1231,8 +1231,8 @@ void AbstractImporterTest::animation() { /* Verify that initializer list is converted to an array with the default deleter and not something disallowed */ if(id == 7) return AnimationData{nullptr, { - {AnimationTrackType::Vector3, - AnimationTrackTargetType::Scaling3D, 0, {}} + AnimationTrackData{AnimationTrackType::Vector3, + AnimationTrackTargetType::Scaling3D, 0, {}} }, &state}; else return AnimationData{{}, {}}; } diff --git a/src/Magnum/Trade/Test/AnimationDataTest.cpp b/src/Magnum/Trade/Test/AnimationDataTest.cpp index 96ae3c3e5..6cf570c5e 100644 --- a/src/Magnum/Trade/Test/AnimationDataTest.cpp +++ b/src/Magnum/Trade/Test/AnimationDataTest.cpp @@ -177,18 +177,18 @@ void AnimationDataTest::construct() { const int state = 5; AnimationData data{std::move(buffer), { - {AnimationTrackTargetType::Translation3D, 42, - Animation::TrackView{ - {view, &view[0].time, view.size(), sizeof(Data)}, - {view, &view[0].position, view.size(), sizeof(Data)}, - Animation::Interpolation::Constant, - animationInterpolatorFor(Animation::Interpolation::Constant)}}, - {AnimationTrackTargetType::Rotation3D, 1337, - Animation::TrackView{ - {view, &view[0].time, view.size(), sizeof(Data)}, - {view, &view[0].rotation, view.size(), sizeof(Data)}, - Animation::Interpolation::Linear, - animationInterpolatorFor(Animation::Interpolation::Linear)}} + AnimationTrackData{AnimationTrackTargetType::Translation3D, 42, + Animation::TrackView{ + {view, &view[0].time, view.size(), sizeof(Data)}, + {view, &view[0].position, view.size(), sizeof(Data)}, + Animation::Interpolation::Constant, + animationInterpolatorFor(Animation::Interpolation::Constant)}}, + AnimationTrackData{AnimationTrackTargetType::Rotation3D, 1337, + Animation::TrackView{ + {view, &view[0].time, view.size(), sizeof(Data)}, + {view, &view[0].rotation, view.size(), sizeof(Data)}, + Animation::Interpolation::Linear, + animationInterpolatorFor(Animation::Interpolation::Linear)}} }, {-1.0f, 7.0f}, &state}; CORRADE_COMPARE(data.dataFlags(), DataFlag::Owned|DataFlag::Mutable); @@ -245,16 +245,16 @@ void AnimationDataTest::constructImplicitDuration() { const int state = 5; AnimationData data{std::move(buffer), { - {AnimationTrackTargetType(129), 0, - Animation::TrackView{ - {view, &view[0].time, 2, sizeof(Data)}, - {view, &view[0].value, 2, sizeof(Data)}, - Animation::Interpolation::Constant}}, - {AnimationTrackTargetType(130), 1, - Animation::TrackView{ - {view, &view[2].time, 2, sizeof(Data)}, - {view, &view[2].value, 2, sizeof(Data)}, - Animation::Interpolation::Linear}} + AnimationTrackData{AnimationTrackTargetType(129), 0, + Animation::TrackView{ + {view, &view[0].time, 2, sizeof(Data)}, + {view, &view[0].value, 2, sizeof(Data)}, + Animation::Interpolation::Constant}}, + AnimationTrackData{AnimationTrackTargetType(130), 1, + Animation::TrackView{ + {view, &view[2].time, 2, sizeof(Data)}, + {view, &view[2].value, 2, sizeof(Data)}, + Animation::Interpolation::Linear}} }, &state}; CORRADE_COMPARE(data.dataFlags(), DataFlag::Owned|DataFlag::Mutable); @@ -313,11 +313,11 @@ void AnimationDataTest::constructNotOwned() { const int state = 5; AnimationData data{instanceData.dataFlags, keyframes, { - {AnimationTrackTargetType::Translation3D, 42, - Animation::TrackView{ - keyframes, - Animation::Interpolation::Constant, - animationInterpolatorFor(Animation::Interpolation::Constant)}} + AnimationTrackData{AnimationTrackTargetType::Translation3D, 42, + Animation::TrackView{ + keyframes, + Animation::Interpolation::Constant, + animationInterpolatorFor(Animation::Interpolation::Constant)}} }, {-1.0f, 7.0f}, &state}; CORRADE_COMPARE(data.dataFlags(), instanceData.dataFlags); @@ -361,8 +361,8 @@ void AnimationDataTest::constructImplicitDurationNotOwned() { const int state = 5; AnimationData data{instanceData.dataFlags, keyframes, { - {AnimationTrackTargetType(129), 0, - Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, + AnimationTrackData{AnimationTrackTargetType(129), 0, + Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }, &state}; CORRADE_COMPARE(data.dataFlags(), instanceData.dataFlags); @@ -433,18 +433,18 @@ void AnimationDataTest::constructMove() { const int state = 5; AnimationData a{std::move(buffer), { - {AnimationTrackTargetType::Translation3D, 42, - Animation::TrackView{ - {view, &view[0].time, view.size(), sizeof(Data)}, - {view, &view[0].position, view.size(), sizeof(Data)}, - Animation::Interpolation::Constant, - animationInterpolatorFor(Animation::Interpolation::Constant)}}, - {AnimationTrackTargetType::Rotation3D, 1337, - Animation::TrackView{ - {view, &view[0].time, view.size(), sizeof(Data)}, - {view, &view[0].rotation, view.size(), sizeof(Data)}, - Animation::Interpolation::Linear, - animationInterpolatorFor(Animation::Interpolation::Linear)}} + AnimationTrackData{AnimationTrackTargetType::Translation3D, 42, + Animation::TrackView{ + {view, &view[0].time, view.size(), sizeof(Data)}, + {view, &view[0].position, view.size(), sizeof(Data)}, + Animation::Interpolation::Constant, + animationInterpolatorFor(Animation::Interpolation::Constant)}}, + AnimationTrackData{AnimationTrackTargetType::Rotation3D, 1337, + Animation::TrackView{ + {view, &view[0].time, view.size(), sizeof(Data)}, + {view, &view[0].rotation, view.size(), sizeof(Data)}, + Animation::Interpolation::Linear, + animationInterpolatorFor(Animation::Interpolation::Linear)}} }, {-1.0f, 7.0f}, &state}; AnimationData b{std::move(a)}; @@ -522,8 +522,8 @@ void AnimationDataTest::mutableAccessNotAllowed() { }; AnimationData data{{}, keyframes, { - {AnimationTrackTargetType(129), 0, - Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, + AnimationTrackData{AnimationTrackTargetType(129), 0, + Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }}; CORRADE_COMPARE(data.dataFlags(), DataFlags{}); @@ -551,13 +551,13 @@ void AnimationDataTest::trackCustomResultType() { view[1] = {5.0f, {30, 60, 100}}; AnimationData data{std::move(buffer), { - {AnimationTrackTargetType::Scaling3D, 0, - Animation::TrackView{ - {view, &view[0].time, view.size(), sizeof(Data)}, - {view, &view[0].position, view.size(), sizeof(Data)}, - [](const Vector3i& a, const Vector3i& b, Float t) -> Vector3 { - return Math::lerp(Vector3{a}*0.01f, Vector3{b}*0.01f, t); - }}} + AnimationTrackData{AnimationTrackTargetType::Scaling3D, 0, + Animation::TrackView{ + {view, &view[0].time, view.size(), sizeof(Data)}, + {view, &view[0].position, view.size(), sizeof(Data)}, + [](const Vector3i& a, const Vector3i& b, Float t) -> Vector3 { + return Math::lerp(Vector3{a}*0.01f, Vector3{b}*0.01f, t); + }}} }}; CORRADE_COMPARE((data.track(0).at(2.5f)), (Vector3{1.65f, 0.8f, 0.55f})); @@ -587,9 +587,9 @@ void AnimationDataTest::trackWrongType() { Error redirectError{&out}; AnimationData data{nullptr, { - {AnimationTrackType::Vector3i, - AnimationTrackType::Vector3, - AnimationTrackTargetType::Scaling3D, 0, {}} + AnimationTrackData{AnimationTrackType::Vector3i, + AnimationTrackType::Vector3, + AnimationTrackTargetType::Scaling3D, 0, {}} }}; data.track(0); @@ -602,9 +602,9 @@ void AnimationDataTest::trackWrongResultType() { Error redirectError{&out}; AnimationData data{nullptr, { - {AnimationTrackType::Vector3i, - AnimationTrackType::Vector3, - AnimationTrackTargetType::Scaling3D, 0, {}} + AnimationTrackData{AnimationTrackType::Vector3i, + AnimationTrackType::Vector3, + AnimationTrackTargetType::Scaling3D, 0, {}} }}; data.track(0); @@ -619,8 +619,8 @@ void AnimationDataTest::release() { }; AnimationData data{{}, keyframes, { - {AnimationTrackTargetType(129), 0, - Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, + AnimationTrackData{AnimationTrackTargetType(129), 0, + Animation::TrackView{keyframes, Animation::Interpolation::Constant}}, }}; CORRADE_COMPARE(data.trackCount(), 1);