[Resolved] Turrets always call writePacketData when turning (now with code fix)

Clipping the old post because it was speculative and I now know what the problem was and how to fix it.

Turrets will always call writePacketData when they're turning. This suggests an out of sync problem, but there is no sync issue. Instead turrets lack a "render" version of the rotation data (F32 TurretShape::mRot). This means that when interpolation is performed the actual sim rotation values are altered, resulting in the client appearing to be out of sync when the writePacketData checksum comparison is done.

The solution is to create a second mRot value for storing only client-side interpolated rotation for rendering and never use this value for simulation (this is the same as the distinction between a "transform" and a "render transform").

The following fix seems to resolve the issue. This will eliminate stuttering when turning turrets as well as significantly reducing network overhead by avoiding a constant stream of writePacketData corrections.

in turretShape.h we need to add a second Point3F for storing the "render" rotation to TurretShape. I called mine mRenderRot:

...
   Point3F mRot;           ///< Current heading and pitch
   // [HNT] turret out of sync fix
   Point3F mRenderRot;     ///< for interpolation and rendering
   // [/HNT]   bool mPitchAllowed;     ///< Are pitch changes allowed
   bool mHeadingAllowed;   ///< Are heading changes allowed
   ...

we also need a new function to set the render rotation:

...
   void _setRotation(const Point3F& rot);
   // [HNT] turret sync fix
   void _setRenderRotation(const Point3F& rot);
   // [/HNT]
   void _updateNodes(const Point3F& rot);
   ...

Done with turretShape.h, now to turretShape.cpp: initialize mRenderRot (TurretShape::TurretShape()):

...
   mRot = mTurretDelta.rot;
   // [HNT] turret sync fix
   mRenderRot = mRot;
   // [/HNT]
   ...

And here's that function added to the header (put wherever you like, I prefer right after _setRotation)

...
// [HNT]
void TurretShape::_setRenderRotation(const Point3F& rot)
{
   _updateNodes(rot);

   mShapeInstance->animate();

   mRenderRot = rot;
}
// [/HNT]
...

Now the actual functional changes. First up, at the end of interpolateTick call the new _setRenderRotation function instead of _setRotation:

...
   _applyLimits(rot);

   // [RPG][RTS]
   _setRenderRotation(rot);
   // [/RPG][/RTS
}

in updateMove there's a condition that skips setting the rotation on the client, assuming that interpolatetick will do it later. What we actually want to do is set the simulation rotation here, and the render rotation in interpolatetick (as in the previous code change):

...
   if (isServerObject())
   {
      // As this ends up animating shape nodes, we have no sense of a transform and
      // render transform.  Therefore we treat this as the true transform and leave the
      // client shape node changes to interpolateTick() as the render transform.  Otherwise
      // on the client we'll have this node change from processTick() and then backstepping
      // and catching up to the true node change in interpolateTick(), which causes the
      // turret to stutter.
      _setRotation( mRot );
   }
   else
   {
      // If on the client, calc delta for backstepping
      mTurretDelta.rot = mRot;
      mTurretDelta.rotVec = mTurretDelta.rotVec - mTurretDelta.rot;
      // [HNT] turret sync fix
      _setRotation( mRot );
      // [/HNT]
   }
   ...

That condition is getting a little redundant now; Clean it up if you want, but remember that we still only want to do the mTurretDelta stuff on the client. The long comment up there actually addresses the issue I'm fixing, but the conclusion is wrong. The original author must have run into the stutter issue and found a workaround that worked well enough (in single player), but it still was triggering writePacketData which means it didn't actually work.

A few changes in updateAnimation. This is where we want to use the interpolated render rotation instead of the sim rotation.

void TurretShape::updateAnimation(F32 dt)
{
   if (mRecoilThread)
      mShapeInstance->advanceTime(dt,mRecoilThread);
   if (mImageStateThread)
      mShapeInstance->advanceTime(dt,mImageStateThread);

   // Update any pitch and heading threads
   if (mPitchThread)
   {
      F32 d = mPitchDown - mPitchUp;
      if (!mIsZero(d))
      {
         // [HNT] turret sync fix
         F32 pos = (mRenderRot.x - mPitchUp) / d; // was mRot.x
         // [/HNT]
         mShapeInstance->setPos(mPitchThread, mClampF(pos, 0.0f, 1.0f));
      }
   }
   if (mHeadingThread)
   {
      F32 pos = 0.0f;
      if (mHeadingMax < mDegToRad(180.0f))
      {
         F32 d = mHeadingMax * 2.0f;
         if (!mIsZero(d))
         {
            // [HNT] turret sync fix
            pos = (mRenderRot.z + mHeadingMax) / d; // was mRot.z
            // [/HNT]
         }
      }
      else
      {
         // [HNT] turret sync fix
         pos = mRenderRot.z / M_2PI; // was mRot.z
         // [/HNT]
         if (pos < 0.0f)
         {
            // We don't want negative rotations to simply mirror the
            // positive rotations but to animate into them as if -0.0
            // is equivalent to 1.0.
            pos = mFmod(pos, 1.0f) + 1.0f;
         }
         if (pos > 1.0f)
         {
            pos = mFmod(pos, 1.0f);
         }
      }
      mShapeInstance->setPos(mHeadingThread, mClampF(pos, 0.0f, 1.0f));
   }
}

And we probably want to use the interpolated rotation in this one line during advanceTime:

...
   if (updateNodes)
   {
      // [HNT] turret sync fix
      _updateNodes(mRenderRot); // was mRot
      // [/HNT]
   }
   ...

And hopefully I didn't miss anything. Let me know if there are any issues with this code. I'm using turrets extensively in a project+ so if there are problems I'll likely spot them and be back with fixes

Be aware that this issue also applies to the old turret resource that's been in circulation since the TGE days. A similar fix should work there.