Cleanup: General CMake and warnings cleanup

Author: zach2good

CMakeLists.txt

@@ -15,6 +15,12 @@ include(Sanitizers)
 include(ClangTidy)
 include(Tracy)
 
+# CMake adds "/W3" by default on MSVC, remove it.
+# Fixed with CMake 3.15
+if(MSVC)
+    string(REGEX REPLACE "/W3[ ]" "" CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
+endif()
+
 message(STATUS "CMAKE_VERSION: ${CMAKE_VERSION}")
 message(STATUS "CMAKE_BUILD_TYPE: ${CMAKE_BUILD_TYPE}")
 message(STATUS "CMAKE_C_COMPILER: ${CMAKE_C_COMPILER}")
@@ -55,6 +61,10 @@ target_compile_features(project_options INTERFACE cxx_std_17)
 add_library(project_warnings INTERFACE)
 set_project_warnings(project_warnings)
 
+# Link this 'library' to disable all warnings
+add_library(no_warnings INTERFACE)
+set_no_warnings(no_warnings)
+
 # Ouput executables into repo root
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_SOURCE_DIR})
 set(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_SOURCE_DIR})

cmake/CompilerWarnings.cmake

@@ -3,11 +3,17 @@
 # https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md
 
 function(set_project_warnings project_name)
-  option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" TRUE)
-
   set(MSVC_WARNINGS
       /W4 # Baseline reasonable warnings
-      /w14242 # 'identifier': conversion from 'type1' to 'type1', possible loss of data
+
+      # /wd Disable
+      /wd4100 # unreferenced formal parameter
+      /wd4127 # conditional expression is constant
+      /wd4201 # nonstandard extension used: nameless struct/union
+      /wd4242 # 'identifier': conversion from 'type1' to 'type1', possible loss of data
+      /wd4244 # 'argument': conversion from 'const type1' to 'type2', possible loss of data
+
+      # /w1 Promote to level 1
       /w14254 # 'operator': conversion from 'type1:field_bits' to 'type2:field_bits', possible loss of data
       /w14263 # 'function': member function does not override any base class virtual member function
       /w14265 # 'classname': class has virtual functions, but destructor is not virtual instances of this class may not
@@ -29,9 +35,6 @@ function(set_project_warnings project_name)
       /w14906 # string literal cast to 'LPWSTR'
       /w14928 # illegal copy-initialization; more than one user-defined conversion has been implicitly applied
       /permissive- # standards conformance mode for MSVC compiler.
-
-      # TODO:
-      /w4100 # unreferenced formal parameter
   )
 
   set(CLANG_WARNINGS
@@ -41,7 +44,7 @@ function(set_project_warnings project_name)
       -Wnon-virtual-dtor # warn the user if a class with virtual functions has a non-virtual destructor. This helps
                          # catch hard to track down memory errors
       # TODO: -Wold-style-cast # warn for c-style casts
-      -Wcast-align # warn for potential performance problem casts
+      # TODO: -Wcast-align # warn for potential performance problem casts
       # TODO: -Wunused # warn on anything being unused
       -Woverloaded-virtual # warn if you overload (not override) a virtual function
       # TODO: -Wpedantic # warn if non-standard C++ is used
@@ -60,11 +63,6 @@ function(set_project_warnings project_name)
       -Wno-sizeof-pointer-memaccess
   )
 
