Clang tidy cleanups

src/world/Actor/BNpc.cpp

@@ -4,9 +4,7 @@
 #include <Exd/ExdData.h>
 #include <utility>
 #include <Network/CommonActorControl.h>
-#include <Network/PacketWrappers/EffectPacket.h>
 #include <Network/PacketWrappers/EffectPacket1.h>
-#include <Network/PacketDef/Zone/ClientZoneDef.h>
 #include <Logging/Logger.h>
 
 #include "Forwards.h"
@@ -18,24 +16,20 @@
 #include "Network/PacketWrappers/ActorControlPacket.h"
 #include "Network/PacketWrappers/ActorControlSelfPacket.h"
 #include "Network/PacketWrappers/ActorControlTargetPacket.h"
-#include "Network/PacketWrappers/UpdateHpMpTpPacket.h"
 #include "Network/PacketWrappers/NpcSpawnPacket.h"
 #include "Network/PacketWrappers/MoveActorPacket.h"
 #include "Navi/NaviProvider.h"
 
-#include "Math/CalcBattle.h"
 #include "Math/CalcStats.h"
 
 #include "WorldServer.h"
 #include "Session.h"
 #include "Chara.h"
-#include "Player.h"
 #include "BNpc.h"
 
 #include "Common.h"
 
 #include <Manager/TerritoryMgr.h>
-#include <Manager/NaviMgr.h>
 #include <Manager/RNGMgr.h>
 #include <Manager/PlayerMgr.h>
 #include <Manager/TaskMgr.h>
@@ -55,7 +49,7 @@ Sapphire::Entity::BNpc::BNpc() :
 {
 }
 
-Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, TerritoryPtr pZone ) :
+Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, const Territory& zone ) :
   Npc( ObjKind::BattleNpc )
 {
   m_id = id;
@@ -104,8 +98,8 @@ Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstance
 
   m_class = ClassJob::Gladiator;
 
-  m_territoryTypeId = pZone->getTerritoryTypeId();
+  m_territoryTypeId = zone.getTerritoryTypeId();
-  m_territoryId = pZone->getGuId();
+  m_territoryId = zone.getGuId();
 
   m_spawnPos = m_pos;
 
@@ -173,7 +167,7 @@ Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstance
 
 }
 
-Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, TerritoryPtr pZone, uint32_t hp, Common::BNpcType type ) :
+Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, const Territory& zone, uint32_t hp, Common::BNpcType type ) :
   Npc( ObjKind::BattleNpc )
 {
   m_id = id;
@@ -201,8 +195,8 @@ Sapphire::Entity::BNpc::BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstance
   m_flags = 0;
   m_rank = pInfo->BNPCRankId;
 
-  m_territoryTypeId = pZone->getTerritoryTypeId();
+  m_territoryTypeId = zone.getTerritoryTypeId();
-  m_territoryId = pZone->getGuId();
+  m_territoryId = zone.getGuId();
 
   if( pInfo->WanderingRange == 0 || pInfo->BoundInstanceID != 0 )
     setFlag( Immobile );
@@ -588,7 +582,6 @@ bool Sapphire::Entity::BNpc::hateListHasActor( const Sapphire::Entity::CharaPtr&
 {
   return std::any_of( m_hateList.begin(), m_hateList.end(),
                       [ pChara ]( const auto& entry ) { return entry->m_pChara == pChara; } );
-  return false;
 }
 
 void Sapphire::Entity::BNpc::aggro( const Sapphire::Entity::CharaPtr& pChara )
@@ -832,7 +825,6 @@ void Sapphire::Entity::BNpc::onActionHostile( Sapphire::Entity::CharaPtr pSource
 
 void Sapphire::Entity::BNpc::onDeath()
 {
-  auto& server = Common::Service< World::WorldServer >::ref();
   auto& playerMgr = Common::Service< World::Manager::PlayerMgr >::ref();
   auto& taskMgr = Common::Service< World::Manager::TaskMgr >::ref();
 
@@ -893,7 +885,7 @@ void Sapphire::Entity::BNpc::checkAggro()
       if( levelDiff >= 10 )
         range = 0.f;
       else
-        range = std::max< float >( 0.f, range - std::pow( 1.53f, levelDiff * 0.6f ) );
+        range = std::max< float >( 0.f, range - std::pow( 1.53f, static_cast< float >( levelDiff ) * 0.6f ) );
     }
 
     auto distance = Util::distance( getPos(), pClosestChara->getPos() );

src/world/Actor/BNpc.h

@@ -64,8 +64,8 @@ namespace Sapphire::Entity
   public:
     BNpc();
 
-    BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, TerritoryPtr pZone );
+    BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, const Territory& zone );
-    BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, TerritoryPtr pZone, uint32_t hp, Common::BNpcType type );
+    BNpc( uint32_t id, std::shared_ptr< Common::BNPCInstanceObject > pInfo, const Territory& zone, uint32_t hp, Common::BNpcType type );
 
     virtual ~BNpc() override;
 

