achievement code cleanup;

src/world/Actor/Player.cpp

@@ -1335,19 +1335,14 @@ void Player::setTitle( uint16_t titleId )
   sendToInRangeSet( makeActorControl( getId(), SetTitle, titleId ), true );
 }
 
-Player::AchievementList& Player::getAchievementList()
-{
-  return m_achievementList;
-}
-
-Player::AchievementDataList& Player::getAchievementDataList()
+const Player::AchievementData& Player::getAchievementData() const
 {
   return m_achievementData;
 }
 
-Player::AchievementHistory& Player::getAchievementHistory()
+void Player::setAchievementData( const Player::AchievementData& achievementData )
 {
-  return m_achievementHistory;
+  m_achievementData = achievementData;
 }
 
 void Player::setMaxGearSets( uint8_t amount )

src/world/Actor/Player.h

@@ -25,9 +25,6 @@ namespace Sapphire::Entity
   class Player : public Chara
   {
   public:
-    using AchievementDataList = std::map< uint32_t, uint32_t >;
-    using AchievementList = std::array< uint8_t, 2048 / 8 >; // up to 2048 achievements
-    using AchievementHistory = std::array< uint16_t, 5 >;
     using TitleList = std::array< uint8_t, 48 >;
     using HowToList = std::array< uint8_t, 34 >;
     using MinionList = std::array< uint8_t, 40 >;
@@ -42,6 +39,12 @@ namespace Sapphire::Entity
     using ClassList = std::array< uint16_t, 28 >;
     using ExpList = std::array< uint32_t, 28 >;
 
+    struct AchievementData {
+      std::array< uint8_t, 2048 / 8 > unlockList;
+      std::unordered_map< uint32_t, uint32_t > progressData;
+      std::array< uint16_t, 5 > history;
+    };
+
     /*! Contructor */
     Player();
 
@@ -368,14 +371,11 @@ namespace Sapphire::Entity
     /*! send the players title list */
     void sendTitleList();
 
-    /*! get player's achievement list */
-    AchievementList& getAchievementList();
-
-    /*! get player's achievement data list */
-    AchievementDataList& getAchievementDataList();
+    /*! get player's achievement data */
+    const AchievementData& getAchievementData() const;
 
-    /*! get player's achievement data history */
-    AchievementHistory& getAchievementHistory();
+    /*! set player's achievement data */
+    void setAchievementData( const AchievementData& achievementData );
 
     /*! set number of gear sets */
     void setMaxGearSets( uint8_t amount );
@@ -875,9 +875,8 @@ namespace Sapphire::Entity
       uint8_t status;
     } m_retainerInfo[8]{};
 
-    AchievementList m_achievementList{};
-    AchievementDataList m_achievementData{};
-    AchievementHistory m_achievementHistory{};
+    AchievementData m_achievementData{};
+
     uint16_t m_activeTitle{};
     TitleList m_titleList{};
     HowToList m_howTo{};

src/world/Manager/AchievementMgr.cpp

