Minor tweaks and code clean up.

src/common/Common.h

@@ -629,7 +629,7 @@ namespace Sapphire::Common
      * @param value The actionid that starts/continues the combo. eg, 3617 will start a spinning slash and/or syphon strike combo
      */
     StartActionCombo = 28,
-    ComboVisualEffect = 29,
+    ComboSucceed = 29,
     Knockback = 33,
     Mount = 38,
     VFX = 59, // links to VFX sheet
@@ -645,6 +645,12 @@ namespace Sapphire::Common
     CritDirectHitDamage = 3
   };
 
+  enum class ActionEffectResultFlag : uint8_t
+  {
+    None = 0,
+    EffectOnSource = 0x80,
+  };
+
   enum ItemActionType : uint16_t
   {
     ItemActionVFX = 852,

src/world/Action/Action.cpp

@@ -468,14 +468,14 @@ void Action::Action::buildEffects()
     if( m_lutEntry.potency > 0 )
     {
       auto dmg = calcDamage( isCorrectCombo() ? m_lutEntry.comboPotency : m_lutEntry.potency );
-      m_effectBuilder->damageTarget( actor, dmg.first, dmg.second );
+      m_effectBuilder->damage( actor, actor, dmg.first, dmg.second );
 
       if( dmg.first > 0 )
         actor->onActionHostile( m_pSource );
 
       if( isCorrectCombo() && shouldShowComboEffect )
       {
-        m_effectBuilder->comboVisualEffect( actor );
+        m_effectBuilder->comboSucceed( actor );
         shouldShowComboEffect = false;
       }
 
@@ -483,12 +483,12 @@ void Action::Action::buildEffects()
       {
         if( m_lutEntry.curePotency > 0 ) // actions with self heal
         {
-          m_effectBuilder->selfHeal( actor, m_pSource, m_lutEntry.curePotency );
+          m_effectBuilder->heal( actor, m_pSource, m_lutEntry.curePotency, Common::ActionHitSeverityType::NormalHeal, Common::ActionEffectResultFlag::EffectOnSource );
         }
 
         if( m_lutEntry.restoreMPPercentage > 0 && shouldRestoreMP )
         {
-          m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100 );
+          m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100, Common::ActionEffectResultFlag::EffectOnSource );
           shouldRestoreMP = false;
         }
 
@@ -501,18 +501,17 @@ void Action::Action::buildEffects()
     else if( m_lutEntry.curePotency > 0 )
     {
       // todo: calcHealing()
-      m_effectBuilder->healTarget( actor, m_lutEntry.curePotency );
+      m_effectBuilder->heal( actor, actor, m_lutEntry.curePotency );
 
       if( m_lutEntry.restoreMPPercentage > 0 && shouldRestoreMP )
       {
-        // always restore caster mp I don't think there are any actions that can restore target MP post 5.0
-        m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100 );
+        m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100, Common::ActionEffectResultFlag::EffectOnSource );
         shouldRestoreMP = false;
       }
     }
     else if( m_lutEntry.restoreMPPercentage > 0 && shouldRestoreMP )
     {
-      m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100 );
+      m_effectBuilder->restoreMP( actor, m_pSource, m_pSource->getMaxMp() * m_lutEntry.restoreMPPercentage / 100, Common::ActionEffectResultFlag::EffectOnSource );
       shouldRestoreMP = false;
     }
   }
@@ -753,7 +752,7 @@ bool Action::Action::preFilterActor( Sapphire::Entity::Actor& actor ) const
   if( kind != ObjKind::BattleNpc && kind != ObjKind::Player )
     return false;
 