src/world/Actor/Chara.cpp

@@ -1,6 +1,5 @@
 #include <Util/Util.h>
 #include <Util/UtilMath.h>
-#include <Network/PacketContainer.h>
 #include <Exd/ExdData.h>
 #include <utility>
 #include <Network/CommonActorControl.h>
@@ -13,7 +12,6 @@
 
 #include "Network/GameConnection.h"
 #include "Network/PacketWrappers/ActorControlPacket.h"
-#include "Network/PacketWrappers/ActorControlSelfPacket.h"
 #include "Network/PacketWrappers/ActorControlTargetPacket.h"
 #include "Network/PacketWrappers/UpdateHpMpTpPacket.h"
 #include "Network/PacketWrappers/EffectPacket1.h"
@@ -60,7 +58,7 @@ Sapphire::Entity::Chara::~Chara()
 /*! \return the actors name */
 std::string Sapphire::Entity::Chara::getName() const
 {
-  return std::string( m_name );
+  return { m_name };
 }
 
 
@@ -539,7 +537,7 @@ void Sapphire::Entity::Chara::addStatusEffect( StatusEffect::StatusEffectPtr pEf
   status.Time = static_cast< float >( pEffect->getDuration() ) / 1000;
   status.Id = static_cast< uint16_t >( pEffect->getId() );
   status.Slot = static_cast< uint8_t >( nextSlot );
-  status.SystemParam = pEffect->getParam();
+  status.SystemParam = static_cast< int16_t >( pEffect->getParam() );
 
   sendToInRangeSet( statusEffectAdd, isPlayer() );
 }
