Some more packet cleanup.

src/world/Manager/HousingMgr.cpp

@@ -8,6 +8,7 @@
 #include <Network/PacketWrappers/ActorControlSelfPacket.h>
 #include <Network/CommonActorControl.h>
 #include <Network/GameConnection.h>
+#include <Network/Util/PacketUtil.h>
 
 #include <unordered_map>
 #include <cstring>
@@ -434,8 +435,7 @@ bool HousingMgr::relinquishLand( Entity::Player& player, HousingZone& zone, uint
   // TODO: actually use permissions here for FC houses
   if( !hasPermission( player, *pLand, 0 ) )
   {
-    auto msgPkt = makeActorControlSelf( player.getId(), ActorControl::LogMsg, 3304, 0 );
+    Network::Util::Packet::sendActorControlSelf( player, ActorControl::LogMsg, 3304 );
-    server.queueForPlayer( player.getCharacterId(), msgPkt );
     return false;
   }
 
@@ -443,8 +443,7 @@ bool HousingMgr::relinquishLand( Entity::Player& player, HousingZone& zone, uint
   // TODO: additionally check for yard items
   if( pLand->getHouse() )
   {
-    auto msgPkt = makeActorControlSelf( player.getId(), ActorControl::LogMsg, 3315, 0 );
+    Network::Util::Packet::sendActorControlSelf( player, ActorControl::LogMsg, 3315 );
-    server.queueForPlayer( player.getCharacterId(), msgPkt );
     return false;
   }
 
@@ -460,11 +459,8 @@ bool HousingMgr::relinquishLand( Entity::Player& player, HousingZone& zone, uint
 
   sendLandFlagsSlot( player, Common::LandFlagsSlot::Private );
 
-  auto screenMsgPkt2 = makeActorControlSelf( player.getId(), ActorControl::LogMsg, 3351, 0x1AA,
+  Network::Util::Packet::sendActorControlSelf( player, ActorControl::LogMsg, 3351, 0x1AA, pLand->getLandIdent().wardNum + 1, plot + 1 );
-                                             pLand->getLandIdent().wardNum + 1, plot + 1 );
-  server.queueForPlayer( player.getCharacterId(), screenMsgPkt2 );
   zone.sendLandUpdate( plot );
-
   return true;
 }
 
@@ -708,9 +704,7 @@ void HousingMgr::buildPresetEstate( Entity::Player& player, HousingZone& zone, u
   pLand->setLandType( Common::LandType::Private );
   zone.sendLandUpdate( plotNum );
 
-  auto pSuccessBuildingPacket = makeActorControl( player.getId(), ActorControl::BuildPresetResponse, plotNum );
+  Network::Util::Packet::sendActorControl( player, ActorControl::BuildPresetResponse, plotNum );
-
-  server.queueForPlayer( player.getCharacterId(), pSuccessBuildingPacket );
 
   pLand->updateLandDb();
 
@@ -1065,7 +1059,7 @@ void HousingMgr::reqPlaceHousingItem( Entity::Player& player, uint16_t landId, u
     status = placeInteriorItem( player, item );
 
   if( status )
-    server.queueForPlayer( player.getCharacterId(), makeActorControlSelf( player.getId(), 0x3f3 ) );
+    Network::Util::Packet::sendActorControlSelf( player, 0x3f3 );
   else
     PlayerMgr::sendUrgent( player, "An internal error occurred when placing the item." );
 }
@@ -1343,7 +1337,7 @@ bool HousingMgr::moveInternalItem( Entity::Player& player, Common::LandIdent ide
   // send confirmation to player
   uint32_t param1 = static_cast< uint32_t >( ( ident.landId << 16 ) | containerId );
 
-  server.queueForPlayer( player.getCharacterId(), makeActorControlSelf( player.getId(), ActorControl::HousingItemMoveConfirm, param1, slotIdx ) );
+  Network::Util::Packet::sendActorControlSelf( player, ActorControl::HousingItemMoveConfirm, param1, slotIdx );
 
   return true;
 }
@@ -1377,7 +1371,7 @@ bool HousingMgr::moveExternalItem( Entity::Player& player, Common::LandIdent ide
   terri.updateYardObjectPos( player, slot, static_cast< uint16_t >( ident.landId ), *item );
 
   uint32_t param1 = static_cast< uint32_t >( ( ident.landId << 16 ) | Common::InventoryType::HousingExteriorPlacedItems );
-  server.queueForPlayer( player.getCharacterId(), makeActorControlSelf( player.getId(), ActorControl::HousingItemMoveConfirm, param1, slot ) );
+  Network::Util::Packet::sendActorControlSelf( player, ActorControl::HousingItemMoveConfirm, param1, slot ) );
 
   return true;
 }
@@ -1614,9 +1608,7 @@ void HousingMgr::reqEstateExteriorRemodel( Entity::Player& player, uint16_t plot
 
   invMgr.sendInventoryContainer( player, it->second );
 
-  auto pkt = makeActorControlSelf( player.getId(), Network::ActorControl::ShowEstateExternalAppearanceUI, plot );
+  Network::Util::Packet::sendActorControlSelf( player, ActorControl::ShowEstateExternalAppearanceUI, plot );
-  server.queueForPlayer( player.getCharacterId(), pkt );
-
 }
 
 void HousingMgr::reqEstateInteriorRemodel( Entity::Player& player )
@@ -1653,8 +1645,7 @@ void HousingMgr::reqEstateInteriorRemodel( Entity::Player& player )
 
   invMgr.sendInventoryContainer( player, it->second );
 
-  auto pkt = makeActorControlSelf( player.getId(), Network::ActorControl::ShowEstateInternalAppearanceUI );
+  Network::Util::Packet::sendActorControlSelf( player, ActorControl::ShowEstateInternalAppearanceUI );
-  server.queueForPlayer( player.getCharacterId(), pkt );
 }
 
 bool HousingMgr::hasPermission( Entity::Player& player, Sapphire::Land& land, uint32_t permission )
@@ -1697,8 +1688,7 @@ void HousingMgr::removeHouse( Entity::Player& player, uint16_t plot )
   // TODO: actually use permissions here for FC houses
   if( !hasPermission( player, *pLand, 0 ) )
   {
-    auto msgPkt = makeActorControlSelf( player.getId(), ActorControl::LogMsg, 3305, 0 );
+    Network::Util::Packet::sendActorControlSelf( player, ActorControl::LogMsg, 3305 );
-    server.queueForPlayer( player.getCharacterId(), msgPkt );
     return;
   }
 

src/world/Manager/PartyMgr.cpp

@@ -200,7 +200,7 @@ void PartyMgr::onMemberDisconnect( Entity::Player& disconnectingPlayer )
   {
     // TODO: 2nd argument here makes it automatically send passing leadership message
     server.queueForPlayer( member->getCharacterId(), { makePcPartyUpdate( disconnectingPlayer, UpdateStatus::OFFLINE_MEMBER, party->PartyCount ),
-                                                          makeZonePacket< FFXIVIpcUpdateParty >( member->getId() ) } );
+                                                       makeZonePacket< FFXIVIpcUpdateParty >( member->getId() ) } );
   }
 
   sendPartyUpdate( *party );