-  if( m_lutEntry.potency > 0 && chara == m_pSource )
+  if( m_lutEntry.potency > 0 && chara->getId() == m_pSource->getId() )
   {
     // damage action shouldn't hit self
     return false;

src/world/Action/EffectBuilder.cpp

@@ -47,43 +47,33 @@ std::shared_ptr< std::vector< EffectResultPtr > > EffectBuilder::getResultList(
   return it->second;
 }
 
-void EffectBuilder::healTarget( Entity::CharaPtr& target, uint32_t amount, Common::ActionHitSeverityType severity )
+void EffectBuilder::heal( Entity::CharaPtr& effectTarget, Entity::CharaPtr& healingTarget, uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
-  auto resultList = getResultList( target );
+  auto resultList = getResultList( effectTarget );
   assert( resultList );
 
-  EffectResultPtr nextResult = make_EffectResult( target, getResultDelayMs() );
-  nextResult->heal( amount, severity, false );
+  EffectResultPtr nextResult = make_EffectResult( healingTarget, getResultDelayMs() );
+  nextResult->heal( amount, severity, flag );
   resultList->push_back( std::move( nextResult ) );
 }
 
-void EffectBuilder::selfHeal( Entity::CharaPtr& target, Entity::CharaPtr& source, uint32_t amount, Common::ActionHitSeverityType severity )
+void EffectBuilder::restoreMP( Entity::CharaPtr& target, Entity::CharaPtr& restoringTarget, uint32_t amount, Common::ActionEffectResultFlag flag )
 {
   auto resultList = getResultList( target );
   assert( resultList );
 
-  EffectResultPtr nextResult = make_EffectResult( source, getResultDelayMs() ); // heal the source actor
-  nextResult->heal( amount, severity, true );
+  EffectResultPtr nextResult = make_EffectResult( restoringTarget, getResultDelayMs() ); // restore mp source actor
+  nextResult->restoreMP( amount, flag );
   resultList->push_back( std::move( nextResult ) );
 }
 
-void EffectBuilder::restoreMP( Entity::CharaPtr& target, Entity::CharaPtr& source, uint32_t amount )
+void EffectBuilder::damage( Entity::CharaPtr& effectTarget, Entity::CharaPtr& damagingTarget, uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
-  auto resultList = getResultList( target );
+  auto resultList = getResultList( effectTarget );
   assert( resultList );
 
-  EffectResultPtr nextResult = make_EffectResult( source, getResultDelayMs() ); // restore mp source actor
-  nextResult->restoreMP( amount );
-  resultList->push_back( std::move( nextResult ) );
-}
-
-void EffectBuilder::damageTarget( Entity::CharaPtr& target, uint32_t amount, Common::ActionHitSeverityType severity )
-{
-  auto resultList = getResultList( target );
-  assert( resultList );
-
-  EffectResultPtr nextResult = make_EffectResult( target, getResultDelayMs() );
-  nextResult->damage( amount, severity );
+  EffectResultPtr nextResult = make_EffectResult( damagingTarget, getResultDelayMs() );
+  nextResult->damage( amount, severity, flag );
   resultList->push_back( std::move( nextResult ) );
 }
 
@@ -97,13 +87,13 @@ void EffectBuilder::startCombo( Entity::CharaPtr& target, uint16_t actionId )
   resultList->push_back( std::move( nextResult ) );
 }
 
-void EffectBuilder::comboVisualEffect( Entity::CharaPtr& target )
+void EffectBuilder::comboSucceed( Entity::CharaPtr& target )
 {
   auto resultList = getResultList( target );
   assert( resultList  );
 
   EffectResultPtr nextResult = make_EffectResult( target, 0 );
-  nextResult->comboVisualEffect();
+  nextResult->comboSucceed();
   resultList->push_back( std::move( nextResult ) );
 }
 