@@ -585,7 +583,7 @@ void Sapphire::Entity::Chara::statusEffectFreeSlot( uint8_t slotId )
 
 void Sapphire::Entity::Chara::removeSingleStatusEffectById( uint32_t id )
 {
-  for( auto effectIt : m_statusEffectMap )
+  for( const auto& effectIt : m_statusEffectMap )
   {
     if( effectIt.second->getId() == id )
     {
@@ -645,7 +643,7 @@ void Sapphire::Entity::Chara::sendStatusEffectUpdate()
 
   auto statusEffectList = makeZonePacket< FFXIVIpcStatus >( getId() );
   uint8_t slot = 0;
-  for( auto effectIt : m_statusEffectMap )
+  for( const auto& effectIt : m_statusEffectMap )
   {
     float timeLeft = static_cast< float >( effectIt.second->getDuration() -
                                            ( currentTimeMs - effectIt.second->getStartTimeMs() ) ) / 1000;
@@ -663,7 +661,7 @@ void Sapphire::Entity::Chara::updateStatusEffects()
 {
   uint64_t currentTimeMs = Util::getTimeMs();
 
-  for( auto effectIt : m_statusEffectMap )
+  for( const auto& effectIt : m_statusEffectMap )
   {
     uint8_t effectIndex = effectIt.first;
     auto effect = effectIt.second;
@@ -710,7 +708,8 @@ uint32_t Sapphire::Entity::Chara::getLastComboActionId() const
   // initially check for the time passed first, if it's more than the threshold just return 0 for the combo
   // we can hide the implementation detail this way and it just works:tm: for anything that uses it
 
-  if( std::difftime( Util::getTimeMs(), m_lastComboActionTime ) > Common::MAX_COMBO_LENGTH )
+  if( std::difftime( static_cast< time_t >( Util::getTimeMs() ),
+                     static_cast< time_t >( m_lastComboActionTime ) ) > Common::MAX_COMBO_LENGTH )
   {
     return 0;
   }
@@ -791,7 +790,7 @@ void Sapphire::Entity::Chara::onTick()
   uint32_t thisTickDmg = 0;
   uint32_t thisTickHeal = 0;
 
-  for( auto effectIt : m_statusEffectMap )
+  for( const auto& effectIt : m_statusEffectMap )
   {
     auto thisEffect = effectIt.second->getTickEffect();
     switch( thisEffect.first )

src/world/Actor/EventObject.cpp

@@ -4,7 +4,6 @@
 #include "Actor/Player.h"
 
 #include "Network/PacketWrappers/ActorControlPacket.h"
-#include "Network/PacketWrappers/ActorControlSelfPacket.h"
 #include "Network/PacketWrappers/ActorControlTargetPacket.h"
 
 #include <Logging/Logger.h>
@@ -13,6 +12,8 @@
 #include <Util/UtilMath.h>
 
 #include <Service.h>
+
+#include <utility>
 #include "WorldServer.h"
 #include "Session.h"
 #include "Network/GameConnection.h"
@@ -70,7 +71,7 @@ Sapphire::Entity::EventObject::OnTalkEventHandler Sapphire::Entity::EventObject:
 
 void Sapphire::Entity::EventObject::setOnTalkHandler( Sapphire::Entity::EventObject::OnTalkEventHandler handler )
 {
-  m_onTalkEventHandler = handler;
+  m_onTalkEventHandler = std::move( handler );
 }
 
 void Sapphire::Entity::EventObject::setGimmickId( uint32_t gimmickId )
@@ -111,7 +112,7 @@ uint32_t Sapphire::Entity::EventObject::getHousingLink() const
 
 void Sapphire::Entity::EventObject::setParentInstance( Sapphire::TerritoryPtr instance )
 {
-  m_parentInstance = instance;
+  m_parentInstance = std::move( instance );
 }
 
 Sapphire::TerritoryPtr Sapphire::Entity::EventObject::getParentInstance() const

src/world/Actor/EventObject.h

@@ -62,7 +62,7 @@ namespace Sapphire::Entity
     uint32_t m_ownerId;
     uint8_t m_state;
     uint8_t m_permissionInvisibility;
-    float m_scale;
+    float m_scale{};
     std::string m_name;
     TerritoryPtr m_parentInstance;
     OnTalkEventHandler m_onTalkEventHandler;

src/world/Actor/GameObject.cpp

@@ -2,7 +2,6 @@
 
 #include <Network/PacketContainer.h>
 
-#include <Util/Util.h>
 #include <Util/UtilMath.h>
 #include <utility>
 #include <Service.h>
@@ -23,8 +22,6 @@
 
 #include "StatusEffect/StatusEffect.h"
 
-#include "Math/CalcBattle.h"
-
 using namespace Sapphire::Common;
 using namespace Sapphire::Network::Packets;
 //using namespace Sapphire::Network::Packets::Server;
@@ -245,8 +242,6 @@ void Sapphire::Entity::GameObject::removeFromInRange()
   if( !hasInRangeActor() )
     return;
 
-  Entity::GameObjectPtr pCurAct;
-
   for( auto& pCurAct : m_inRangeActor )
   {
     pCurAct->removeInRangeActor( *this );

src/world/Actor/Npc.cpp

@@ -1,10 +1,7 @@
 #include <Network/PacketContainer.h>
-#include <Exd/ExdData.h>
 #include "Action/Action.h"
 #include "Territory/Territory.h"
-#include "Network/PacketWrappers/UpdateHpMpTpPacket.h"
 
-#include "WorldServer.h"
 #include "Player.h"
 #include "Npc.h"
 #include "Common.h"

src/world/Actor/Player.cpp

@@ -34,10 +34,8 @@
 #include "Network/PacketWrappers/PlayerSetupPacket.h"
 
 #include "Network/PacketWrappers/PlayerSpawnPacket.h"
-#include "Network/PacketWrappers/EffectPacket.h"
 #include "Network/PacketWrappers/EffectPacket1.h"
 #include "Network/PacketWrappers/InitZonePacket.h"
-#include "Network/PacketWrappers/WarpPacket.h"
 
 #include "Action/Action.h"
 
@@ -93,12 +91,12 @@ Player::Player() :
   m_invincibilityType = InvincibilityType::InvincibilityNone;
   m_radius = 1.f;
 
-  std::memset( m_questTracking.data(), 0, sizeof( m_questTracking ) );
   memset( m_name, 0, sizeof( m_name ) );
-  memset( m_stateFlags.data(), 0, m_stateFlags.size() );
   memset( m_searchMessage, 0, sizeof( m_searchMessage ) );
-  memset( m_classArray.data(), 0, sizeof( m_classArray.data() ) );
+  std::fill( std::begin( m_questTracking ), std::end( m_questTracking ), 0 );
-  memset( m_expArray.data(), 0, sizeof( m_expArray.data() ) );
+  std::fill( std::begin( m_stateFlags ), std::end( m_stateFlags ), 0 );
+  std::fill( std::begin( m_classArray ), std::end( m_classArray ), 0 );
+  std::fill( std::begin( m_expArray ), std::end( m_expArray ), 0 );
 
   for( uint8_t i = 0; i < 80; ++i )
   {

src/world/Actor/PlayerEvent.cpp

@@ -1,7 +1,4 @@
-#include <Common.h>
-
 #include <Network/PacketContainer.h>
-#include <Service.h>
 #include <Exd/ExdData.h>
 
 #include <utility>
@@ -70,14 +67,14 @@ void Sapphire::Entity::Player::onTick()
   if( !isAlive() || !isLoadingComplete() )
     return;
 
-  uint32_t addHp = static_cast< uint32_t >( getMaxHp() * 0.1f + 1 );
+  auto addHp = static_cast< uint32_t >( static_cast< float >( getMaxHp() ) * 0.1f + 1 );
-  uint32_t addMp = static_cast< uint32_t >( getMaxMp() * 0.06f + 1 );
+  auto addMp = static_cast< uint32_t >( static_cast< float >( getMaxMp() ) * 0.06f + 1 );
   uint32_t addTp = 100;
 
   if( !m_actorIdTohateSlotMap.empty() )
   {
-    addHp = static_cast< uint32_t >( getMaxHp() * 0.01f + 1 );
+    addHp = static_cast< uint32_t >( static_cast< float >( getMaxHp() ) * 0.01f + 1 );
-    addMp = static_cast< uint32_t >( getMaxMp() * 0.02f + 1 );
+    addMp = static_cast< uint32_t >( static_cast< float >( getMaxMp() ) * 0.02f + 1 );
     addTp = 60;
   }
 

src/world/Actor/PlayerInventory.cpp

@@ -9,7 +9,6 @@
 #include "Inventory/ItemContainer.h"
 
 #include <Exd/ExdData.h>
-#include <Logging/Logger.h>
 #include <Database/DatabaseDef.h>
 
 #include "Actor/Player.h"
@@ -18,7 +17,6 @@
 #include "Network/PacketWrappers/ActorControlPacket.h"
 #include "Network/PacketWrappers/ActorControlSelfPacket.h"
 #include "Network/PacketWrappers/UpdateInventorySlotPacket.h"
-#include "Network/PacketWrappers/ServerNoticePacket.h"
 #include <Network/PacketDef/Zone/ServerZoneDef.h>
 
 #include "Manager/InventoryMgr.h"
@@ -438,9 +436,9 @@ void Sapphire::Entity::Player::sendInventory()
 {
   auto& invMgr = Common::Service< World::Manager::InventoryMgr >::ref();
 
-  for( auto it = m_storageMap.begin(); it != m_storageMap.end(); ++it )
+  for( auto& it : m_storageMap )
   {
-    invMgr.sendInventoryContainer( *this, it->second );
+    invMgr.sendInventoryContainer( *this, it.second );
   }
 }
 
@@ -450,7 +448,7 @@ Sapphire::Entity::Player::InvSlotPairVec Sapphire::Entity::Player::getSlotsOfIte
   for( auto i : { Bag0, Bag1, Bag2, Bag3 } )
   {
     auto inv = m_storageMap[ i ];
-    for( auto item : inv->getItemMap() )
+    for( const auto& item : inv->getItemMap() )
     {
       if( item.second && item.second->getId() == catalogId )
         outVec.push_back( std::make_pair( i, static_cast< int8_t >( item.first ) ) );
@@ -1046,7 +1044,7 @@ void Sapphire::Entity::Player::addSoldItem( uint32_t itemId, uint8_t stackSize )
 {
   if( m_soldItems.size() > 10 )
     m_soldItems.pop_back();
-  m_soldItems.push_front( std::make_pair( itemId, stackSize ) );
+  m_soldItems.emplace_front( itemId, stackSize );
 }
 
 std::deque< std::pair< uint32_t, uint8_t > > *Sapphire::Entity::Player::getSoldItems()
@@ -1069,7 +1067,7 @@ bool Sapphire::Entity::Player::getFreeInventoryContainerSlot( Inventory::Invento
 
     auto& container = needle->second;
 
-    for( uint8_t idx = 0; idx < container->getMaxSize(); idx++ )
+    for( uint16_t idx = 0; idx < container->getMaxSize(); idx++ )
     {
       auto item = container->getItem( idx );
       if( !item )

src/world/Actor/PlayerQuest.cpp

@@ -1,15 +1,11 @@
-#include <Common.h>
 #include <Network/PacketDef/Zone/ServerZoneDef.h>
 #include <Exd/ExdData.h>
-#include <Network/PacketContainer.h>
 #include <Service.h>
 
-#include "Network/GameConnection.h"
 #include "Network/PacketWrappers/Notice2Packet.h"
 
 #include "Manager/QuestMgr.h"
 
-#include "Session.h"
 #include "Player.h"
 
 
@@ -118,8 +114,7 @@ bool Sapphire::Entity::Player::addQuest( const World::Quest& quest )
 
 int8_t Sapphire::Entity::Player::getFreeQuestSlot()
 {
-  int8_t idx = 0;
+  int8_t result = -1;
-  int result = -1;
   for( int8_t idx = 0; idx < 30; idx++ )
     if( m_quests[ idx ].getId() == 0 )
     {

src/world/Actor/PlayerSql.cpp

@@ -3,14 +3,12 @@
 #include <Common.h>
 #include <Logging/Logger.h>
 #include <Exd/ExdData.h>
-#include <Network/PacketContainer.h>
 #include <Database/DatabaseDef.h>
 #include <Service.h>
 
 #include "Network/PacketWrappers/PlayerSetupPacket.h"
 
 #include "Manager/TerritoryMgr.h"
-#include "Territory/Territory.h"
 #include "Inventory/Item.h"
 #include "Inventory/ItemContainer.h"
 #include "Manager/ItemMgr.h"

src/world/Territory/Territory.cpp

@@ -828,7 +828,7 @@ Entity::BNpcPtr Territory::createBNpcFromLayoutId( uint32_t levelId, uint32_t hp
   if( infoPtr == m_bNpcBaseMap.end() )
     return nullptr;
 
-  auto pBNpc = std::make_shared< Entity::BNpc >( getNextActorId(), infoPtr->second, shared_from_this(), hp, bnpcType );
+  auto pBNpc = std::make_shared< Entity::BNpc >( getNextActorId(), infoPtr->second, *this, hp, bnpcType );
   pBNpc->setTriggerOwnerId( triggerOwnerId );
   pushActor( pBNpc );
   return pBNpc;