@@ -48,14 +48,16 @@ bool AchievementMgr::cacheAchievements()
 void AchievementMgr::unlockAchievement( Entity::Player& player, uint32_t achievementId )
 {
   auto& exdData = Common::Service< Data::ExdData >::ref();
-  auto achvData = exdData.getRow< Excel::Achievement >( achievementId );
+  auto achvExd = exdData.getRow< Excel::Achievement >( achievementId );
 
   // set flag on mask format expected by client
   uint16_t index;
   uint8_t value;
   Common::Util::valueToFlagByteIndexValue( achievementId, value, index );
 
-  player.getAchievementList()[ index ] |= value;
+  auto achvData = player.getAchievementData();
+  achvData.unlockList[ index ] |= value;
+  player.setAchievementData( achvData );
 
   // handle player achievement history
   // todo: verify retail behavior due to client copying the last achievement unlocked
@@ -68,7 +70,7 @@ void AchievementMgr::unlockAchievement( Entity::Player& player, uint32_t achieve
   Common::Service< World::Manager::PlayerMgr >::ref().onUnlockAchievement( player, achievementId );
 
   // check and add title to player
-  auto achvTitleId = achvData->data().Title;
+  auto achvTitleId = achvExd->data().Title;
   if( achvTitleId != 0 )
   {
     player.addTitle( achvTitleId );
@@ -83,27 +85,27 @@ bool AchievementMgr::hasAchievementUnlocked( Entity::Player& player, uint32_t ac
   uint8_t value;
   Common::Util::valueToFlagByteIndexValue( achievementId, value, index );
 
-  return ( player.getAchievementList()[ index ] & value ) != 0;
+  return ( player.getAchievementData().unlockList[ index ] & value ) != 0;
 }
 
 std::pair< uint32_t, uint32_t > AchievementMgr::getAchievementDataById( Entity::Player& player, uint32_t achievementId )
 {
   auto& exdData = Common::Service< Data::ExdData >::ref();
 
-  auto& achvDataList = player.getAchievementDataList();
-  auto achvExdData = exdData.getRow< Excel::Achievement >( achievementId )->data();
-  auto achvType = static_cast< Common::Achievement::Type >( achvExdData.ConditionType );
+  auto achvDataList = player.getAchievementData().progressData;
+  auto achvExd = exdData.getRow< Excel::Achievement >( achievementId )->data();
+  auto achvType = static_cast< Common::Achievement::Type >( achvExd.ConditionType );
 
   // get paired type:subtype key for stored data
-  auto dataKey = getKeyFromType( achvType, achvExdData.ConditionArg[ 0 ] );
+  auto dataKey = getKeyFromType( achvType, achvExd.ConditionArg[ 0 ] );
 
   // get achievement progress data, if it exists (otherwise pass 0)
   uint32_t currProg = 0;
   if( achvDataList.count( dataKey.u32 ) )
     currProg = achvDataList[ dataKey.u32 ];
 
   // get maximum progress for given achievement, as required by client
-  uint32_t maxProg = static_cast< uint32_t >( achvExdData.ConditionArg[ 1 ] );
+  uint32_t maxProg = static_cast< uint32_t >( achvExd.ConditionArg[ 1 ] );
 
   // cap maximum progress display to maximum progress
   return { std::min( currProg, maxProg ), maxProg };

src/world/Manager/AchievementMgr.h

@@ -58,10 +58,8 @@ namespace Sapphire::World::Manager
     /// <returns>true/false</returns>
     bool hasAchievementUnlocked( Entity::Player& player, uint32_t achievementId );
 
-    void unlockAchievement( Entity::Player& player, uint32_t achievementId );
-
     /// <summary>
-    /// get a pair of current progress and maximum count for a given achievement ID
+    /// get a pair of current progress and maximum count for a given achievement id
     /// </summary>
     /// <param name="player"></param>
     /// <param name="achievementId"></param>
@@ -76,6 +74,7 @@ namespace Sapphire::World::Manager
     AchievementDetailCache m_achievementDetailCacheMap;
     AchievementKeyCache m_achievementKeyCacheMap;
 
+    // cache fetch functions
     std::shared_ptr< Excel::ExcelStruct< Excel::Achievement > > getAchievementDetail( uint32_t achvId ) const;
     std::vector< uint32_t > getAchievementIdByType( Common::Achievement::Type type ) const;
     std::vector< uint32_t > getAchievementIdByType( uint32_t type ) const;
@@ -89,12 +88,20 @@ namespace Sapphire::World::Manager
     Common::AchievementDataKey getKeyFromType( Common::Achievement::Type achvType, int32_t argument );
 
     /// <summary>
-    /// parse and unlock achievements linked to a given achievement Id
+    /// parse and unlock achievements linked to a given achievement id
     /// </summary>
     /// <param name="player"></param>
     /// <param name="achievementId"></param>
     void handleLinkedAchievementsForId( Entity::Player& player, uint32_t achievementId );
 
+    /// <summary>
+    /// internal use: unlock achievement in the player achievement unlock flagmask, from a given id
+    /// progressAchievement should be used instead, due to certain achievements using progress data
+    /// </summary>
+    /// <param name="player"></param>
+    /// <param name="achievementId"></param>
+    void unlockAchievement( Entity::Player& player, uint32_t achievementId );
+
     template< typename AchievementTypeT, AchievementTypeT achievementType >
     inline void progressAchievement( Entity::Player& player, int32_t argument, uint32_t progressCount );
   };
@@ -103,14 +110,16 @@ namespace Sapphire::World::Manager
   template<>
   inline void AchievementMgr::progressAchievement< Common::Achievement::Type, Common::Achievement::Type::General >( Entity::Player& player, int32_t subtype, uint32_t progressCount )
   {
-    auto& achvDataList = player.getAchievementDataList();
+    auto achvData = player.getAchievementData();
 
     auto dataKey = getKeyFromType( Common::Achievement::Type::General, subtype );
 
-    if( !achvDataList.count( dataKey.u32 ) )
-      achvDataList[ dataKey.u32 ] = 0;
+    if( !achvData.progressData.count( dataKey.u32 ) )
+      achvData.progressData[ dataKey.u32 ] = 0;
+
+    achvData.progressData[ dataKey.u32 ] += progressCount;
 
-    achvDataList[ dataKey.u32 ] += progressCount;
+    player.setAchievementData( achvData );
 
     const auto achvIdList = getAchievementIdByType( dataKey.u32 );
 
@@ -125,24 +134,24 @@ namespace Sapphire::World::Manager
 
       auto achvExdData = pAchv->data();
 
-      if( achvExdData.ConditionArg[ 1 ] <= static_cast< int32_t >( achvDataList[ dataKey.u32 ] ) )
+      if( achvExdData.ConditionArg[ 1 ] <= static_cast< int32_t >( achvData.progressData[ dataKey.u32 ] ) )
         unlockAchievement( player, achvId );
     }
   }
 
   template<>
   inline void AchievementMgr::progressAchievement< Common::Achievement::Type, Common::Achievement::Type::Classjob >( Entity::Player& player, int32_t classJob, uint32_t unused )
   {
-    auto& achvDataList = player.getAchievementDataList();
+    auto achvData = player.getAchievementData();
 
     auto dataKey = getKeyFromType( Common::Achievement::Type::Classjob, classJob );
 
-    if( !achvDataList.count( dataKey.u32 ) )
-      achvDataList[ dataKey.u32 ] = 0;
+    if( !achvData.progressData.count( dataKey.u32 ) )
+      achvData.progressData[ dataKey.u32 ] = 0;
 
     auto level = player.getLevelForClass( static_cast< Common::ClassJob >( classJob ) );
 
-    achvDataList[ dataKey.u32 ] = level;
+    achvData.progressData[ dataKey.u32 ] = level;
 
     const auto achvIdList = getAchievementIdByType( dataKey.u32 );
 
@@ -157,15 +166,15 @@ namespace Sapphire::World::Manager
 
       auto achvExdData = pAchv->data();
 
-      if( achvExdData.ConditionArg[ 1 ] <= static_cast< int32_t >( achvDataList[ dataKey.u32 ] ) )
+      if( achvExdData.ConditionArg[ 1 ] <= static_cast< int32_t >( achvData.progressData[ dataKey.u32 ] ) )
         unlockAchievement( player, achvId );
     }
   }
 
   template<>
   inline void AchievementMgr::progressAchievement< Common::Achievement::Type, Common::Achievement::Type::Quest >( Entity::Player& player, int32_t questId, uint32_t unused )
   {
-    auto& achvDataList = player.getAchievementDataList();
+    auto& achvDataList = player.getAchievementData().progressData;
 
     // get achievements that need all achv in args completed
     const auto questAchvAllList = getAchievementIdByType( Common::Achievement::Type::Quest );

src/world/Manager/DebugCommandMgr.cpp

@@ -560,16 +560,6 @@ void DebugCommandMgr::add( char* data, Entity::Player& player, std::shared_ptr<
 
     pSession->getZoneConnection()->queueOutPacket( effectPacket );
   }
-  else if( subCommand == "achv" )
-  {
-    uint32_t achvId;
-
-    sscanf( params.c_str(), "%u", &achvId );
-
-    auto& achvMgr = Common::Service< Manager::AchievementMgr >::ref();
-
-    achvMgr.unlockAchievement( player, achvId );
-  }
   else if( subCommand == "achvGeneral" )
   {
     uint32_t achvSubtype;

src/world/Manager/PlayerMgr.cpp

@@ -83,9 +83,11 @@ void PlayerMgr::onSendAchievementList( Entity::Player& player )
 {
   auto& server = Common::Service< World::WorldServer >::ref();
 
+  auto achvData = player.getAchievementData();
+
   auto achvPacket = makeZonePacket< FFXIVIpcAchievement >( player.getId() );
-  std::memcpy( &achvPacket->data().complete[ 0 ], &player.getAchievementList()[ 0 ], sizeof( achvPacket->data().complete ) );
-  std::memcpy( &achvPacket->data().history[ 0 ], &player.getAchievementHistory()[ 0 ], sizeof( achvPacket->data().history ) );
+  std::memcpy( &achvPacket->data().complete[ 0 ], &achvData.unlockList[ 0 ], sizeof( achvPacket->data().complete ) );
+  std::memcpy( &achvPacket->data().history[ 0 ], &achvData.history[ 0 ], sizeof( achvPacket->data().history ) );
 
   server.queueForPlayer( player.getCharacterId(), achvPacket );
 }