Clarity through better naming of fields.

src/world/Action/Action.cpp

@@ -44,12 +44,12 @@ using namespace Sapphire::World::Manager;
 Action::Action::Action() = default;
 Action::Action::~Action() = default;
 
-Action::Action::Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t sequence ) :
+Action::Action::Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t requestId ) :
-  Action( std::move( caster ), actionId, sequence, nullptr )
+  Action( std::move( caster ), actionId, requestId, nullptr )
 {
 }
 
-Action::Action::Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t sequence,
+Action::Action::Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t requestId,
                         std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData ) :
   m_pSource( std::move( caster ) ),
   m_actionData( std::move( actionData ) ),
@@ -57,7 +57,7 @@ Action::Action::Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t seq
   m_targetId( 0 ),
   m_startTime( 0 ),
   m_interruptType( Common::ActionInterruptType::None ),
-  m_sequence( sequence ),
+  m_requestId( requestId ),
   m_actionKind( Common::SkillType::Normal )
 {
 }
@@ -81,7 +81,7 @@ bool Action::Action::init()
     m_actionData = actionData;
   }
 
-  m_effectBuilder = make_EffectBuilder( m_pSource, getId(), m_sequence );
+  m_effectBuilder = make_EffectBuilder( m_pSource, getId(), m_requestId );
 
   m_castTimeMs = static_cast< uint32_t >( m_actionData->data().CastTime * 100 );
   m_recastTimeMs = static_cast< uint32_t >( m_actionData->data().RecastTime * 100 );

src/world/Action/Action.h

@@ -16,8 +16,8 @@ namespace Sapphire::World::Action
   public:
 
     Action();
-    Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t sequence );
+    Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t requestId );
-    Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t sequence, std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData );
+    Action( Entity::CharaPtr caster, uint32_t actionId, uint16_t requestId, std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData );
 
     virtual ~Action();
 
@@ -171,7 +171,7 @@ namespace Sapphire::World::Action
     uint32_t m_id{};
     uint8_t m_actionKind{};
 
-    uint16_t m_sequence{};
+    uint16_t m_requestId{};
 
     Common::ActionPrimaryCostType m_primaryCostType;
     uint16_t m_primaryCost{};

src/world/Action/EffectBuilder.cpp

@@ -25,10 +25,10 @@ using namespace Sapphire::World::Manager;
 using namespace Sapphire::Network::Packets;
 using namespace Sapphire::Network::Packets::WorldPackets::Server;
 
-EffectBuilder::EffectBuilder( Entity::CharaPtr source, uint32_t actionId, uint16_t sequence ) :
+EffectBuilder::EffectBuilder( Entity::CharaPtr source, uint32_t actionId, uint16_t requestId ) :
   m_sourceChara( std::move( source ) ),
   m_actionId( actionId ),
-  m_sequence( sequence )
+  m_requestId( requestId )
 {
 
 }
