From 22229dd674339ef7eeaf479579a08f6c236ad20d Mon Sep 17 00:00:00 2001
From: Sam Hellawell <sshellawell@gmail.com>
Date: Sat, 20 Apr 2024 20:08:50 +0100
Subject: [PATCH] Cleanup, use string_view comparison

---
 apps/openmw/mwrender/animation.cpp           | 10 +++-------
 apps/openmw/mwrender/animblendcontroller.cpp | 20 ++++++--------------
 2 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/apps/openmw/mwrender/animation.cpp b/apps/openmw/mwrender/animation.cpp
index 9afc5baafa..9b9236cde5 100644
--- a/apps/openmw/mwrender/animation.cpp
+++ b/apps/openmw/mwrender/animation.cpp
@@ -421,7 +421,7 @@ namespace MWRender
             osg::Callback* updateCb = bone->getUpdateCallback();
             while (updateCb)
             {
-                if (updateCb->className() == std::string(controller->className()))
+                if (updateCb->className() == std::string_view(controller->className()))
                 {
                     osg::ref_ptr<osg::Callback> nextCb = updateCb->getNestedCallback();
                     bone->removeUpdateCallback(updateCb);
@@ -438,7 +438,7 @@ namespace MWRender
             updateCb = bone->getUpdateCallback();
             while (updateCb)
             {
-                if (updateCb->className() == std::string("UpdateBone"))
+                if (updateCb->className() == std::string_view("UpdateBone"))
                 {
                     // Override the immediate callback after the UpdateBone
                     osg::ref_ptr<osg::Callback> lastCb = updateCb->getNestedCallback();
@@ -452,7 +452,7 @@ namespace MWRender
             }
         }
 
-        // Traverse childrne if this is a group
+        // Traverse child bones if this is a group
         osg::Group* group = parent->asGroup();
         if (group)
             for (unsigned int i = 0; i < group->getNumChildren(); ++i)
@@ -900,8 +900,6 @@ namespace MWRender
         if (!mObjectRoot || mAnimSources.empty())
             return;
 
-        // Log(Debug::Info) << "Please play: " << groupname << ":" << start << "..." << stop << " mask: " << blendMask;
-
         if (groupname.empty())
         {
             resetActiveGroups();
@@ -918,8 +916,6 @@ namespace MWRender
         while (stateiter != mStates.end())
         {
             if (stateiter->second.mPriority == priority && stateiter->first != groupname)
-                // This MIGH be a problem since we want old states to be still running so the AnimBlendingController can
-                // blend them properly
                 mStates.erase(stateiter++);
             else
                 ++stateiter;
diff --git a/apps/openmw/mwrender/animblendcontroller.cpp b/apps/openmw/mwrender/animblendcontroller.cpp
index 06c18c9ee9..5edd6c8e75 100644
--- a/apps/openmw/mwrender/animblendcontroller.cpp
+++ b/apps/openmw/mwrender/animblendcontroller.cpp
@@ -82,7 +82,6 @@ namespace MWRender
 
     namespace
     {
-        // Helper methods
         osg::Vec3f vec3fLerp(float t, const osg::Vec3f& A, const osg::Vec3f& B)
         {
             return A + (B - A) * t;
@@ -103,11 +102,12 @@ namespace MWRender
     void AnimBlendControllerBase<NodeClass>::setKeyframeTrack(osg::ref_ptr<SceneUtil::KeyframeController> kft,
         AnimBlendStateData newState, osg::ref_ptr<const SceneUtil::AnimBlendRules> blendRules)
     {
+        // If aimation has changed, start blending
         if (newState.mGroupname != mAnimState.mGroupname || newState.mStartKey != mAnimState.mStartKey
             || kft != mKeyframeTrack)
         {
-            // Animation have changed, start blending!
-            // Log(Debug::Info) << "Animation change to: " << newState.mGroupname << ":" << newState.mStartKey;
+            // Allow logging of cahnge to aid with implementing animations for developers/modders
+            // Log(Debug::Verbose) << "Animation change to: " << newState.mGroupname << ":" << newState.mStartKey;
 
             // Default blend settings
             mBlendDuration = 0;
@@ -118,11 +118,7 @@ namespace MWRender
                 // Finds a matching blend rule either in this or previous ruleset
                 auto blendRule = blendRules->findBlendingRule(
                     mAnimState.mGroupname, mAnimState.mStartKey, newState.mGroupname, newState.mStartKey);
-                // This will also check the previous ruleset, not sure it's a good idea though, commenting out
-                // for now.
-                /*if (!blendRule && mAnimBlendRules)
-                    blendRule = mAnimBlendRules->findBlendingRule(
-                        mAnimState.mGroupname, mAnimState.mStartKey, newState.mGroupname, newState.mStartKey);*/
+
                 if (blendRule)
                 {
                     if (Easings::easingsMap.contains(blendRule->mEasing))
@@ -171,10 +167,10 @@ namespace MWRender
 
         // Shouldnt happen, but potentially an edge case where a new bone was added
         // between gatherRecursiveBoneTransforms and this update
-        // so far OpenMW will never do this, so this check shouldn't be needed in production
+        // currently OpenMW will never do this, but potentially useful
         assert(mBlendBoneTransforms.find(bone) != mBlendBoneTransforms.end());
 
-        // every frame the osgAnimation controller updates this
+        // Every frame the osgAnimation controller updates this
         // so it is ok that we update it directly below
         osg::Matrixf currentSampledMatrix = bone->getMatrix();
         const osg::Matrixf& lastSampledMatrix = mBlendBoneTransforms.at(bone);
@@ -275,7 +271,6 @@ namespace MWRender
 
         if (mInterpActive)
         {
-            // Interpolate node's rotation
             if (rotation)
             {
                 osg::Quat lerpedRot;
@@ -288,7 +283,6 @@ namespace MWRender
                 node->setRotation(node->mRotation);
             }
 
-            // Update node's translation
             if (translation)
             {
                 osg::Vec3f lerpedTrans = vec3fLerp(mInterpFactor, mBlendStartTrans, *translation);
@@ -297,7 +291,6 @@ namespace MWRender
         }
         else
         {
-            // Update node's translation
             if (translation)
                 node->setTranslation(*translation);
 
@@ -307,7 +300,6 @@ namespace MWRender
                 node->setRotation(node->mRotation);
         }
 
-        // Update node's scale
         if (scale)
             // Scale is not lerped based on the idea that it is much more likely that scale animation will be used to
             // instantly hide/show objects in which case the scale interpolation is undesirable.