src/world/Manager/PlayerMgr.cpp

@@ -307,35 +307,38 @@ void PlayerMgr::onUpdate( Entity::Player& player, uint64_t tickCount )
   if( !player.isAlive() )
     return;
 
-  auto mainWeap = player.getItemAt( Common::GearSet0, Common::GearSetSlot::MainHand );
+  checkAutoAttack( player, tickCount );
-  if( mainWeap && !player.checkAction() && ( player.getTargetId() && player.getStance() == Common::Stance::Active && player.isAutoattackOn() ) )
+}
+
+void PlayerMgr::checkAutoAttack( Entity::Player& player, uint64_t tickCount ) const
+{
+  auto mainWeap = player.getItemAt( Common::GearSet0, Common::MainHand );
+  if( !mainWeap || !player.isAutoattackOn() || player.checkAction() || !player.getTargetId() || player.getStance() != Common::Active )
+    return;
+
+  for( const auto& actor : player.getInRangeActors() )
   {
-    // @TODO i dislike this, iterating over all in range actors when you already know the id of the actor you need...
+    if( actor->getId() != player.getTargetId() || !actor->getAsChara()->isAlive() )
-    for( const auto& actor : player.getInRangeActors() )
+      continue;
+    auto chara = actor->getAsChara();
+
+    // default autoattack range
+    float range = 3.f + chara->getRadius() + player.getRadius() * 0.5f;
+
+    // default autoattack range for ranged classes
+    auto classJob = player.getClass();
+
+    if( classJob == Common::ClassJob::Machinist || classJob == Common::ClassJob::Bard || classJob == Common::ClassJob::Archer )
+      range = 25.f + chara->getRadius() + player.getRadius() * 0.5f;
+
+    if( ( Common::Util::distance( player.getPos(), actor->getPos() ) <= range ) &&
+        ( ( tickCount - player.getLastAttack() ) > mainWeap->getDelay() ) )
     {
-      if( actor->getId() != player.getTargetId() || !actor->getAsChara()->isAlive() )
+      player.setLastAttack( tickCount );
-        continue;
+      player.autoAttack( actor->getAsChara() );
-      auto chara = actor->getAsChara();
-
-      // default autoattack range
-      float range = 3.f + chara->getRadius() + player.getRadius() * 0.5f;
-
-      // default autoattack range for ranged classes
-      auto classJob = player.getClass();
-
-      if( classJob == Common::ClassJob::Machinist || classJob == Common::ClassJob::Bard || classJob == Common::ClassJob::Archer )
-        range = 25.f + chara->getRadius() + player.getRadius() * 0.5f;
-
-      if( Common::Util::distance( player.getPos(), actor->getPos() ) <= range )
-      {
-        if( ( tickCount - player.getLastAttack() ) > mainWeap->getDelay() )
-        {
-          player.setLastAttack( tickCount );
-          player.autoAttack( actor->getAsChara() );
-        }
-      }
     }
   }
+
 }
 
 