@@ -144,40 +134,40 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( uint32_
       case 8:
       {
         auto p = makeZonePacket< Server::FFXIVIpcAoeEffect8 >( m_sourceChara->getId() );
-        pHeader = ( EffectHeader* )( &( p->data() ) );
-        pEntry = ( Common::EffectEntry* )( &( p->data().effects ) );
-        pEffectTargetId = ( uint64_t* )( &( p->data().effectTargetId ) );
-        pFlag = ( uint16_t* )( &( p->data().unkFlag ) );
+        pHeader = reinterpret_cast< EffectHeader* >( &p->data() );
+        pEntry = reinterpret_cast< Common::EffectEntry* >( &p->data().effects );
+        pEffectTargetId = reinterpret_cast< uint64_t* >( &p->data().effectTargetId );
+        pFlag = reinterpret_cast< uint16_t* >( &p->data().unkFlag );
         effectPacket = std::move( p );
         break;
       }
       case 16:
       {
         auto p = makeZonePacket< Server::FFXIVIpcAoeEffect16 >( m_sourceChara->getId() );
-        pHeader = ( EffectHeader* )( &( p->data() ) );
-        pEntry = ( Common::EffectEntry* )( &( p->data().effects ) );
-        pEffectTargetId = ( uint64_t* )( &( p->data().effectTargetId ) );
-        pFlag = ( uint16_t* )( &( p->data().unkFlag ) );
+        pHeader = reinterpret_cast< EffectHeader* >( &p->data() );
+        pEntry = reinterpret_cast< Common::EffectEntry* >( &p->data().effects );
+        pEffectTargetId = reinterpret_cast< uint64_t* >( &p->data().effectTargetId );
+        pFlag = reinterpret_cast< uint16_t* >( &p->data().unkFlag );
         effectPacket = std::move( p );
         break;
       }
       case 24:
       {
         auto p = makeZonePacket< Server::FFXIVIpcAoeEffect24 >( m_sourceChara->getId() );
-        pHeader = ( EffectHeader* )( &( p->data() ) );
-        pEntry = ( Common::EffectEntry* )( &( p->data().effects ) );
-        pEffectTargetId = ( uint64_t* )( &( p->data().effectTargetId ) );
-        pFlag = ( uint16_t* )( &( p->data().unkFlag ) );
+        pHeader = reinterpret_cast< EffectHeader* >( &p->data() );
+        pEntry = reinterpret_cast< Common::EffectEntry* >( &p->data().effects );
+        pEffectTargetId = reinterpret_cast< uint64_t* >( &p->data().effectTargetId );
+        pFlag = reinterpret_cast< uint16_t* >( &p->data().unkFlag );
         effectPacket = std::move( p );
         break;
       }
       case 32:
       {
         auto p = makeZonePacket< Server::FFXIVIpcAoeEffect32 >( m_sourceChara->getId() );
-        pHeader = ( EffectHeader* )( &( p->data() ) );
-        pEntry = ( Common::EffectEntry* )( &( p->data().effects ) );
-        pEffectTargetId = ( uint64_t* )( &( p->data().effectTargetId ) );
-        pFlag = ( uint16_t* )( &( p->data().unkFlag ) );
+        pHeader = reinterpret_cast< EffectHeader* >( &p->data() );
+        pEntry = reinterpret_cast< Common::EffectEntry* >( &p->data().effects );
+        pEffectTargetId = reinterpret_cast< uint64_t* >( &p->data().effectTargetId );
+        pFlag = reinterpret_cast< uint16_t* >( &p->data().unkFlag );
         effectPacket = std::move( p );
         break;
       }
@@ -228,7 +218,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( uint32_
   else
   {
     auto resultList = m_resolvedEffects.begin()->second;
-    assert( resultList->size() > 0 );
+    assert( !resultList->empty() );
     auto firstResult = resultList->data()[ 0 ];
     Logger::debug( " - id: {}", firstResult->getTarget()->getId() );
 

src/world/Action/EffectBuilder.h

@@ -11,21 +11,20 @@ namespace Sapphire::World::Action
   public:
     EffectBuilder( Entity::CharaPtr source, uint32_t actionId, uint16_t sequence );
 
+    void heal( Entity::CharaPtr& effectTarget, Entity::CharaPtr& healingTarget, uint32_t amount,
+               Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalHeal,
+               Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None);
 
-    void healTarget( Entity::CharaPtr& target, uint32_t amount,
-                     Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalHeal );
+    void restoreMP( Entity::CharaPtr& effectTarget, Entity::CharaPtr& restoringTarget, uint32_t amount,
+                    Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None);
 
-    void selfHeal( Entity::CharaPtr& target, Entity::CharaPtr& source, uint32_t amount,
-                     Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalHeal );
-
-    void restoreMP( Entity::CharaPtr& target, Entity::CharaPtr& source, uint32_t amount );
-
-    void damageTarget( Entity::CharaPtr& target, uint32_t amount,
-                       Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalDamage );
+    void damage( Entity::CharaPtr& effectTarget, Entity::CharaPtr& damagingTarget, uint32_t amount,
+                 Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalDamage,
+                 Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None);
 
     void startCombo( Entity::CharaPtr& target, uint16_t actionId );
 
-    void comboVisualEffect( Entity::CharaPtr& target );
+    void comboSucceed( Entity::CharaPtr& target );
 
     void buildAndSendPackets();
 

src/world/Action/EffectResult.cpp

@@ -15,7 +15,7 @@ EffectResult::EffectResult( Entity::CharaPtr target, uint64_t runAfter ) :
   m_severity( Common::ActionHitSeverityType::NormalDamage ),
   m_type( Common::ActionEffectType::Nothing ),
   m_param( 0 ),
-  m_flag( 0 )
+  m_flag( Common::ActionEffectResultFlag::None )
 {
 
 }
@@ -40,27 +40,28 @@ void EffectResult::setParam( uint8_t param )
   m_param = param;
 }
 
-void EffectResult::damage( uint32_t amount, Common::ActionHitSeverityType severity )
+void EffectResult::damage( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
   m_severity = severity;
   m_value = amount;
+  m_flag = flag;
 
   m_type = Common::ActionEffectType::Damage;
 }
 
-void EffectResult::heal( uint32_t amount, Sapphire::Common::ActionHitSeverityType severity, bool isSelfHeal )
+void EffectResult::heal( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
   m_severity = severity;
   m_value = amount;
-  m_flag = isSelfHeal ? 0x80 : 0; // flag == 0x80 displays healing text at source actor
+  m_flag = flag;
 
   m_type = Common::ActionEffectType::Heal;
 }
 
-void EffectResult::restoreMP( uint32_t amount )
+void EffectResult::restoreMP( uint32_t amount, Common::ActionEffectResultFlag flag )
 {
   m_value = amount;
-  m_flag = 0x80;
+  m_flag = flag;
 
   m_type = Common::ActionEffectType::MpGain;
 }
@@ -68,14 +69,15 @@ void EffectResult::restoreMP( uint32_t amount )
 void EffectResult::startCombo( uint16_t actionId )
 {
   m_value = actionId;
-  m_flag = 0x80;
+  m_flag = Common::ActionEffectResultFlag::EffectOnSource;
 
   m_type = Common::ActionEffectType::StartActionCombo;
 }
 
-void EffectResult::comboVisualEffect()
+void EffectResult::comboSucceed()
 {
-  m_type = Common::ActionEffectType::ComboVisualEffect;
+  // no EffectOnSource flag on this
+  m_type = Common::ActionEffectType::ComboSucceed;
 }
 
 Common::EffectEntry EffectResult::buildEffectEntry() const
@@ -87,7 +89,7 @@ Common::EffectEntry EffectResult::buildEffectEntry() const
   entry.hitSeverity = m_severity;
   entry.effectType = m_type;
   entry.param = m_param;
-  entry.flags = m_flag;
+  entry.flags = static_cast< uint8_t >( m_flag );
 
   return entry;
 }

src/world/Action/EffectResult.h

@@ -15,11 +15,11 @@ namespace Sapphire::World::Action
   public:
     explicit EffectResult( Entity::CharaPtr target, uint64_t delayMs );
 
-    void damage( uint32_t amount, Common::ActionHitSeverityType severity );
-    void heal( uint32_t amount, Common::ActionHitSeverityType severity, bool isSelfHeal );
-    void restoreMP( uint32_t amount );
+    void damage( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None );
+    void heal( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None );
+    void restoreMP( uint32_t amount, Common::ActionEffectResultFlag flag = Common::ActionEffectResultFlag::None );
     void startCombo( uint16_t actionId );
-    void comboVisualEffect();
+    void comboSucceed();
 
     Entity::CharaPtr getTarget() const;
 
@@ -43,7 +43,7 @@ namespace Sapphire::World::Action
 
     uint32_t m_value;
     uint8_t m_param;
-    uint8_t m_flag;
+    Common::ActionEffectResultFlag m_flag;
   };
 }
 

src/world/Actor/Player.cpp

@@ -2140,8 +2140,7 @@ bool Sapphire::Entity::Player::hasQueuedAction() const
 
 void Sapphire::Entity::Player::setQueuedAction( Sapphire::World::Action::ActionPtr pAction )
 {
-  m_pQueuedAction = nullptr; // overwrite whatever is already there
-  m_pQueuedAction = std::move( pAction );
+  m_pQueuedAction = std::move( pAction ); // overwrite previous queued action if any
 }
 
 bool Sapphire::Entity::Player::checkAction()

src/world/Network/PacketWrappers/EffectPacket.h

@@ -26,7 +26,7 @@ namespace Sapphire::Network::Packets::Server
 
       m_data.effectDisplayType = Common::ActionEffectDisplayType::ShowActionName;
 
-      std::memset( m_data.effects, 0, sizeof(Common::EffectEntry) * 8 );
+      std::memset( m_data.effects, 0, sizeof( Common::EffectEntry ) * 8 );
     }
 
     void addEffect( const Common::EffectEntry& effect )