@@ -131,7 +131,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( const s
   {
     auto effectPacket = std::make_shared< EffectPacket >( m_sourceChara->getId(), m_actionId );
     effectPacket->setRotation( Common::Util::floatToUInt16Rot( m_sourceChara->getRot() ) );
-    effectPacket->setRequestId( m_sequence );
+    effectPacket->setRequestId( m_requestId );
     effectPacket->setResultId( resultId );
     effectPacket->setTargetActor( targetList[ 0 ]->getId() );
 
@@ -144,7 +144,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( const s
       for( auto i = 0; i < actorResultList.size(); ++i )
       {
         auto result = actorResultList.data()[ i ];
-        auto effect = result->buildEffectEntry();
+        auto effect = result->getCalcResultParam();
 
         // if effect result is a source/caster effect
         if( result->getTarget() == m_sourceChara )
@@ -180,7 +180,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( const s
 
     auto effectPacket = std::make_shared< EffectPacket1 >( m_sourceChara->getId(), targetList[ 0 ]->getId(), m_actionId );
     effectPacket->setRotation( Common::Util::floatToUInt16Rot( m_sourceChara->getRot() ) );
-    effectPacket->setRequestId( m_sequence );
+    effectPacket->setRequestId( m_requestId );
     effectPacket->setResultId( resultId );
 
     for( auto it = m_actorEffectsMap.begin(); it != m_actorEffectsMap.end(); )
@@ -191,7 +191,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( const s
       for( auto i = 0; i < actorResultList.size(); ++i )
       {
         auto result = actorResultList.data()[ i ];
-        auto effect = result->buildEffectEntry();
+        auto effect = result->getCalcResultParam();
 
         // if effect result is a source/caster effect
         if( result->getTarget() == m_sourceChara )
@@ -226,7 +226,7 @@ std::shared_ptr< FFXIVPacketBase > EffectBuilder::buildNextEffectPacket( const s
     effectPacket->data().MainTarget = static_cast< uint64_t >( m_sourceChara->getId() );
     effectPacket->data().DirTarget = Common::Util::floatToUInt16Rot( m_sourceChara->getRot() );
     effectPacket->data().Flag = Common::ActionEffectDisplayType::HideActionName;
-    effectPacket->data().RequestId = m_sequence;
+    effectPacket->data().RequestId = m_requestId;
     effectPacket->data().ResultId = resultId;
 
     m_actorEffectsMap.clear();

src/world/Action/EffectBuilder.h

@@ -8,7 +8,7 @@ namespace Sapphire::World::Action
   class EffectBuilder
   {
   public:
-    EffectBuilder( Entity::CharaPtr source, uint32_t actionId, uint16_t sequence );
+    EffectBuilder( Entity::CharaPtr source, uint32_t actionId, uint16_t requestId );
 
     void heal( Entity::CharaPtr& effectTarget, Entity::CharaPtr& healingTarget, uint32_t amount,
                Common::ActionHitSeverityType severity = Common::ActionHitSeverityType::NormalHeal,
@@ -36,11 +36,11 @@ namespace Sapphire::World::Action
 
     uint64_t getResultDelayMs();
 
-    std::shared_ptr< Sapphire::Network::Packets::FFXIVPacketBase > buildNextEffectPacket( const std::vector< Entity::CharaPtr >& targetList );
+    Network::Packets::FFXIVPacketBasePtr buildNextEffectPacket( const std::vector< Entity::CharaPtr >& targetList );
 
   private:
     uint32_t m_actionId;
-    uint16_t m_sequence;
+    uint16_t m_requestId;
 
     Entity::CharaPtr m_sourceChara;
     std::unordered_map< uint32_t, std::vector< EffectResultPtr > > m_actorEffectsMap;

src/world/Action/EffectResult.cpp

@@ -14,15 +14,14 @@ using namespace Sapphire::World::Action;
 
 EffectResult::EffectResult( Entity::CharaPtr target, uint64_t runAfter ) :
   m_target( std::move( target ) ),
-  m_delayMs( runAfter ),
+  m_delayMs( runAfter )
-  m_value( 0 ),
-  m_param0( 0 ),
-  m_param1( 0 ),
-  m_type( Common::ActionEffectType::CALC_RESULT_TYPE_NONE ),
-  m_param2( 0 ),
-  m_flag( Common::ActionEffectResultFlag::None )
 {
-
+  m_result.Arg0 = 0;
+  m_result.Arg1 = 0;
+  m_result.Arg2 = 0;
+  m_result.Value = 0;
+  m_result.Flag = static_cast< uint8_t >( Common::ActionEffectResultFlag::None );
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_NONE;
 }
 
 Entity::CharaPtr EffectResult::getTarget() const
@@ -30,11 +29,6 @@ Entity::CharaPtr EffectResult::getTarget() const
   return m_target;
 }
 
-uint32_t EffectResult::getValue() const
-{
-  return m_value;
-}
-
 uint64_t EffectResult::getDelay()
 {
   return m_delayMs;
@@ -42,101 +36,85 @@ uint64_t EffectResult::getDelay()
 
 void EffectResult::damage( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
-  m_param0 = static_cast< uint8_t >( severity );
+  m_result.Arg0 = static_cast< uint8_t >( severity );
-  m_value = amount;
+  m_result.Value = static_cast< int16_t >( amount );
-  m_flag = flag;
+  m_result.Flag = static_cast< uint8_t >( flag );
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_DAMAGE_HP;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_DAMAGE_HP;
 }
 
 void EffectResult::heal( uint32_t amount, Common::ActionHitSeverityType severity, Common::ActionEffectResultFlag flag )
 {
-  m_param1 = static_cast< uint8_t >( severity );
+  m_result.Arg1 = static_cast< uint8_t >( severity );
-  m_value = amount;
+  m_result.Value = static_cast< int16_t >( amount );
-  m_flag = flag;
+  m_result.Flag = static_cast< uint8_t >( flag );
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_HP;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_HP;
 }
 
 void EffectResult::restoreMP( uint32_t amount, Common::ActionEffectResultFlag flag )
 {
-  m_value = amount;
+  m_result.Value = static_cast< int16_t >( amount );
-  m_flag = flag;
+  m_result.Flag = static_cast< uint8_t >( flag );
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_MP;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_MP;
 }
 
 void EffectResult::startCombo( uint16_t actionId )
 {
-  m_value = actionId;
+  m_result.Value = static_cast< int16_t >( actionId );
-  m_flag = Common::ActionEffectResultFlag::EffectOnSource;
+  m_result.Flag = static_cast< uint8_t >( Common::ActionEffectResultFlag::EffectOnSource );
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_COMBO;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_COMBO;
 }
 
 void EffectResult::comboSucceed()
 {
   // no EffectOnSource flag on this
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_COMBO_HIT;
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_COMBO_HIT;
 }
 
 void EffectResult::applyStatusEffect( uint16_t statusId, uint8_t param )
 {
-  m_value = statusId;
+  m_result.Value = static_cast< int16_t >( statusId );
-  m_param2 = param;
+  m_result.Arg2 = param;
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_SET_STATUS;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_SET_STATUS;
 }
 
 void EffectResult::mount( uint16_t mountId )
 {
-  m_value = mountId;
+  m_result.Value = static_cast< int16_t >( mountId );
-  m_param0 = 1;
+  m_result.Arg0 = 1;
-
+  m_result.Type = Common::ActionEffectType::CALC_RESULT_TYPE_MOUNT;
-  m_type = Common::ActionEffectType::CALC_RESULT_TYPE_MOUNT;
 }
 
-Common::CalcResultParam EffectResult::buildEffectEntry() const
+const Common::CalcResultParam& EffectResult::getCalcResultParam() const
 {
-  Common::CalcResultParam entry{};
+  return m_result;
-
-  // todo: that retarded shit so > u16 max numbers work
-  entry.Value = getValue();
-  entry.Arg0 = m_param0;
-  entry.Arg1 = m_param1;
-  entry.Type = m_type;
-  entry.Arg2 = m_param2;
-  entry.Flag = static_cast< uint8_t >( m_flag );
-
-  return entry;
 }
 
 void EffectResult::execute()
 {
-  switch( m_type )
+  switch( m_result.Type )
   {
     case Common::ActionEffectType::CALC_RESULT_TYPE_DAMAGE_HP:
     {
-      m_target->takeDamage( m_value );
+      m_target->takeDamage( m_result.Value );
       break;
     }
 
     case Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_HP:
     {
-      m_target->heal( m_value );
+      m_target->heal( m_result.Value );
       break;
     }
 
     case Common::ActionEffectType::CALC_RESULT_TYPE_RECOVER_MP:
     {
-      m_target->restoreMP( m_value );
+      m_target->restoreMP( m_result.Value );
       break;
     }
 
     case Common::ActionEffectType::CALC_RESULT_TYPE_MOUNT:
     {
       auto pPlayer = m_target->getAsPlayer();
-      Common::Service< World::Manager::PlayerMgr >::ref().onMountUpdate( *pPlayer, m_value );
+      Common::Service< World::Manager::PlayerMgr >::ref().onMountUpdate( *pPlayer, m_result.Value );
       break;
     }
 

src/world/Action/EffectResult.h

@@ -24,11 +24,9 @@ namespace Sapphire::World::Action
 
     Entity::CharaPtr getTarget() const;
 
-    uint32_t getValue() const;
-
     uint64_t getDelay();
 
-    Common::CalcResultParam buildEffectEntry() const;
+    const Common::CalcResultParam& getCalcResultParam() const;
 
     void execute();
 
@@ -37,13 +35,7 @@ namespace Sapphire::World::Action
 
     Entity::CharaPtr m_target;
 
-    Common::ActionEffectType m_type;
+    Common::CalcResultParam m_result;
 
-    uint8_t m_param0;
-    uint8_t m_param1;
-    uint8_t m_param2;
-
-    uint32_t m_value;
-    Common::ActionEffectResultFlag m_flag;
   };
 }

src/world/Action/ItemManipulationAction.cpp

@@ -16,14 +16,14 @@ using namespace Sapphire;
 using namespace Sapphire::World::Action;
 using namespace Sapphire::Network::Packets::WorldPackets::Server;
 
-ItemManipulationAction::ItemManipulationAction( Entity::CharaPtr source, uint32_t actionId, uint16_t sequence,
+ItemManipulationAction::ItemManipulationAction( Entity::CharaPtr source, uint32_t actionId, uint16_t requestId,
-                            std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, uint32_t delayTime ) :
+                                                std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, uint32_t delayTime ) :
   m_delayTimeMs( delayTime )
 {
   m_id = actionId;
   m_pSource = std::move( source );
   m_actionData = std::move( actionData );
-  m_sequence = sequence;
+  m_requestId = requestId;
 }
 
 void ItemManipulationAction::start()

src/world/Action/ItemManipulationAction.h

@@ -8,7 +8,7 @@ namespace Sapphire::World::Action
   class ItemManipulationAction : public Action
   {
   public:
-    ItemManipulationAction( Entity::CharaPtr source, uint32_t actionId, uint16_t sequence,
+    ItemManipulationAction( Entity::CharaPtr source, uint32_t actionId, uint16_t requestId,
                   std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, uint32_t delayTime );
     virtual ~ItemManipulationAction() = default;
 

src/world/Manager/ActionMgr.cpp

@@ -60,9 +60,9 @@ void ActionMgr::handleItemManipulationAction( Entity::Player& player, uint32_t a
 }
 
 void ActionMgr::handleTargetedPlayerAction( Entity::Player& player, uint32_t actionId,
-                                            Excel::ExcelStructPtr< Excel::Action > actionData, uint64_t targetId, uint16_t sequence )
+                                            Excel::ExcelStructPtr< Excel::Action > actionData, uint64_t targetId, uint16_t requestId )
 {
-  auto action = Action::make_Action( player.getAsPlayer(), actionId, sequence, actionData );
+  auto action = Action::make_Action( player.getAsPlayer(), actionId, requestId, actionData );
 
   action->setTargetId( targetId );
 

src/world/Manager/ActionMgr.h

@@ -25,7 +25,7 @@ namespace Sapphire::World::Manager
     void handleItemManipulationAction( Entity::Player& player, uint32_t actionId, Excel::ExcelStructPtr< Excel::Action > actionData, uint16_t sequence );
 
     void handleTargetedPlayerAction( Entity::Player& player, uint32_t actionId,
-                                     std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, uint64_t targetId, uint16_t sequence );
+                                     std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, uint64_t targetId, uint16_t requestId );
     void handlePlacedPlayerAction( Entity::Player& player, uint32_t actionId,
                                    std::shared_ptr< Excel::ExcelStruct< Excel::Action > > actionData, Common::FFXIVARR_POSITION3 pos, uint16_t sequence );
 

src/world/Network/GameConnection.cpp

@@ -289,7 +289,7 @@ void GameConnection::handleChatPacket( Packets::FFXIVARR_PACKET_RAW& pPacket )
   if( it != m_chatHandlerMap.end() )
   {
     auto itStr = m_chatHandlerStrMap.find( opcode );
-    std::string name = itStr != m_chatHandlerStrMap.end() ? itStr->second : "unknown";
+    std::string name = itStr != m_chatHandlerStrMap.end() ? itStr->second : "Unknown";
     // dont display packet notification if it is a ping or pos update, don't want the spam
 
     Logger::debug( "[{0}] Handling Chat IPC : {1} ( {2:04X} )", m_pSession->getId(), name, opcode );

src/world/Network/Handlers/ActionHandler.cpp

@@ -23,12 +23,12 @@ void Sapphire::Network::GameConnection::actionRequest( const Packets::FFXIVARR_P
 
   const auto type = packet.data().ActionKind;
   const auto actionId = packet.data().ActionKey;
-  const auto sequence = packet.data().RequestId;
+  const auto requestId = packet.data().RequestId;
   const auto targetId = packet.data().Target;
   const auto itemSourceSlot = packet.data().Arg & 0x0000FFFF;
   const auto itemSourceContainer = packet.data().Arg & 0xFFFF0000;
 
-  PlayerMgr::sendDebug( player, "Skill type: {0}, sequence: {1}, actionId: {2}, targetId: {3}", type, sequence, actionId, targetId );
+  PlayerMgr::sendDebug( player, "Skill type: {0}, requestId: {1}, actionId: {2}, targetId: {3}", type, requestId, actionId, targetId );
 
   auto& exdData = Common::Service< Data::ExdData >::ref();
   auto& actionMgr = Common::Service< World::Manager::ActionMgr >::ref();
@@ -50,9 +50,9 @@ void Sapphire::Network::GameConnection::actionRequest( const Packets::FFXIVARR_P
       auto category = static_cast< Common::ActionCategory >( action->data().Category );
 
       if( category  == Common::ActionCategory::ItemManipulation )
-        actionMgr.handleItemManipulationAction( player, actionId, action, sequence );
+        actionMgr.handleItemManipulationAction( player, actionId, action, requestId );
       else
-        actionMgr.handleTargetedPlayerAction( player, actionId, action, targetId, sequence );
+        actionMgr.handleTargetedPlayerAction( player, actionId, action, targetId, requestId );
       break;
     }
 
@@ -78,7 +78,7 @@ void Sapphire::Network::GameConnection::actionRequest( const Packets::FFXIVARR_P
     {
       auto action = exdData.getRow< Excel::EventItem >( actionId );
       assert( action );
-      actionMgr.handleEventItemAction( player, actionId, action, sequence, targetId );
+      actionMgr.handleEventItemAction( player, actionId, action, requestId, targetId );
       break;
     }
 
@@ -86,7 +86,7 @@ void Sapphire::Network::GameConnection::actionRequest( const Packets::FFXIVARR_P
     {
       auto action = exdData.getRow< Excel::Action >( 4 );
       assert( action );
-      actionMgr.handleMountAction( player, static_cast< uint16_t >( actionId ), action, targetId, sequence );
+      actionMgr.handleMountAction( player, static_cast< uint16_t >( actionId ), action, targetId, requestId );
       break;
     }
   }

src/world/Network/PacketWrappers/EffectPacket1.h

@@ -75,7 +75,7 @@ namespace Sapphire::Network::Packets::WorldPackets::Server
 
     void setResultId( uint32_t resultId )
     {
-      m_data.ResultId = static_cast< uint32_t >( resultId );
+      m_data.ResultId = resultId;
     }
 
   private:

src/world/WorldServer.cpp

@@ -70,10 +70,6 @@ WorldServer::WorldServer( const std::string& configName ) :
 {
 }
 
-WorldServer::~WorldServer()
-{
-}
-
 size_t WorldServer::getSessionCount() const
 {
   return m_sessionMapById.size();
@@ -389,12 +385,13 @@ void WorldServer::updateSessions( uint32_t currTime )
     if( !session || !session->getPlayer() )
       continue;
 
+    auto& player = *session->getPlayer();
+
     // if the player is in a zone, let the zone handler take care of his updates, else do it here.
-    if( !session->getPlayer()->isConnected() )
+    if( !player.isConnected() )
       session->update();
 
     auto diff = difftime( currTime, session->getLastDataTime() );
-    auto& player = *session->getPlayer();
 
     // remove session of players marked for removal ( logoff / kick )
     if( ( player.isMarkedForRemoval() && diff > 5 ) || diff > 20 )

src/world/WorldServer.h

@@ -16,7 +16,7 @@ namespace Sapphire::World
   public:
     WorldServer( const std::string& configName );
 
-    ~WorldServer();
+    ~WorldServer() = default;
 
     using WorldConfigPtr = std::shared_ptr< Sapphire::Common::Config::WorldConfig >;