src/world/Manager/PlayerMgr.h

@@ -65,6 +65,8 @@ namespace Sapphire::World::Manager
     std::map< uint32_t, Entity::PlayerPtr > m_playerMapById;
     std::map< uint64_t, Entity::PlayerPtr > m_playerMapByCharacterId;
     std::map< std::string, Entity::PlayerPtr > m_playerMapByName;
+
+    void checkAutoAttack( Entity::Player& player, uint64_t tickCount ) const;
   };
 
 

src/world/Manager/WarpMgr.cpp

@@ -14,6 +14,7 @@
 #include <Network/CommonActorControl.h>
 #include <Network/PacketWrappers/ActorControlSelfPacket.h>
 #include <Network/PacketWrappers/ActorControlPacket.h>
+#include <Network/Util/PacketUtil.h>
 
 #include <Manager/PlayerMgr.h>
 #include <Manager/MapMgr.h>
@@ -90,10 +91,8 @@ void WarpMgr::requestWarp( Entity::Player& player, Common::WarpType warpType, Co
 {
   m_entityIdToWarpInfoMap[ player.getId() ] = { 0, warpType, targetPos, targetRot };
 
-  server().queueForPlayers( player.getInRangePlayerIds( true ),
+  Network::Util::Packet::sendActorControlSelf( player.getInRangePlayerIds( true ), player, WarpStart, warpType, warpType, 0, player.getTerritoryTypeId(), 1 );
-                            makeActorControlSelf( player.getId(), WarpStart, warpType, warpType, 0, player.getTerritoryTypeId(), 1 ) );
+  Network::Util::Packet::sendActorControl( player.getInRangePlayerIds(), player, ActorDespawnEffect, warpType, player.getTerritoryTypeId() );
-  server().queueForPlayers( player.getInRangePlayerIds(),
-                            makeActorControl( player.getId(), ActorDespawnEffect, warpType, player.getTerritoryTypeId() ) );
 
   auto& taskMgr = Common::Service< TaskMgr >::ref();
   taskMgr.queueTask( makeWarpTask( player, warpType, targetPos, targetRot, 1000 ) );
@@ -127,14 +126,11 @@ void WarpMgr::finishWarp( Entity::Player& player )
 
   auto warpFinishAnim = warpType - 1;
 
-  auto zoneInPacket = makeActorControlSelf( player.getId(), Appear, warpFinishAnim, raiseAnim, 0, 0 );
-  auto setStatusPacket = makeActorControl( player.getId(), SetStatus, static_cast< uint8_t >( Common::ActorStatus::Idle ) );
-
   if( !player.getGmInvis() )
-    server().queueForPlayers( player.getInRangePlayerIds(), zoneInPacket );
+    Network::Util::Packet::sendActorControlSelf( player.getInRangePlayerIds(), player, Appear, warpFinishAnim, raiseAnim );
 
-  server().queueForPlayer( player.getCharacterId(), zoneInPacket );
+  Network::Util::Packet::sendActorControlSelf( player, Appear, warpFinishAnim, raiseAnim );
-  server().queueForPlayers( player.getInRangePlayerIds( true ), setStatusPacket );
+  Network::Util::Packet::sendActorControl( player.getInRangePlayerIds( true ), player, SetStatus, static_cast< uint8_t >( Common::ActorStatus::Idle ) );
 
   player.removeCondition( PlayerCondition::BetweenAreas );
 

src/world/Network/Handlers/PacketHandlers.cpp

@@ -103,17 +103,15 @@ void Sapphire::Network::GameConnection::getSearchCommentHandler( const Packets::
 
   Logger::debug( "getSearchCommentHandler: {0}", targetId );
 
-  if( pPlayer )
+  if( !pPlayer || pPlayer->isActingAsGm() || pPlayer->getTerritoryTypeId() != player.getTerritoryTypeId() )
-  {
+    return;
-    if( pPlayer->isActingAsGm() || pPlayer->getTerritoryTypeId() != player.getTerritoryTypeId() )
+
-      return;
+  // retail sends the requester's id as both (isForSelf)
+  auto searchInfoPacket = makeZonePacket< FFXIVIpcGetSearchCommentResult >( player.getId() );
+  searchInfoPacket->data().TargetEntityID = targetId;
+  strcpy( searchInfoPacket->data().SearchComment, pPlayer->getSearchMessage() );
+  server().queueForPlayer( player.getCharacterId(), searchInfoPacket );
 
-    // retail sends the requester's id as both (isForSelf)
-    auto searchInfoPacket = makeZonePacket< FFXIVIpcGetSearchCommentResult >( player.getId() );
-    searchInfoPacket->data().TargetEntityID = targetId;
-    strcpy( searchInfoPacket->data().SearchComment, pPlayer->getSearchMessage() );
-    server().queueForPlayer( player.getCharacterId(), searchInfoPacket );
-  }
 }
 
 void Sapphire::Network::GameConnection::reqExamineFcInfo( const Packets::FFXIVARR_PACKET_RAW& inPacket, Entity::Player& player )
@@ -313,14 +311,14 @@ void Sapphire::Network::GameConnection::newDiscoveryHandler( const Packets::FFXI
 
   PlayerMgr::sendDebug( player, "Discovery ref pos id#{0}", layoutId );
 
-  if( pRefInfo )
+  if( !pRefInfo )
-  {
+    return;
-    auto discoveryPacket = makeZonePacket< FFXIVIpcDiscoveryReply >( player.getId() );
+
-    discoveryPacket->data().mapId = tInfo->data().Map;
+  auto discoveryPacket = makeZonePacket< FFXIVIpcDiscoveryReply >( player.getId() );
-    discoveryPacket->data().mapPartId = pRefInfo->data.discoveryIndex;
+  discoveryPacket->data().mapId = tInfo->data().Map;
-    server().queueForPlayer( player.getCharacterId(), discoveryPacket );
+  discoveryPacket->data().mapPartId = pRefInfo->data.discoveryIndex;
-    player.discover( tInfo->data().Map, pRefInfo->data.discoveryIndex );
+  server().queueForPlayer( player.getCharacterId(), discoveryPacket );
-  }
+  player.discover( tInfo->data().Map, pRefInfo->data.discoveryIndex );
 
 }
 
@@ -363,9 +361,7 @@ void Sapphire::Network::GameConnection::setLanguageHandler( const Packets::FFXIV
 
   // if this is a login event
   if( player.isLogin() )
-  {
     playerMgr().sendLoginMessage( player );
-  }
 
   // spawn the player for himself
   player.spawn( player.getAsPlayer() );
@@ -619,9 +615,7 @@ void Sapphire::Network::GameConnection::gearSetEquip( const Packets::FFXIVARR_PA
   player.sendGearInventory();
 
   if( packet.data().contextId < 0xFE )
-  {
+    Network::Util::Packet::sendActorControlSelf( player, GearSetEquipMsg, packet.data().contextId );
-    server().queueForPlayer( player.getCharacterId(), makeActorControlSelf( player.getId(), Network::ActorControl::GearSetEquipMsg, packet.data().contextId ) );
-  }
 
   auto invTransFinPacket = makeZonePacket< FFXIVIpcItemOperationBatch >( player.getId() );
   invTransFinPacket->data().contextId = contextId;