-  if(WARNINGS_AS_ERRORS)
-    set(CLANG_WARNINGS ${CLANG_WARNINGS} -Werror)
-    set(MSVC_WARNINGS ${MSVC_WARNINGS} /WX)
-  endif()
-
   set(GCC_WARNINGS
       ${CLANG_WARNINGS}
       -Wmisleading-indentation # warn if indentation implies blocks where blocks do not exist
@@ -93,6 +91,24 @@ function(set_project_warnings project_name)
     message(AUTHOR_WARNING "No compiler warnings set for '${CMAKE_CXX_COMPILER_ID}' compiler.")
   endif()
 
-  target_compile_options(${project_name} INTERFACE ${PROJECT_WARNINGS})
+  option(WARNINGS_AS_ERRORS "Treat compiler warnings as errors" TRUE)
+  if(WARNINGS_AS_ERRORS)
+    if(UNIX)
+      set(ERRORS "-Werror")
+    elseif(WIN32)
+      set(ERRORS "/WX")
+    endif()
+  endif()
 
-endfunction()
+  target_compile_options(${project_name} INTERFACE ${ERRORS} ${PROJECT_WARNINGS})
+endfunction() #set_project_warnings
+
+function(set_no_warnings project_name)
+  if(WARNINGS_AS_ERRORS)
+    if(UNIX)
+      target_compile_options(${project_name} INTERFACE "-Wno-everything")
+    elseif(WIN32)
+      target_compile_options(${project_name} INTERFACE "/w")
+    endif()
+  endif()
+endfunction() # set_no_warnings

src/common/utils.cpp

@@ -152,16 +152,12 @@ uint8 worldAngle(const position_t& A, const position_t& B)
 
 uint8 relativeAngle(uint8 world, int16 diff)
 {
-    uint8 angle = world + diff;
+    int16 angle = world + diff;
     if (angle < 0)
     {
         angle = 256 - abs(angle);
     }
-    else
-    {
-        angle = angle % 256;
-    }
-    return angle;
+    return static_cast<uint8>(angle);
 }
 
 int16 angleDifference(uint8 worldAngleA, uint8 worldAngleB)

src/map/ai/controllers/mob_controller.cpp

@@ -774,7 +774,7 @@ void CMobController::DoRoamTick(time_point tick)
                     // move back every 5 seconds
                     m_LastActionTime = m_Tick - (std::chrono::milliseconds(PMob->getBigMobMod(MOBMOD_ROAM_COOL)) + 10s);
                 }
-                else if (!PMob->getMobMod(MOBMOD_NO_DESPAWN) != 0 &&
+                else if (!(PMob->getMobMod(MOBMOD_NO_DESPAWN) != 0) &&
                     !map_config.mob_no_despawn)
                 {
                     PMob->PAI->Despawn();

src/map/guild.cpp

@@ -120,7 +120,7 @@ std::pair<uint16, uint16> CGuild::getDailyGPItem(CCharEntity* PChar)
 
     auto GPItem = m_GPItems[rank - 3];
     auto curPoints = (uint16)charutils::GetCharVar(PChar, "[GUILD]daily_points");
-    if (curPoints == -1)
+    if (curPoints == 0)
     {
         return std::make_pair(GPItem[0].item->getID(), 0);
     }

src/map/lua/lua_baseentity.cpp

@@ -699,17 +699,15 @@ inline int32 CLuaBaseEntity::injectPacket(lua_State *L)
             return 0;
         }
 
-        if (size <= 256)
+        fseek(File, 0, SEEK_SET);
+        if (fread(*PPacket, 1, size * 2, File) != size * 2)
         {
-            fseek(File, 0, SEEK_SET);
-            if (fread(*PPacket, 1, size * 2, File) != size * 2)
-            {
-                ShowError(CL_RED"CLuaBaseEntity::injectPacket : Did not read entire packet\n" CL_RESET);
-                return 0;
-            }
-
-            ((CCharEntity*)m_PBaseEntity)->pushPacket(PPacket);
+            ShowError(CL_RED"CLuaBaseEntity::injectPacket : Did not read entire packet\n" CL_RESET);
+            return 0;
         }
+
+        ((CCharEntity*)m_PBaseEntity)->pushPacket(PPacket);
+
         fclose(File);
     }
     else
@@ -6268,7 +6266,7 @@ inline int32 CLuaBaseEntity::addQuest(lua_State *L)
 
     CCharEntity* PChar = (CCharEntity*)m_PBaseEntity;
     uint8 questLogID = (uint8)lua_tointeger(L, lua_isnumber(L, 1) ? 1 : -1);
