transfer result handling to blmgr; more error checking; const correct;

src/world/Manager/BlacklistMgr.cpp

@@ -18,19 +18,33 @@ using namespace Sapphire::Common;
 using namespace Sapphire::Network::Packets;
 using namespace Sapphire::Network::Packets::WorldPackets;
 
-bool Sapphire::World::Manager::BlacklistMgr::onAddCharacter( Entity::Player& source, Entity::Player& target )
+bool Sapphire::World::Manager::BlacklistMgr::onAddCharacter( Entity::Player& source, const std::string& targetName )
 {
   // add target to blacklist
 
+  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
+
+  auto pTarget = server.getPlayer( targetName );
+  if( !pTarget )
+  {
+    // target doesn't exist in server player table
+    sendAddResultPacket( source, pTarget, 0x7 );
+    return false;
+  }
+
+  auto& target = *pTarget;
+
   if( source.getCharacterId() == target.getCharacterId() )
   {
     // can't add self to blacklist
+    sendAddResultPacket( source, pTarget, 0x7 );
     return false;
   }
   
   if( isBlacklisted( source, target ) )
   {
     // target already added to blacklist
+    sendAddResultPacket( source, pTarget, 0x7 );
     return false;
   }
 
@@ -49,6 +63,8 @@ bool Sapphire::World::Manager::BlacklistMgr::onAddCharacter( Entity::Player& sou
 
   source.updateDbBlacklist();
 
+  sendAddResultPacket( source, pTarget, 0 );
+
   // check if player is friends with target
   auto& flMgr = Common::Service< Sapphire::World::Manager::FriendListMgr >::ref();
   if( flMgr.isFriend( source, target ) )
@@ -57,15 +73,30 @@ bool Sapphire::World::Manager::BlacklistMgr::onAddCharacter( Entity::Player& sou
   return true;
 }
 
-bool Sapphire::World::Manager::BlacklistMgr::onRemoveCharacter( Entity::Player& source, Entity::Player& target )
+bool Sapphire::World::Manager::BlacklistMgr::onRemoveCharacter( Entity::Player& source, const std::string& targetName )
 {
   // remove target from blacklist
 
+  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
+
+  uint32_t result = 0;
+
+  auto pTarget = server.getPlayer( targetName );
+  if( !pTarget )
+  {
+    // target doesn't exist in server player table
+    sendRemoveResultPacket( source, pTarget, 0x7 );
+    return false;
+  }
+
+  auto& target = *pTarget;
+
   auto sourceIdx = getEntryIndex( source, target.getCharacterId() );
   
   if( !isBlacklisted( source, target ) )
   {
     // target not in blacklist
+    sendRemoveResultPacket( source, pTarget, 0x7 );
     return false;
   }
 
@@ -74,13 +105,15 @@ bool Sapphire::World::Manager::BlacklistMgr::onRemoveCharacter( Entity::Player&
   sourceBL[sourceIdx] = 0;
   source.updateDbBlacklist();
 
+  sendRemoveResultPacket( source, pTarget, 0 );
+
   return true;
 }
 
 bool Sapphire::World::Manager::BlacklistMgr::onGetBlacklistPage( Entity::Player& source, uint8_t key, uint8_t nextIdx )
 {
   // this function will handle client side indexing and paginate blacklist entries
-  // it'll also be called multiple times sequentially until there are no more valid entries left
+  // it'll also be called multiple times sequentially until there are no more entries left (id == 0)
 
   auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
 
@@ -133,12 +166,12 @@ bool Sapphire::World::Manager::BlacklistMgr::onGetBlacklistPage( Entity::Player&
   return true;
 }
 
-bool Sapphire::World::Manager::BlacklistMgr::isBlacklisted( Entity::Player& source, Entity::Player& target )
+bool Sapphire::World::Manager::BlacklistMgr::isBlacklisted( Entity::Player& source, Entity::Player& target ) const
 {
   return getEntryIndex( source, target.getCharacterId() ) != -1;
 }
 
-ptrdiff_t Sapphire::World::Manager::BlacklistMgr::getEntryIndex( Entity::Player& source, uint64_t characterId )
+ptrdiff_t Sapphire::World::Manager::BlacklistMgr::getEntryIndex( Entity::Player& source, uint64_t characterId ) const
 {
   auto& sourceBL = source.getBlacklistID();
   auto sourceBlIt = std::find( std::begin( sourceBL ), std::end( sourceBL ), characterId );
@@ -148,4 +181,46 @@ ptrdiff_t Sapphire::World::Manager::BlacklistMgr::getEntryIndex( Entity::Player&
     return -1;
 
   return sourceBlIt - std::begin( sourceBL );
-}
+}
+
+void Sapphire::World::Manager::BlacklistMgr::sendAddResultPacket( Entity::Player& source, Entity::PlayerPtr pTarget, uint32_t result )
+{
+  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
+
+  auto resultPacket = makeZonePacket< Server::FFXIVIpcBlacklistAddResult >( source.getId() );
+
+  if( pTarget )
+  {
+    Server::BlacklistCharacter blChar;
+    blChar.CharacterID = pTarget->getCharacterId();
+    strcpy( blChar.CharacterName, pTarget->getName().c_str() );
+
+    resultPacket->data().AddedCharacter = blChar;
+    resultPacket->data().Identity = pTarget->getGender();
+  }
+  
+  resultPacket->data().Result = result;
+
+  server.queueForPlayer( source.getCharacterId(), resultPacket );
+}
+
+void Sapphire::World::Manager::BlacklistMgr::sendRemoveResultPacket( Entity::Player& source, Entity::PlayerPtr pTarget, uint32_t result )
+{
+  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
+
+  auto resultPacket = makeZonePacket< Server::FFXIVIpcBlacklistRemoveResult >( source.getId() );
+
+  if( pTarget )
+  {
+    Server::BlacklistCharacter blChar;
+    blChar.CharacterID = pTarget->getCharacterId();
+    strcpy( blChar.CharacterName, pTarget->getName().c_str() );
+
+    resultPacket->data().RemovedCharacter = blChar;
+    resultPacket->data().Identity = pTarget->getGender();
+  }
+  
+  resultPacket->data().Result = result;
+
+  server.queueForPlayer( source.getCharacterId(), resultPacket );
+}

src/world/Manager/BlacklistMgr.h

@@ -11,14 +11,17 @@ namespace Sapphire::World::Manager
   public:
     BlacklistMgr() = default;
 
-    bool onAddCharacter( Entity::Player& source, Entity::Player& target );
+    bool onAddCharacter( Entity::Player& source, const std::string& targetName );
-    bool onRemoveCharacter( Entity::Player& source, Entity::Player& target );
+    bool onRemoveCharacter( Entity::Player& source, const std::string& targetName );
 
     bool onGetBlacklistPage( Entity::Player& source, uint8_t key, uint8_t nextIdx );
 
-    bool isBlacklisted( Entity::Player& source, Entity::Player& target );
+    bool isBlacklisted( Entity::Player& source, Entity::Player& target ) const;
 
    private:
-    ptrdiff_t getEntryIndex( Entity::Player& source, uint64_t characterId );
+    ptrdiff_t getEntryIndex( Entity::Player& source, uint64_t characterId ) const;
+
+    void sendAddResultPacket( Entity::Player& source, Entity::PlayerPtr pTarget, uint32_t result );
+    void sendRemoveResultPacket( Entity::Player& source, Entity::PlayerPtr pTarget, uint32_t result );
   };
 }

src/world/Manager/FriendListMgr.cpp

@@ -41,12 +41,13 @@ bool Sapphire::World::Manager::FriendListMgr::onInviteCreate( Entity::Player& so
   Common::HierarchyData hierarchy;
   hierarchy.data.dateAdded = Common::Util::getTimeSeconds();
   hierarchy.data.group = 0;
-  hierarchy.data.status = Common::HierarchyStatus::SentRequest; // set type for invite sender
   hierarchy.data.type = Common::HierarchyType::FRIENDLIST;
 
+  // set hierarchy status for invite sender
+  hierarchy.data.status = Common::HierarchyStatus::SentRequest;
   sourceFLData[ sourceIdx ] = hierarchy;
 
-  // set type for invite receiver
+  // set hierarchy status for invite receiver
   hierarchy.data.status = Common::HierarchyStatus::ReceivedRequest;
   targetFLData[ targetIdx ] = hierarchy;
 
@@ -72,6 +73,7 @@ bool Sapphire::World::Manager::FriendListMgr::onInviteAccept( Entity::Player& so
   auto& sourceFLData = source.getFriendListData();
   auto& targetFLData = target.getFriendListData();
 
+  // currently, type on hierarchy indicates invite type - since it is no longer an invite, set type to NONE
   sourceFLData[ sourceIdx ].data.status = Common::HierarchyStatus::Added;
   sourceFLData[ sourceIdx ].data.type   = Common::HierarchyType::NONE_2;
   targetFLData[ targetIdx ].data.status = Common::HierarchyStatus::Added;
@@ -140,12 +142,12 @@ bool Sapphire::World::Manager::FriendListMgr::onAssignGroup( Entity::Player& sou
   return true;
 }
 
-bool Sapphire::World::Manager::FriendListMgr::isFriend( Entity::Player& source, Entity::Player& target )
+bool Sapphire::World::Manager::FriendListMgr::isFriend( Entity::Player& source, Entity::Player& target ) const
 {
   return getEntryIndex( source, target.getCharacterId() ) != -1;
 }
 
-ptrdiff_t Sapphire::World::Manager::FriendListMgr::getEntryIndex( Entity::Player& source, uint64_t characterId )
+ptrdiff_t Sapphire::World::Manager::FriendListMgr::getEntryIndex( Entity::Player& source, uint64_t characterId ) const
 {
   auto& sourceFL = source.getFriendListID();
   auto sourceInvIt = std::find( std::begin( sourceFL ), std::end( sourceFL ), characterId );

src/world/Manager/FriendListMgr.h

@@ -19,9 +19,9 @@ namespace Sapphire::World::Manager
     bool onRemoveFriend( Entity::Player& source, Entity::Player& target );
     bool onAssignGroup( Entity::Player& source, Entity::Player& target, uint8_t group );
 
-    bool isFriend( Entity::Player& source, Entity::Player& target );
+    bool isFriend( Entity::Player& source, Entity::Player& target ) const;
 
   private:
-    ptrdiff_t getEntryIndex( Entity::Player& source, uint64_t characterId );
+    ptrdiff_t getEntryIndex( Entity::Player& source, uint64_t characterId ) const;
   };
 }

src/world/Network/Handlers/BlacklistHandler.cpp

@@ -22,83 +22,34 @@ using namespace Sapphire::World::Manager;
 
 void Sapphire::Network::GameConnection::getBlacklistHandler( const Packets::FFXIVARR_PACKET_RAW& inPacket, Entity::Player& player )
 {
-  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
   auto& blMgr = Common::Service< Sapphire::World::Manager::BlacklistMgr >::ref();
 
   const auto packet = ZoneChannelPacket< Client::FFXIVIpcGetBlacklist >( inPacket );
   auto& data = packet.data();
 
-  // TODO: remove this paging test!!
-  /*
-  for( size_t i = 0; i < 200; ++i )
-  {
-    player.getBlacklistID()[i] = player.getCharacterId();
-  }
-  */
-
   blMgr.onGetBlacklistPage( player, data.RequestKey, data.NextIndex );
 }
 
 void Sapphire::Network::GameConnection::blacklistAddHandler( const Packets::FFXIVARR_PACKET_RAW& inPacket, Entity::Player& player )
 {
-  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
   auto& blMgr = Common::Service< Sapphire::World::Manager::BlacklistMgr >::ref();
 
   const auto packet = ZoneChannelPacket< Client::FFXIVIpcBlacklistAdd >( inPacket );
   auto& data = packet.data();
 
-  auto resultPacket = makeZonePacket< Server::FFXIVIpcBlacklistAddResult >( player.getId() );
-
   std::string targetName( data.TargetCharacterName );
 
-  auto target = server.getPlayer( targetName );
+  blMgr.onAddCharacter( player, targetName );
-
-  if( !target || !blMgr.onAddCharacter( player, *target ) )
-  {
-    resultPacket->data().Result = 0x7; // TODO: find the correct value for each invalid arg
-  }
-  else
-  {
-    Server::BlacklistCharacter blChar;
-    blChar.CharacterID = target->getCharacterId();
-    strcpy( blChar.CharacterName, target->getName().c_str() );
-
-    resultPacket->data().AddedCharacter = blChar;
-    resultPacket->data().Identity = target->getGender();
-    resultPacket->data().Result = 0;
-  }
-
-  queueOutPacket( resultPacket );
 }
 
 void Sapphire::Network::GameConnection::blacklistRemoveHandler( const Packets::FFXIVARR_PACKET_RAW& inPacket, Entity::Player& player )
 {
-  auto& server = Common::Service< Sapphire::World::WorldServer >::ref();
   auto& blMgr = Common::Service< Sapphire::World::Manager::BlacklistMgr >::ref();
 
   const auto packet = ZoneChannelPacket< Client::FFXIVIpcBlacklistRemove >( inPacket );
   auto& data = packet.data();
 
-  auto resultPacket = makeZonePacket< Server::FFXIVIpcBlacklistRemoveResult >( player.getId() );
-
   std::string targetName( data.TargetCharacterName );
 
-  auto target = server.getPlayer( targetName );
+  blMgr.onRemoveCharacter( player, targetName );
-
-  if( !target || !blMgr.onRemoveCharacter( player, *target ) )
-  {
-    resultPacket->data().Result = 0x7; // TODO: find the correct value for each invalid arg
-  }
-  else
-  {
-    Server::BlacklistCharacter blChar;
-    blChar.CharacterID = target->getCharacterId();
-    strcpy( blChar.CharacterName, target->getName().c_str() );
-
-    resultPacket->data().RemovedCharacter = blChar;
-    resultPacket->data().Identity = target->getGender();
-    resultPacket->data().Result = 0;
-  }
-
-  queueOutPacket( resultPacket );
 }