-    uint8 questID = (uint8)lua_tointeger(L, 2);
+    uint16 questID = (uint16)lua_tointeger(L, 2);
 
     if (questLogID < MAX_QUESTAREA && questID < MAX_QUESTID)
     {
@@ -6312,7 +6310,7 @@ inline int32 CLuaBaseEntity::delQuest(lua_State *L)
 
     CCharEntity* PChar = (CCharEntity*)m_PBaseEntity;
     uint8 questLogID = (uint8)lua_tointeger(L, lua_isnumber(L, 1) ? 1 : -1);
-    uint8 questID = (uint8)lua_tointeger(L, 2);
+    uint16 questID = (uint16)lua_tointeger(L, 2);
 
     if (questLogID < MAX_QUESTAREA && questID < MAX_QUESTID)
     {
@@ -6358,7 +6356,7 @@ inline int32 CLuaBaseEntity::getQuestStatus(lua_State *L)
     TPZ_DEBUG_BREAK_IF(lua_isnil(L, 2) || !lua_isnumber(L, 2));
 
     uint8 questLogID = (uint8)lua_tointeger(L, lua_isnumber(L, 1) ? 1 : -1);
-    uint8 questID = (uint8)lua_tointeger(L, 2);
+    uint16 questID = (uint16)lua_tointeger(L, 2);
 
     if (questLogID < MAX_QUESTAREA && questID < MAX_QUESTID)
     {
@@ -6396,7 +6394,7 @@ inline int32 CLuaBaseEntity::hasCompletedQuest(lua_State *L)
     TPZ_DEBUG_BREAK_IF(lua_isnil(L, 2) || !lua_isnumber(L, 2));
 
     uint8 questLogID = (uint8)lua_tointeger(L, lua_isnumber(L, 1) ? 1 : -1);
-    uint8 questID = (uint8)lua_tointeger(L, 2);
+    uint16 questID = (uint16)lua_tointeger(L, 2);
 
     if (questLogID < MAX_QUESTAREA && questID < MAX_QUESTID)
     {
@@ -6432,7 +6430,7 @@ inline int32 CLuaBaseEntity::completeQuest(lua_State *L)
 
     CCharEntity* PChar = (CCharEntity*)m_PBaseEntity;
     uint8 questLogID = (uint8)lua_tointeger(L, lua_isnumber(L, 1) ? 1 : -1);
-    uint8 questID = (uint8)lua_tointeger(L, 2);
+    uint16 questID = (uint16)lua_tointeger(L, 2);
 
     if (questLogID < MAX_QUESTAREA && questID < MAX_QUESTID)
     {

src/map/utils/battleutils.cpp

@@ -556,7 +556,7 @@ namespace battleutils
         // int16 intStat = PDefender->INT();
         // int16 mattStat = PDefender->getMod(Mod::MATT);
 
-        switch (Action->spikesEffect)
+        switch (static_cast<SPIKES>(Action->spikesEffect))
         {
             case SPIKE_DREAD:
                 // drain same as damage taken
@@ -632,7 +632,7 @@ namespace battleutils
             // calculate damage
             Action->spikesParam = HandleStoneskin(PAttacker, CalculateSpikeDamage(PAttacker, PDefender, Action, (uint16)(abs(damage))));
 
-            switch (Action->spikesEffect)
+            switch (static_cast<SPIKES>(Action->spikesEffect))
             {
                 case SPIKE_BLAZE:
                 case SPIKE_ICE:

src/map/utils/charutils.cpp

@@ -1915,7 +1915,7 @@ namespace charutils
                                 {
                                     UnequipItem(PChar, SLOT_MAIN, false);
                                 }
-                                else if (!((CItemWeapon*)PItem)->getSkillType() == SKILL_NONE)
+                                else if (!(((CItemWeapon*)PItem)->getSkillType() == SKILL_NONE))
                                 {
                                     //allow Grips to be equipped
                                     return false;
@@ -2268,7 +2268,7 @@ namespace charutils
                 // Unequip if no main weapon or a non-grip subslot without DW
                 if (!PChar->getEquip(SLOT_MAIN) ||
                     (!charutils::hasTrait(PChar, TRAIT_DUAL_WIELD) &&
-                     !((CItemWeapon*)PItem)->getSkillType() == SKILL_NONE))
+                     !(((CItemWeapon*)PItem)->getSkillType() == SKILL_NONE)))
                 {
                     UnequipItem(PChar, SLOT_SUB);
                     continue;

src/map/utils/fishingutils.cpp

@@ -203,7 +203,7 @@ bool CheckFisherLuck(CCharEntity* PChar)
                 // ловля предметов, необходимых для поисков
 
                 uint8 logid = (uint8)Sql_GetIntData(SqlHandle,5);
-                uint8 quest = (uint8)Sql_GetIntData(SqlHandle,6);
+                uint16 quest = (uint16)Sql_GetIntData(SqlHandle,6);
 
                 if(logid < MAX_QUESTAREA && quest < MAX_QUESTID)
 	            {