From b48c0eac6b011ec0c28dad552a659d9b8c5c5b0e Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 16 Jan 2025 10:29:52 +0100 Subject: [PATCH 01/13] Github workflows ignore .txt and .md files --- .github/workflows/build_aux_files.yml | 3 +++ .github/workflows/build_linux_x86.yml | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/build_aux_files.yml b/.github/workflows/build_aux_files.yml index 9dc820fbe..8fc747e9d 100644 --- a/.github/workflows/build_aux_files.yml +++ b/.github/workflows/build_aux_files.yml @@ -6,6 +6,9 @@ on: - 'master' - 'main' - 'dev' + paths-ignore: + - '*.txt' + - '*.md' jobs: upload_github_release: diff --git a/.github/workflows/build_linux_x86.yml b/.github/workflows/build_linux_x86.yml index 557da3502..5ab5776fc 100644 --- a/.github/workflows/build_linux_x86.yml +++ b/.github/workflows/build_linux_x86.yml @@ -6,11 +6,16 @@ on: - 'master' - 'main' - 'dev' + paths-ignore: + - '*.txt' pull_request: branches: - 'master' - 'main' - 'dev' + paths-ignore: + - '*.txt' + - '*.md' jobs: linux-x86: From 0f1baa67349fdf1ff227021d85a125c9a2dc9cd0 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Fri, 18 Oct 2024 11:13:48 +0200 Subject: [PATCH 02/13] Fixed: SQLite UTF8 strings handling in linux. Error in getting the length in bytes of strings containing non-ASCII characters. Refactored some CSQLite methods, for better reading. --- src/common/CException.cpp | 1 + src/common/assertion.h | 1 + src/common/sphere_library/sstring.cpp | 84 +++++++------------ src/common/sphere_library/sstring.h | 22 +++-- src/common/sqlite/SQLite.cpp | 116 ++++++++------------------ src/common/sqlite/SQLite.h | 9 +- 6 files changed, 88 insertions(+), 145 deletions(-) diff --git a/src/common/CException.cpp b/src/common/CException.cpp index 6b41a9db0..c2d94ee88 100644 --- a/src/common/CException.cpp +++ b/src/common/CException.cpp @@ -229,6 +229,7 @@ bool CWinStructuredException::GetErrorMessage(lptstr lpszError, uint uiMaxError) // -------------------------------------------------------------------------------- // -------------------------------------------------------------------------------- +[[noreturn]] void Assert_Fail( lpctstr pExp, lpctstr pFile, long long llLine ) { EXC_NOTIFY_DEBUGGER; diff --git a/src/common/assertion.h b/src/common/assertion.h index fd96e715a..296e7d7f0 100644 --- a/src/common/assertion.h +++ b/src/common/assertion.h @@ -7,6 +7,7 @@ #endif #endif +[[noreturn]] extern void Assert_Fail(const char * pExp, const char *pFile, long long llLine); #define ASSERT_ALWAYS(exp) if ( !(exp) ) [[unlikely]] Assert_Fail(#exp, __FILE__, __LINE__) diff --git a/src/common/sphere_library/sstring.cpp b/src/common/sphere_library/sstring.cpp index 1c9953f30..c5916329a 100644 --- a/src/common/sphere_library/sstring.cpp +++ b/src/common/sphere_library/sstring.cpp @@ -479,7 +479,7 @@ size_t strlen_mb(const char* ptr) } */ -size_t Str_LengthUTF8(const char* strInUTF8MB) noexcept +size_t Str_UTF8CharCount(const char* strInUTF8MB) noexcept { size_t len; // number of characters in the string #ifdef MSVC_RUNTIME @@ -1695,94 +1695,71 @@ void CharToMultiByteNonNull(byte * Dest, const char * Src, int MBytes) noexcept } } -UTF8MBSTR::UTF8MBSTR() noexcept -{ - m_strUTF8_MultiByte = new char[1](); -} +UTF8MBSTR::UTF8MBSTR() = default; -UTF8MBSTR::UTF8MBSTR(lpctstr lpStr) noexcept +UTF8MBSTR::UTF8MBSTR(lpctstr lpStr) { - if (lpStr) - { - m_strUTF8_MultiByte = nullptr; - ConvertStringToUTF8(lpStr, m_strUTF8_MultiByte); - } - else - m_strUTF8_MultiByte = new char[1](); + operator=(lpStr); } -UTF8MBSTR::UTF8MBSTR(UTF8MBSTR& lpStr) noexcept +UTF8MBSTR::UTF8MBSTR(UTF8MBSTR& lpStr) { - size_t len = Str_LengthUTF8(lpStr.m_strUTF8_MultiByte); - m_strUTF8_MultiByte = new char[len + 1](); - memcpy(m_strUTF8_MultiByte, lpStr.m_strUTF8_MultiByte, len); + m_strUTF8_MultiByte = lpStr.m_strUTF8_MultiByte; } -UTF8MBSTR::~UTF8MBSTR() -{ - if (m_strUTF8_MultiByte) - delete[] m_strUTF8_MultiByte; -} +UTF8MBSTR::~UTF8MBSTR() = default; -void UTF8MBSTR::operator =(lpctstr lpStr) noexcept +void UTF8MBSTR::operator =(lpctstr lpStr) { - if (m_strUTF8_MultiByte) - delete[] m_strUTF8_MultiByte; - if (lpStr) - { - m_strUTF8_MultiByte = nullptr; - ConvertStringToUTF8(lpStr, m_strUTF8_MultiByte); - } + ConvertStringToUTF8(lpStr, &m_strUTF8_MultiByte); else - m_strUTF8_MultiByte = new char[1](); + m_strUTF8_MultiByte.clear(); } void UTF8MBSTR::operator =(UTF8MBSTR& lpStr) noexcept { - if (m_strUTF8_MultiByte) - delete[] m_strUTF8_MultiByte; - - size_t len = Str_LengthUTF8(lpStr.m_strUTF8_MultiByte); - m_strUTF8_MultiByte = new char[len + 1](); - memcpy(m_strUTF8_MultiByte, lpStr.m_strUTF8_MultiByte, len); + m_strUTF8_MultiByte = lpStr.m_strUTF8_MultiByte; } -size_t UTF8MBSTR::ConvertStringToUTF8(lpctstr strIn, char*& strOutUTF8MB) noexcept +size_t UTF8MBSTR::ConvertStringToUTF8(lpctstr strIn, std::vector* strOutUTF8MB) { + ASSERT(strOutUTF8MB); size_t len; #if defined(_WIN32) && defined(UNICODE) // tchar is wchar_t len = wcslen(strIn); #else // tchar is char (UTF8 encoding) - len = Str_LengthUTF8(strIn); + len = strlen(strIn); #endif - strOutUTF8MB = new char[len + 1](); + strOutUTF8MB->resize(len + 1); #if defined(_WIN32) && defined(UNICODE) -#ifdef MSVC_RUNTIME +# ifdef MSVC_RUNTIME size_t aux = 0; - wcstombs_s(&aux, strOutUTF8MB, len + 1, strIn, len); + wcstombs_s(&aux, strOutUTF8MB->data(), len + 1, strIn, len); +# else + wcstombs(strOutUTF8MB->data(), strIn, len); +# endif #else - wcstombs(strOutUTF8MB, strIn, len); -#endif - -#else - memcpy(strOutUTF8MB, strIn, len); + // already utf8 + memcpy(strOutUTF8MB->data(), strIn, len); #endif + (*strOutUTF8MB)[len] = '\0'; return len; } -size_t UTF8MBSTR::ConvertUTF8ToString(const char* strInUTF8MB, lptstr& strOut) noexcept +size_t UTF8MBSTR::ConvertUTF8ToString(const char* strInUTF8MB, std::vector* strOut) { - size_t len = Str_LengthUTF8(strInUTF8MB); - if (!strOut) - strOut = new tchar[len + 1](); + ASSERT(strOut); + size_t len; #if defined(_WIN32) && defined(UNICODE) // tchar is wchar_t + len = Str_UTF8CharCount(strInUTF8MB); + strOut->resize(len + 1); #ifdef MSVC_RUNTIME size_t aux = 0; mbstowcs_s(&aux, strInUTF8MB, len + 1, strInUTF8MB, len); @@ -1791,9 +1768,12 @@ size_t UTF8MBSTR::ConvertUTF8ToString(const char* strInUTF8MB, lptstr& strOut) n #endif #else // tchar is char (UTF8 encoding) - memcpy(strOut, strInUTF8MB, len); + len = strlen(strInUTF8MB); + strOut->resize(len + 1); + memcpy(strOut->data(), strInUTF8MB, len); #endif + (*strOut)[len] = '\0'; return len; } diff --git a/src/common/sphere_library/sstring.h b/src/common/sphere_library/sstring.h index acbfecfeb..271ab0825 100644 --- a/src/common/sphere_library/sstring.h +++ b/src/common/sphere_library/sstring.h @@ -128,7 +128,7 @@ size_t Str_CopyLen(tchar * pDst, lpctstr pSrc) noexcept; * @param pStr UTF8 string. * @return number of characters in the string, excluding the '\0' terminator. */ -size_t Str_LengthUTF8(const char* pStr) noexcept; +size_t Str_UTF8CharCount(const char* pStr) noexcept; /** * @brief Appends pSrc to string pDst of maximum size uiMaxSize. Always '\0' terminates (unless uiMaxSize <= strlen(pDst)). @@ -391,23 +391,27 @@ void CharToMultiByteNonNull(byte* Dest, const char* Src, int MBytes) noexcept; class UTF8MBSTR { public: - UTF8MBSTR() noexcept; - UTF8MBSTR(lpctstr lpStr) noexcept; - UTF8MBSTR(UTF8MBSTR& lpStr) noexcept; + UTF8MBSTR(); + UTF8MBSTR(lpctstr lpStr); + UTF8MBSTR(UTF8MBSTR& lpStr); virtual ~UTF8MBSTR(); - void operator =(lpctstr lpStr) noexcept; + void operator =(lpctstr lpStr); void operator =(UTF8MBSTR& lpStr) noexcept; operator char* () noexcept { - return m_strUTF8_MultiByte; + return m_strUTF8_MultiByte.data(); + } + operator const char* () const noexcept + { + return m_strUTF8_MultiByte.data(); } - static size_t ConvertStringToUTF8(lpctstr strIn, char*& strOutUTF8MB) noexcept; - static size_t ConvertUTF8ToString(const char* strInUTF8MB, lptstr& strOut) noexcept; + static size_t ConvertStringToUTF8(lpctstr strIn, std::vector* strOutUTF8MB); + static size_t ConvertUTF8ToString(const char* strInUTF8MB, std::vector* strOut); private: - char* m_strUTF8_MultiByte; + std::vector m_strUTF8_MultiByte; }; diff --git a/src/common/sqlite/SQLite.cpp b/src/common/sqlite/SQLite.cpp index 2414c39e5..7e2595107 100644 --- a/src/common/sqlite/SQLite.cpp +++ b/src/common/sqlite/SQLite.cpp @@ -58,6 +58,7 @@ bool CSQLite::IsOpen() int CSQLite::QuerySQL( lpctstr strSQL, CVarDefMap & mapQueryResult ) { + ADDTOCALLSTACK("CSQLite::QuerySQL (lpctstr,CVarDefMap)"); mapQueryResult.Clear(); mapQueryResult.SetNumNew("NUMROWS", 0); @@ -67,8 +68,8 @@ int CSQLite::QuerySQL( lpctstr strSQL, CVarDefMap & mapQueryResult ) if (retTable.m_pTable->GetRowCount()) { - int iRows = retTable.m_pTable->GetRowCount(); - int iCols = retTable.m_pTable->GetColCount(); + const int iRows = retTable.m_pTable->GetRowCount(); + const int iCols = retTable.m_pTable->GetColCount(); mapQueryResult.SetNum("NUMROWS", iRows); mapQueryResult.SetNum("NUMCOLS", iCols); @@ -94,80 +95,28 @@ int CSQLite::QuerySQL( lpctstr strSQL, CVarDefMap & mapQueryResult ) SQLiteTable CSQLite::QuerySQL( lpctstr strSQL ) { + ADDTOCALLSTACK("CSQLite::QuerySQL (lpctstr)"); if (!IsOpen()) { m_iLastError=SQLITE_ERROR; return SQLiteTable(); } - char ** retStrings = nullptr; - char * errmsg = nullptr; - int iRows=0, iCols=0; - - int iErr=sqlite3_get_table(m_sqlite3, UTF8MBSTR(strSQL), &retStrings, - &iRows, &iCols, &errmsg); - - if (iErr!=SQLITE_OK) - { - m_iLastError=iErr; - g_Log.Event(LOGM_NOCONTEXT|LOGL_ERROR, "SQLite query \"%s\" failed. Error: %d\n", strSQL, iErr); - } - - sqlite3_free(errmsg); - - SQLiteTable retTable; - - if (iRows>0) - retTable.m_iPos=0; - - retTable.m_iCols=iCols; - retTable.m_iRows=iRows; - retTable.m_strlstCols.reserve(iCols); - - int iPos=0; - - for (; iPosm_iCols=iCols; retTable->m_iRows=iRows; - retTable->m_strlstCols.reserve(iCols); + retTable->m_strlstCols.resize(iCols); int iPos=0; for (; iPosm_strlstCols.emplace_back(stdvstring()); - - if (retStrings[iPos]) - ConvertUTF8ToVString( retStrings[iPos], retTable->m_strlstCols.back() ); + stdvtstring &curColRef = retTable->m_strlstCols[iPos]; + lpctstr curStringPtr = retStrings[iPos]; + if (curStringPtr) + ConvertUTF8ToVString( curStringPtr, &curColRef ); else - retTable->m_strlstCols.back().emplace_back('\0'); + curColRef.emplace_back('\0'); } retTable->m_lstRows.resize(iRows); for (int iRow=0; iRowm_lstRows[iRow].reserve(iCols); + SQLiteTable::row &curRowRef = retTable->m_lstRows[iRow]; + curRowRef.resize(iCols); for (int iCol=0; iColm_lstRows[iRow].emplace_back(stdvstring()); - - if (retStrings[iPos]) - ConvertUTF8ToVString( retStrings[iPos], retTable->m_lstRows[iRow].back() ); + stdvtstring &curColRef = curRowRef[iCol]; + lpctstr curStringPtr = retStrings[iPos]; + if (curStringPtr) + ConvertUTF8ToVString( curStringPtr, &curColRef ); else - retTable->m_lstRows[iRow].back().emplace_back('\0'); + curColRef.emplace_back('\0'); ++iPos; } } + sqlite3_free_table(retStrings); m_iLastError=SQLITE_OK; return SQLiteTablePtr(retTable); } -void CSQLite::ConvertUTF8ToVString( const char * strInUTF8MB, stdvstring & strOut ) +void CSQLite::ConvertUTF8ToVString(const char * strInUTF8MB, stdvtstring *pStrOut ) { - const size_t len = Str_LengthUTF8(strInUTF8MB); - strOut.resize(len + 1, 0); - lptstr ptcStrOut = strOut.data(); - ASSERT(ptcStrOut); - UTF8MBSTR::ConvertUTF8ToString(strInUTF8MB, ptcStrOut); + ADDTOCALLSTACK("CSQLite::ConvertUTF8ToVString"); + ASSERT(pStrOut); + + const size_t len = Str_UTF8CharCount(strInUTF8MB); + pStrOut->resize(len + 1, 0); + UTF8MBSTR::ConvertUTF8ToString(strInUTF8MB, pStrOut); } int CSQLite::ExecuteSQL( lpctstr strSQL ) { + ADDTOCALLSTACK("CSQLite::ExecuteSQL"); if (!IsOpen()) { m_iLastError=SQLITE_ERROR; @@ -258,11 +211,14 @@ int CSQLite::ExecuteSQL( lpctstr strSQL ) int CSQLite::IsSQLComplete( lpctstr strSQL ) { + ADDTOCALLSTACK("CSQLite::IsSQLComplete"); return sqlite3_complete( UTF8MBSTR(strSQL) ); } int CSQLite::ImportDB(lpctstr strInFileName) { + ADDTOCALLSTACK("CSQLite::ImportDB"); + if (!CSFile::FileExists(strInFileName)) return SQLITE_CANTOPEN; //if (IsStrEmpty(strTable)) @@ -327,6 +283,8 @@ int CSQLite::ImportDB(lpctstr strInFileName) int CSQLite::ExportDB(lpctstr strOutFileName) { + ADDTOCALLSTACK("CSQLite::ExportDB"); + int iErr; sqlite3 *out_db; diff --git a/src/common/sqlite/SQLite.h b/src/common/sqlite/SQLite.h index 5386bba5f..474c3c12d 100644 --- a/src/common/sqlite/SQLite.h +++ b/src/common/sqlite/SQLite.h @@ -16,10 +16,8 @@ // Typedefs ////////////////////////////////////////////////////////////////////////// -typedef std::vector stdvstring; -typedef std::vector vstrlist; -typedef vstrlist row; - +typedef std::vector stdvtstring; +typedef std::vector vstrlist; ////////////////////////////////////////////////////////////////////////// // Classes @@ -88,7 +86,7 @@ class CSQLite : public CScriptObj CSString _sFileName; bool _fInMemory; - static void ConvertUTF8ToVString( const char * strInUTF8MB, stdvstring & strOut ); + static void ConvertUTF8ToVString( const char * strInUTF8MB, stdvtstring *pStrOut ); }; // Table class... @@ -159,6 +157,7 @@ class SQLiteTable private: int m_iRows, m_iCols; + typedef vstrlist row; row m_strlstCols; std::vector m_lstRows; From 77618589e86bf032b9d8c333ed201a6868926fc1 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Fri, 18 Oct 2024 11:15:19 +0200 Subject: [PATCH 03/13] Fixed some const-correctness violations, changed every ResDisp method to use the RESDISPLAY_VERSION typedef. Renamed GetBaseID method, since it's doing something different than one would infer from the BASEID script property. --- src/common/CScriptObj.cpp | 4 +- src/common/CTextConsole.cpp | 9 ++++- src/common/CTextConsole.h | 3 +- src/game/CBase.cpp | 4 +- src/game/CBase.h | 12 ++++-- src/game/CObjBase.h | 6 +-- src/game/chars/CChar.cpp | 5 ++- src/game/chars/CChar.h | 12 +++--- src/game/chars/CCharAct.cpp | 12 +++--- src/game/chars/CCharStatus.cpp | 10 ++--- src/game/clients/CAccount.cpp | 6 +-- src/game/clients/CAccount.h | 10 ++--- src/game/clients/CClient.cpp | 8 ++-- src/game/clients/CClient.h | 12 ++++-- src/game/clients/CClientEvent.cpp | 2 +- src/game/clients/CClientMsg.cpp | 2 +- src/game/items/CItem.cpp | 4 +- src/game/items/CItem.h | 2 +- src/game/items/CItemMultiCustom.cpp | 8 ++-- src/network/receive.cpp | 63 +++++++++++++++-------------- 20 files changed, 106 insertions(+), 88 deletions(-) diff --git a/src/common/CScriptObj.cpp b/src/common/CScriptObj.cpp index 73c5d2575..ebd8c4f01 100644 --- a/src/common/CScriptObj.cpp +++ b/src/common/CScriptObj.cpp @@ -1432,7 +1432,7 @@ bool CScriptObj::r_Verb( CScript & s, CTextConsole * pSrc ) // Execute command f pChar->m_Act_UID = g_World.m_uidNew; else { - const CClient *pClient = dynamic_cast(this); + CClient *pClient = dynamic_cast(this); if ( pClient && pClient->GetChar() ) pClient->GetChar()->m_Act_UID = g_World.m_uidNew; } @@ -1465,7 +1465,7 @@ bool CScriptObj::r_Verb( CScript & s, CTextConsole * pSrc ) // Execute command f pCharSrc->m_Act_UID = g_World.m_uidNew; else { - const CClient *pClient = dynamic_cast(this); + CClient *pClient = dynamic_cast(this); if (pClient && pClient->GetChar()) pClient->GetChar()->m_Act_UID = g_World.m_uidNew; } diff --git a/src/common/CTextConsole.cpp b/src/common/CTextConsole.cpp index 89fdbbf8b..5859b6a43 100644 --- a/src/common/CTextConsole.cpp +++ b/src/common/CTextConsole.cpp @@ -9,10 +9,15 @@ extern CSStringList g_AutoComplete; -CChar * CTextConsole::GetChar() const +CChar * CTextConsole::GetChar() { - return const_cast ( dynamic_cast ( this )); + return dynamic_cast ( this ); } +const CChar * CTextConsole::GetChar() const +{ + return dynamic_cast ( this ); +} + int CTextConsole::OnConsoleKey( CSString & sText, tchar nChar, bool fEcho ) { diff --git a/src/common/CTextConsole.h b/src/common/CTextConsole.h index 4bd15f897..47caf232e 100644 --- a/src/common/CTextConsole.h +++ b/src/common/CTextConsole.h @@ -37,7 +37,8 @@ class CTextConsole // What privs do i have ? virtual PLEVEL_TYPE GetPrivLevel() const = 0; virtual lpctstr GetName() const = 0; // ( every object must have at least a type name ) - virtual CChar * GetChar() const; // are we also a CChar ? dynamic_cast ? + virtual CChar * GetChar(); // are we also a CChar ? dynamic_cast ? + virtual const CChar * GetChar() const; virtual void SysMessage( lpctstr pszMessage ) const = 0; // Feed back message. void VSysMessage( lpctstr pszFormat, va_list args ) const; diff --git a/src/game/CBase.cpp b/src/game/CBase.cpp index 52773a54e..15ce568bf 100644 --- a/src/game/CBase.cpp +++ b/src/game/CBase.cpp @@ -379,7 +379,7 @@ bool CBaseBaseDef::r_LoadVal( CScript & s ) m_BaseResources.Load(s.GetArgStr()); return true; case OBC_RESLEVEL: - return( SetResLevel((uchar)(s.GetArgVal())) ); + return( SetResLevel(enum_alias_cast(s.GetArgCVal())) ); case OBC_RESDISPDNHUE: SetResDispDnHue((HUE_TYPE)(s.GetArgVal())); return true; @@ -453,7 +453,7 @@ byte CBaseBaseDef::GetResLevel() const return( m_ResLevel ); } -bool CBaseBaseDef::SetResLevel( byte ResLevel ) +bool CBaseBaseDef::SetResLevel(RESDISPLAY_VERSION ResLevel ) { if ( ResLevel >= RDS_T2A && ResLevel < RDS_QTY ) { diff --git a/src/game/CBase.h b/src/game/CBase.h index 3f635a826..2093e72e6 100644 --- a/src/game/CBase.h +++ b/src/game/CBase.h @@ -105,7 +105,11 @@ struct CBaseBaseDef : public CResourceLink, public CEntityProps // Base type of both CItemBase and CCharBase protected: - dword m_dwDispIndex; // The base artwork id. (may be the same as GetResourceID() in base set.) but can also be "flipped" + // The base artwork id. (may be the same as GetResourceID() in base set.) but can also be "flipped". + // Since it should hold only base/mul/artwork/anim ID, it shouldn't reach values greater than those held by a word (like m_ResDispDnId), + // but we use a dword to ease conversions. + dword m_dwDispIndex; + CSString m_sName; // default type name. (ei, "human" vs specific "Dennis") private: @@ -116,8 +120,8 @@ struct CBaseBaseDef : public CResourceLink, public CEntityProps public: RESDISPLAY_VERSION _iEraLimitProps; // Don't allow to have properties newer than the given era. private: - byte m_Expansion; - byte m_ResLevel; // ResLevel required for players to see me + RESDISPLAY_VERSION m_Expansion; + RESDISPLAY_VERSION m_ResLevel; // ResLevel required for players to see me HUE_TYPE m_ResDispDnHue; // Hue shown to players who don't have my ResLevel word m_ResDispDnId; // ID shown to players who don't have my ResLevel // ------------------------------------- @@ -298,7 +302,7 @@ struct CBaseBaseDef : public CResourceLink, public CEntityProps * @param ResLevel The ResLevel. * @return true if it succeeds, false if it fails. */ - bool SetResLevel( byte ResLevel ); + bool SetResLevel( RESDISPLAY_VERSION ResLevel ); /** * @brief Gets ResDispDNHue. diff --git a/src/game/CObjBase.h b/src/game/CObjBase.h index 857db7cdd..d03a9aa41 100644 --- a/src/game/CObjBase.h +++ b/src/game/CObjBase.h @@ -524,14 +524,14 @@ public: virtual bool IsDeleted() const override; // Accessors /** - * @fn virtual dword CObjBase::GetBaseID() const = 0; + * @fn virtual dword CObjBase::GetIDCommon() const = 0; * - * @brief Gets base identifier. + * @brief Gets base identifier (will NOT be the same as artwork if outside artwork range) * * @return The base identifier. */ - virtual dword GetBaseID() const = 0; + virtual dword GetIDCommon() const = 0; /** * @fn void CObjBase::SetUID( dword dwVal, bool fItem ); diff --git a/src/game/chars/CChar.cpp b/src/game/chars/CChar.cpp index 2504cbed8..693ec86fa 100644 --- a/src/game/chars/CChar.cpp +++ b/src/game/chars/CChar.cpp @@ -1536,7 +1536,7 @@ CREID_TYPE CChar::GetID() const return pCharDef->GetID(); } -dword CChar::GetBaseID() const +dword CChar::GetIDCommon() const { return GetID(); } @@ -2884,7 +2884,8 @@ bool CChar::r_WriteVal( lpctstr ptcKey, CSString & sVal, CTextConsole * pSrc, bo sVal.Clear(); } return true; - case CHC_ID: + case CHC_ID: + // Same as BASEID?? sVal = g_Cfg.ResourceGetName( pCharDef->GetResourceID()); return true; case CHC_ISGM: diff --git a/src/game/chars/CChar.h b/src/game/chars/CChar.h index 35a5edff1..968a5361f 100644 --- a/src/game/chars/CChar.h +++ b/src/game/chars/CChar.h @@ -399,8 +399,8 @@ public: void StatFlag_Mod(uint64 uiStatFlag, bool fMod) noexcept; // Information about us. CREID_TYPE GetID() const; - virtual dword GetBaseID() const override final; - CREID_TYPE GetDispID() const; + virtual dword GetIDCommon() const override final; // The unique index id (will NOT be the same as artwork if outside artwork range). + CREID_TYPE GetDispID() const; bool SetDispID(CREID_TYPE id); void SetID( CREID_TYPE id ); @@ -437,14 +437,14 @@ public: void StatFlag_Mod(uint64 uiStatFlag, bool fMod) noexcept; bool CanHear( const CObjBaseTemplate * pSrc, TALKMODE_TYPE mode ) const; bool CanSeeItem( const CItem * pItem ) const; bool CanTouch( const CPointMap & pt ) const; - bool CanTouch( const CObjBase * pObj ) const; - IT_TYPE CanTouchStatic( CPointMap * pPt, ITEMID_TYPE id, const CItem * pItem ) const; + bool CanTouch( const CObjBase * pObj ); + IT_TYPE CanTouchStatic( CPointMap * pPt, ITEMID_TYPE id, const CItem * pItem ); bool CanMoveItem( const CItem * pItem, bool fMsg = true ) const; byte GetLightLevel() const; - bool CanUse( const CItem * pItem, bool fMoveOrConsume ) const; + bool CanUse( const CItem * pItem, bool fMoveOrConsume ); bool IsMountCapable() const; - ushort Food_CanEat( CObjBase * pObj ) const; + ushort Food_CanEat( CObjBase * pObj ) const; short Food_GetLevelPercent() const; lpctstr Food_GetLevelMessage( bool fPet, bool fHappy ) const; diff --git a/src/game/chars/CCharAct.cpp b/src/game/chars/CCharAct.cpp index d6bf5d4e1..384e4b3c3 100644 --- a/src/game/chars/CCharAct.cpp +++ b/src/game/chars/CCharAct.cpp @@ -3720,8 +3720,8 @@ CItem* CChar::Horse_GetValidMountItem() // Assume pMountItem->m_itFigurine.m_ID is correct? g_Log.EventWarn("Mount (UID=0%x, id=0%x '%s'): Fixed mislinked figurine with UID=ACTARG1=0%x, id=0%x '%s'\n", - (dword)GetUID(), GetBaseID(), GetName(), - (dword)(pMountItem->GetUID()), pMountItem->GetBaseID(), pMountItem->GetName()); + (dword)GetUID(), GetIDCommon(), GetName(), + (dword)(pMountItem->GetUID()), pMountItem->GetIDCommon(), pMountItem->GetName()); } return pMountItem; @@ -3842,8 +3842,8 @@ CItem* CChar::Horse_GetValidMountItem() pMountItem->m_itFigurine.m_ID = GetID(); g_Log.EventWarn("Mount (UID=0%x, id=0%x '%s'): Fixed mount item (mount item UID=ACTARG1=0%x) with UID=0%x, id=0%x '%s'\n", - (dword)GetUID(), GetBaseID(), GetName(), (dword)m_atRidden.m_uidFigurine, - (dword)(pMountItem->GetUID()), pMountItem->GetBaseID(), pMountItem->GetName()); + (dword)GetUID(), GetIDCommon(), GetName(), (dword)m_atRidden.m_uidFigurine, + (dword)(pMountItem->GetUID()), pMountItem->GetIDCommon(), pMountItem->GetName()); lpctstr ptcFixString; switch (iFixCode) @@ -3862,7 +3862,7 @@ CItem* CChar::Horse_GetValidMountItem() if (iFailureCode) { g_Log.EventError("Mount (UID=0%x, id=0%x '%s'): Can't auto-fix invalid mount item (mount item UID=ACTARG1=0%x)'\n", - (dword)GetUID(), GetBaseID(), GetName(), (dword)m_atRidden.m_uidFigurine); + (dword)GetUID(), GetIDCommon(), GetName(), (dword)m_atRidden.m_uidFigurine); lpctstr ptcFailureString; switch (iFailureCode) @@ -4007,7 +4007,7 @@ bool CChar::Horse_UnMount() return false; } - if (pMountItem->GetBaseID() == ITEMID_SHIP_PILOT) + if (pMountItem->GetIDCommon() == ITEMID_SHIP_PILOT) { CItem *pShip = pMountItem->m_uidLink.ItemFind(); if (pShip) diff --git a/src/game/chars/CCharStatus.cpp b/src/game/chars/CCharStatus.cpp index f6cff4e63..80f9c7bb7 100644 --- a/src/game/chars/CCharStatus.cpp +++ b/src/game/chars/CCharStatus.cpp @@ -1332,7 +1332,7 @@ bool CChar::CanTouch( const CPointMap &pt ) const return CanSeeLOS(pt, nullptr, 6); } -bool CChar::CanTouch( const CObjBase *pObj ) const +bool CChar::CanTouch( const CObjBase *pObj ) { ADDTOCALLSTACK("CChar::CanTouch"); // Can I reach this from where i am. swords, spears, arms length = x units. @@ -1442,8 +1442,8 @@ bool CChar::CanTouch( const CObjBase *pObj ) const { // Check if the item is in my bankbox, and i'm not in the same position from which I opened it the last time. const CPointMap& ptTop = GetTopPoint(); - CItemContainer* pBank = GetChar()->GetBank(); - bool fItemContIsInsideBankBox = pBank->IsItemInside(pObj->GetUID().ItemFind()); + CItemContainer* pBank = GetBank(); + const bool fItemContIsInsideBankBox = pBank->IsItemInside(pObj->GetUID().ItemFind()); if (fItemContIsInsideBankBox && (pBank->m_itEqBankBox.m_pntOpen != ptTop)) return false; } @@ -1472,7 +1472,7 @@ bool CChar::CanTouch( const CObjBase *pObj ) const return fCanTouch; } -IT_TYPE CChar::CanTouchStatic( CPointMap *pPt, ITEMID_TYPE id, const CItem *pItem ) const +IT_TYPE CChar::CanTouchStatic( CPointMap *pPt, ITEMID_TYPE id, const CItem *pItem ) { ADDTOCALLSTACK("CChar::CanTouchStatic"); // Might be a dynamic or a static. @@ -1774,7 +1774,7 @@ bool CChar::IsTakeCrime( const CItem *pItem, CChar ** ppCharMark ) const return true; } -bool CChar::CanUse( const CItem *pItem, bool fMoveOrConsume ) const +bool CChar::CanUse( const CItem *pItem, bool fMoveOrConsume ) { ADDTOCALLSTACK("CChar::CanUse"); // Can the Char use ( CONSUME ) the item where it is ? diff --git a/src/game/clients/CAccount.cpp b/src/game/clients/CAccount.cpp index 4d4eca1b0..7e2027853 100644 --- a/src/game/clients/CAccount.cpp +++ b/src/game/clients/CAccount.cpp @@ -1082,7 +1082,7 @@ void CAccount::SetNewPassword( lpctstr pszPassword ) m_sNewPassword.Resize(MAX_ACCOUNT_PASSWORD_ENTER); } -bool CAccount::SetResDisp(byte what) +bool CAccount::SetResDisp(RESDISPLAY_VERSION what) { if (what >= RDS_T2A && what < RDS_QTY) { @@ -1092,7 +1092,7 @@ bool CAccount::SetResDisp(byte what) return false; } -bool CAccount::SetGreaterResDisp(byte what) +bool CAccount::SetGreaterResDisp(RESDISPLAY_VERSION what) { if (what > m_ResDisp) return SetResDisp(what); @@ -1485,7 +1485,7 @@ bool CAccount::r_LoadVal( CScript & s ) } break; case AC_RESDISP: - SetResDisp(s.GetArgBVal()); + SetResDisp(enum_alias_cast(s.GetArgCVal())); break; case AC_TAG0: diff --git a/src/game/clients/CAccount.h b/src/game/clients/CAccount.h index c1e38d5c0..85ac42d09 100644 --- a/src/game/clients/CAccount.h +++ b/src/game/clients/CAccount.h @@ -54,7 +54,7 @@ class CAccount : public CScriptObj word m_PrivFlags; // optional privileges for char (bit-mapped) - byte m_ResDisp; // current CAccount resdisp. + RESDISPLAY_VERSION m_ResDisp; // current CAccount resdisp. byte m_MaxChars; // Max chars allowed for this CAccount. typedef struct { llong m_First; llong m_Last; llong m_vcDelay; } TimeTriesStruct_t; @@ -196,18 +196,18 @@ class CAccount : public CScriptObj * @param what resdisp to set. * @return true on success, false otherwise. */ - bool SetResDisp(byte what); + bool SetResDisp(RESDISPLAY_VERSION what); /** * @brief Gets the current resdisp on this CAccount. * @return The current resdisp. */ - byte GetResDisp() const { return m_ResDisp; } + RESDISPLAY_VERSION GetResDisp() const { return m_ResDisp; } /** * @brief Updates the resdisp if the current is lesser. * @param what the resdisp to update. * @return true if success, false otherwise. */ - bool SetGreaterResDisp(byte what); + bool SetGreaterResDisp(RESDISPLAY_VERSION what); /** * @brief Sets the resdisp on this CAccount based on pClient version. * @return true if success, false otherwise. @@ -218,7 +218,7 @@ class CAccount : public CScriptObj * @param what the resdisp to check. * @return true if the current resdisp is equal to what, false otherwise. */ - bool IsResDisp(byte what) const { return ( m_ResDisp == what ); } + bool IsResDisp(RESDISPLAY_VERSION what) const { return ( m_ResDisp == what ); } /************************************************************************ * Privileges related section. diff --git a/src/game/clients/CClient.cpp b/src/game/clients/CClient.cpp index 7fc23ab34..6b398f771 100644 --- a/src/game/clients/CClient.cpp +++ b/src/game/clients/CClient.cpp @@ -256,11 +256,11 @@ void CClient::ClearPrivFlags(word wPrivFlags) // ------------------------------------------------ -bool CClient::IsResDisp(byte flag) const +bool CClient::IsResDisp(RESDISPLAY_VERSION res) const { if (GetAccount() == nullptr) return false; - return(GetAccount()->IsResDisp(flag)); + return(GetAccount()->IsResDisp(res)); } byte CClient::GetResDisp() const @@ -270,14 +270,14 @@ byte CClient::GetResDisp() const return(GetAccount()->GetResDisp()); } -bool CClient::SetResDisp(byte res) +bool CClient::SetResDisp(RESDISPLAY_VERSION res) { if (GetAccount() == nullptr) return false; return (GetAccount()->SetResDisp(res)); } -bool CClient::SetGreaterResDisp(byte res) +bool CClient::SetGreaterResDisp(RESDISPLAY_VERSION res) { if (GetAccount() == nullptr) return false; diff --git a/src/game/clients/CClient.h b/src/game/clients/CClient.h index d96f64d1b..1a74d0957 100644 --- a/src/game/clients/CClient.h +++ b/src/game/clients/CClient.h @@ -786,10 +786,10 @@ class CClient : public CSObjListRec, public CScriptObj, public CChatChanMember, // ------------------------------------------------ - bool IsResDisp(byte flag) const; + bool IsResDisp(RESDISPLAY_VERSION res) const; byte GetResDisp() const; - bool SetResDisp(byte res); - bool SetGreaterResDisp(byte res); + bool SetResDisp(RESDISPLAY_VERSION res); + bool SetGreaterResDisp(RESDISPLAY_VERSION res); // ------------------------------------------------ @@ -797,10 +797,14 @@ class CClient : public CSObjListRec, public CScriptObj, public CChatChanMember, virtual PLEVEL_TYPE GetPrivLevel() const override; virtual lpctstr GetName() const override; - virtual CChar * GetChar() const override + virtual CChar * GetChar() override { return m_pChar; } + virtual const CChar * GetChar() const override + { + return m_pChar; + } virtual void SysMessage( lpctstr pMsg ) const override; // System message (In lower left corner) bool CanSee( const CObjBaseTemplate * pObj ) const; diff --git a/src/game/clients/CClientEvent.cpp b/src/game/clients/CClientEvent.cpp index 491ec1962..9bdffa0d6 100644 --- a/src/game/clients/CClientEvent.cpp +++ b/src/game/clients/CClientEvent.cpp @@ -115,7 +115,7 @@ void CClient::Event_Item_Dye( CUID uid, HUE_TYPE wHue ) // Rehue an item if ( !pObj->IsChar() ) { CItem *pItem = dynamic_cast(pObj); - if (pItem == nullptr || (( pObj->GetBaseID() != 0xFAB ) && (!pItem->IsType(IT_DYE_VAT) || !IsSetOF(OF_DyeType)))) + if (pItem == nullptr || (( pObj->GetIDCommon() != 0xFAB ) && (!pItem->IsType(IT_DYE_VAT) || !IsSetOF(OF_DyeType)))) return; if ( wHue < HUE_BLUE_LOW ) diff --git a/src/game/clients/CClientMsg.cpp b/src/game/clients/CClientMsg.cpp index d2e6f6611..adfafa2e8 100644 --- a/src/game/clients/CClientMsg.cpp +++ b/src/game/clients/CClientMsg.cpp @@ -36,7 +36,7 @@ void CClient::resendBuffs() const if ( PacketBuff::CanSendTo(GetNetState()) == false ) return; - CChar *pChar = GetChar(); + const CChar *pChar = GetChar(); ASSERT(pChar); // NOTE: If the player logout and login again without close the client, buffs with remaining diff --git a/src/game/items/CItem.cpp b/src/game/items/CItem.cpp index ebf524cd0..5791edb52 100644 --- a/src/game/items/CItem.cpp +++ b/src/game/items/CItem.cpp @@ -2140,7 +2140,7 @@ ITEMID_TYPE CItem::GetID() const return pItemDef->GetID(); } -dword CItem::GetBaseID() const +dword CItem::GetIDCommon() const { return GetID(); } @@ -2516,7 +2516,7 @@ bool CItem::LoadSetContainer(const CUID& uidCont, LAYER_TYPE layer ) } } - DEBUG_ERR(( "Non container uid=0%x,id=0%x\n", (dword)uidCont, pObjCont->GetBaseID() )); + DEBUG_ERR(( "Non container uid=0%x,id=0%x\n", (dword)uidCont, pObjCont->GetIDCommon() )); return false; // not a container. } diff --git a/src/game/items/CItem.h b/src/game/items/CItem.h index 7377fdc5a..c8feb6beb 100644 --- a/src/game/items/CItem.h +++ b/src/game/items/CItem.h @@ -633,7 +633,7 @@ class CItem : public CObjBase CItemBase * Item_GetDef() const noexcept; ITEMID_TYPE GetID() const; - virtual dword GetBaseID() const override final; + virtual dword GetIDCommon() const override final; // The unique index id (will NOT be the same as artwork if outside artwork range). inline ITEMID_TYPE GetDispID() const noexcept { // This is what the item looks like. // May not be the same as the item that defines it's type. diff --git a/src/game/items/CItemMultiCustom.cpp b/src/game/items/CItemMultiCustom.cpp index 3cf453ec2..26c02429d 100644 --- a/src/game/items/CItemMultiCustom.cpp +++ b/src/game/items/CItemMultiCustom.cpp @@ -1796,7 +1796,7 @@ bool CItemMultiCustom::r_WriteVal(lpctstr ptcKey, CSString & sVal, CTextConsole case IMCC_DESIGNER: { - ptcKey += 8; + //ptcKey += 8; CChar * pDesigner = m_pArchitect ? m_pArchitect->GetChar() : nullptr; if (pDesigner != nullptr) sVal.FormatHex(pDesigner->GetUID()); @@ -1807,18 +1807,18 @@ bool CItemMultiCustom::r_WriteVal(lpctstr ptcKey, CSString & sVal, CTextConsole case IMCC_EDITAREA: { - ptcKey += 8; + //ptcKey += 8; CRect rectDesign = GetDesignArea(); sVal.Format("%d,%d,%d,%d", rectDesign.m_left, rectDesign.m_top, rectDesign.m_right, rectDesign.m_bottom); } break; case IMCC_FIXTURES: - ptcKey += 8; + //ptcKey += 8; sVal.FormatSTVal(GetFixtureCount()); break; case IMCC_REVISION: - ptcKey += 8; + //ptcKey += 8; sVal.FormatVal(m_designMain.m_iRevision); break; diff --git a/src/network/receive.cpp b/src/network/receive.cpp index 50e36e308..b8a24d89c 100644 --- a/src/network/receive.cpp +++ b/src/network/receive.cpp @@ -64,32 +64,32 @@ bool PacketCreate::onReceive(CNetState* net) skip(9); // 4=pattern1, 4=pattern2, 1=kuoc readStringASCII(charname, MAX_NAME_SIZE); skip(2); // 0x00 - dword flags = readInt32(); + const dword flags = readInt32(); skip(8); // unk - PROFESSION_TYPE prof = static_cast(readByte()); + const auto prof = static_cast(readByte()); skip(15); // 0x00 - byte race_sex_flag = readByte(); - byte strength = readByte(); - byte dexterity = readByte(); - byte intelligence = readByte(); + const byte race_sex_flag = readByte(); + const byte strength = readByte(); + const byte dexterity = readByte(); + const byte intelligence = readByte(); skill1 = (SKILL_TYPE)readByte(); skillval1 = readByte(); skill2 = (SKILL_TYPE)readByte(); skillval2 = readByte(); skill3 = (SKILL_TYPE)readByte(); skillval3 = readByte(); - HUE_TYPE hue = (HUE_TYPE)(readInt16()); - ITEMID_TYPE hairid = (ITEMID_TYPE)(readInt16()); - HUE_TYPE hairhue = (HUE_TYPE)(readInt16()); - ITEMID_TYPE beardid = (ITEMID_TYPE)(readInt16()); - HUE_TYPE beardhue = (HUE_TYPE)(readInt16()); + const auto hue = (HUE_TYPE)(readInt16()); + const auto hairid = (ITEMID_TYPE)(readInt16()); + const auto hairhue = (HUE_TYPE)(readInt16()); + const auto beardid = (ITEMID_TYPE)(readInt16()); + const auto beardhue = (HUE_TYPE)(readInt16()); skip(1); // shard index - byte startloc = readByte(); + const byte startloc = readByte(); skip(8); // 4=slot, 4=ip - HUE_TYPE shirthue = (HUE_TYPE)(readInt16()); - HUE_TYPE pantshue = (HUE_TYPE)(readInt16()); + const auto shirthue = (HUE_TYPE)(readInt16()); + const auto pantshue = (HUE_TYPE)(readInt16()); - bool isFemale = (race_sex_flag % 2) != 0; // Even=Male, Odd=Female (rule applies to all clients) + const bool isFemale = (race_sex_flag % 2) != 0; // Even=Male, Odd=Female (rule applies to all clients) RACE_TYPE rtRace = RACETYPE_HUMAN; // Human // determine which race the client has selected @@ -133,17 +133,20 @@ bool PacketCreate::onReceive(CNetState* net) } // validate race against resdisp - byte resdisp = net->getClient()->GetAccount() != nullptr? net->getClient()->GetAccount()->GetResDisp() : (byte)RDS_T2A; - if (resdisp < RDS_ML) // prior to ML, only human - { - if (rtRace >= RACETYPE_ELF) - rtRace = RACETYPE_HUMAN; - } - else if (resdisp < RDS_SA) // prior to SA, only human and elf - { - if (rtRace >= RACETYPE_GARGOYLE) - rtRace = RACETYPE_HUMAN; - } + { + const CAccount *pAccount = net->getClient()->GetAccount(); + const RESDISPLAY_VERSION resdisp = pAccount ? pAccount->GetResDisp() : RDS_T2A; + if (resdisp < RDS_ML) // prior to ML, only human + { + if (rtRace >= RACETYPE_ELF) + rtRace = RACETYPE_HUMAN; + } + else if (resdisp < RDS_SA) // prior to SA, only human and elf + { + if (rtRace >= RACETYPE_GARGOYLE) + rtRace = RACETYPE_HUMAN; + } + } return doCreate(net, charname, isFemale, rtRace, strength, dexterity, intelligence, prof, @@ -714,7 +717,7 @@ bool PacketVendorBuyReq::onReceive(CNetState* net) CClient* client = net->getClient(); ASSERT(client); - const CChar* buyer = client->GetChar(); + CChar* buyer = client->GetChar(); if (buyer == nullptr) return false; @@ -842,7 +845,7 @@ bool PacketMapEdit::onReceive(CNetState* net) CClient* client = net->getClient(); ASSERT(client); - const CChar* character = client->GetChar(); + CChar* character = client->GetChar(); if (character == nullptr) return false; @@ -1156,7 +1159,7 @@ bool PacketBulletinBoardReq::onReceive(CNetState* net) CClient* client = net->getClient(); ASSERT(client); - const CChar* character = client->GetChar(); + CChar* character = client->GetChar(); if (character == nullptr) return false; @@ -1848,7 +1851,7 @@ bool PacketVendorSellReq::onReceive(CNetState* net) CClient* client = net->getClient(); ASSERT(client); - const CChar* seller = client->GetChar(); + CChar* seller = client->GetChar(); if (seller == nullptr) return false; From 36ca3bdec47069cda5dfb132961c91bd0ee71abc Mon Sep 17 00:00:00 2001 From: cbnolok Date: Sat, 19 Oct 2024 16:11:04 +0200 Subject: [PATCH 04/13] Reworked worldobjs ticking lists to use vectors. It should be faster and safer than before, also it should fix a possible bug with the previous ticking system. Also, made github workflows run on push on every branch, and added some ignored files. --- .github/workflows/build_linux_x86.yml | 16 +- .github/workflows/build_linux_x86_64.yml | 14 +- .github/workflows/build_osx_arm.yml | 14 +- .github/workflows/build_osx_x86_64.yml | 14 +- .github/workflows/build_win_x86.yml | 14 +- .github/workflows/build_win_x86_64.yml | 14 +- src/common/CException.h | 26 +- src/common/CPointBase.cpp | 2 +- src/common/sphere_library/scontainer_ops.h | 53 + src/game/CObjBase.cpp | 3 +- src/game/CTimedObject.cpp | 1 + src/game/CWorldMap.cpp | 13 +- src/game/CWorldTicker.cpp | 1584 ++++++++++++++------ src/game/CWorldTicker.h | 60 +- src/game/chars/CChar.cpp | 10 +- src/game/chars/CCharNPCAct.cpp | 2 +- src/game/chars/CCharStatus.cpp | 2 +- 17 files changed, 1304 insertions(+), 538 deletions(-) diff --git a/.github/workflows/build_linux_x86.yml b/.github/workflows/build_linux_x86.yml index 5ab5776fc..8b402df46 100644 --- a/.github/workflows/build_linux_x86.yml +++ b/.github/workflows/build_linux_x86.yml @@ -2,12 +2,16 @@ name: Linux x86 on: push: - branches: - - 'master' - - 'main' - - 'dev' - paths-ignore: - - '*.txt' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/.github/workflows/build_linux_x86_64.yml b/.github/workflows/build_linux_x86_64.yml index efe9296ae..1147a056b 100644 --- a/.github/workflows/build_linux_x86_64.yml +++ b/.github/workflows/build_linux_x86_64.yml @@ -2,10 +2,16 @@ name: Linux x86_64 on: push: - branches: - - 'master' - - 'main' - - 'dev' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/.github/workflows/build_osx_arm.yml b/.github/workflows/build_osx_arm.yml index d72618273..b3c4d0c3f 100644 --- a/.github/workflows/build_osx_arm.yml +++ b/.github/workflows/build_osx_arm.yml @@ -2,10 +2,16 @@ name: MacOS ARM on: push: - branches: - - 'master' - - 'main' - - 'dev' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/.github/workflows/build_osx_x86_64.yml b/.github/workflows/build_osx_x86_64.yml index 41a4d4dc5..904aa56c6 100644 --- a/.github/workflows/build_osx_x86_64.yml +++ b/.github/workflows/build_osx_x86_64.yml @@ -2,10 +2,16 @@ name: MacOS x86_64 on: push: - branches: - - 'master' - - 'main' - - 'dev' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/.github/workflows/build_win_x86.yml b/.github/workflows/build_win_x86.yml index 8eb384837..6664b77ed 100644 --- a/.github/workflows/build_win_x86.yml +++ b/.github/workflows/build_win_x86.yml @@ -2,10 +2,16 @@ name: Windows x86 on: push: - branches: - - 'master' - - 'main' - - 'dev' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/.github/workflows/build_win_x86_64.yml b/.github/workflows/build_win_x86_64.yml index 41dac07d8..ee74649de 100644 --- a/.github/workflows/build_win_x86_64.yml +++ b/.github/workflows/build_win_x86_64.yml @@ -2,10 +2,16 @@ name: Windows x86_64 on: push: - branches: - - 'master' - - 'main' - - 'dev' +# branches: +# - 'master' +# - 'main' +# - 'dev' + paths-ignore: + - '.gitignore' + - '.gitattributes' + - '**.txt' + - '**.md' + - '**.rc' pull_request: branches: - 'master' diff --git a/src/common/CException.h b/src/common/CException.h index 4a121107f..1f6156d2a 100644 --- a/src/common/CException.h +++ b/src/common/CException.h @@ -138,9 +138,9 @@ class CAssert : public CSError /*--- Main (non SUB) macros ---*/ // EXC_TRY -#define EXC_TRY(a) \ +#define EXC_TRY(ptcMethodName) \ lpctstr inLocalBlock = ""; \ - lpctstr inLocalArgs = a; \ + lpctstr inLocalArgs = ptcMethodName; \ uint inLocalBlockCnt = 0; /* NOLINT(misc-const-correctness) */ \ bool fCATCHExcept = false; \ UnreferencedParameter(fCATCHExcept); \ @@ -148,8 +148,8 @@ class CAssert : public CSError { // EXC_SET_BLOCK -#define EXC_SET_BLOCK(a) \ - inLocalBlock = a; \ +#define EXC_SET_BLOCK(ptcBlockName) \ + inLocalBlock = ptcBlockName; \ ++inLocalBlockCnt // _EXC_CATCH_EXCEPTION_GENERIC (used inside other macros! don't use it manually!) @@ -217,9 +217,9 @@ class CAssert : public CSError /*--- SUB macros ---*/ // EXC_TRYSUB -#define EXC_TRYSUB(a) \ +#define EXC_TRYSUB(ptcMethodName) \ lpctstr inLocalSubBlock = ""; \ - lpctstr inLocalSubArgs = a; \ + lpctstr inLocalSubArgs = ptcMethodName; \ uint inLocalSubBlockCnt = 0; \ bool fCATCHExceptSub = false; \ UnreferencedParameter(fCATCHExceptSub); \ @@ -227,8 +227,8 @@ class CAssert : public CSError { // EXC_SETSUB_BLOCK -#define EXC_SETSUB_BLOCK(a) \ - inLocalSubBlock = a; \ +#define EXC_SETSUB_BLOCK(ptcBlockName) \ + inLocalSubBlock = ptcBlockName; \ ++inLocalSubBlockCnt // _EXC_CATCH_SUB_EXCEPTION_GENERIC(a,b, "ExceptionType") (used inside other macros! don't use it manually!) @@ -252,28 +252,28 @@ class CAssert : public CSError _EXC_CAUGHT // EXC_CATCHSUB(a) -#define EXC_CATCHSUB(a) \ +#define EXC_CATCHSUB(ptcCatchContext) \ } \ catch ( const CAssert& e ) \ { \ - _EXC_CATCH_SUB_EXCEPTION_GENERIC(&e, a, "CAssert"); \ + _EXC_CATCH_SUB_EXCEPTION_GENERIC(&e, ptcCatchContext, "CAssert"); \ GetCurrentProfileData().Count(PROFILE_STAT_FAULTS, 1); \ } \ catch ( const CSError& e ) \ { \ - _EXC_CATCH_SUB_EXCEPTION_GENERIC(&e, a, "CSError"); \ + _EXC_CATCH_SUB_EXCEPTION_GENERIC(&e, ptcCatchContext, "CSError"); \ GetCurrentProfileData().Count(PROFILE_STAT_FAULTS, 1); \ EXC_NOTIFY_DEBUGGER; \ } \ catch ( const std::exception& e ) \ { \ - _EXC_CATCH_SUB_EXCEPTION_STD(&e, a); \ + _EXC_CATCH_SUB_EXCEPTION_STD(&e, ptcCatchContext); \ GetCurrentProfileData().Count(PROFILE_STAT_FAULTS, 1); \ EXC_NOTIFY_DEBUGGER; \ } \ catch (...) \ { \ - _EXC_CATCH_SUB_EXCEPTION_GENERIC(nullptr, a, "pure"); \ + _EXC_CATCH_SUB_EXCEPTION_GENERIC(nullptr, ptcCatchContext, "pure"); \ GetCurrentProfileData().Count(PROFILE_STAT_FAULTS, 1); \ EXC_NOTIFY_DEBUGGER; \ } diff --git a/src/common/CPointBase.cpp b/src/common/CPointBase.cpp index 7f969a217..194725ee3 100644 --- a/src/common/CPointBase.cpp +++ b/src/common/CPointBase.cpp @@ -34,7 +34,7 @@ DIR_TYPE GetDirTurn( DIR_TYPE dir, int offset ) lpctstr CPointBase::sm_szDirs[] {"\0"}; void CPointBase::InitRuntimeDefaultValues() { - AssignInitlistToCSizedArray( + sl::AssignInitlistToCSizedArray( CPointBase::sm_szDirs, ARRAY_COUNT(CPointBase::sm_szDirs), { g_Cfg.GetDefaultMsg(DEFMSG_MAP_DIR_0), diff --git a/src/common/sphere_library/scontainer_ops.h b/src/common/sphere_library/scontainer_ops.h index 08ad2eaa7..e2251cbc2 100644 --- a/src/common/sphere_library/scontainer_ops.h +++ b/src/common/sphere_library/scontainer_ops.h @@ -4,6 +4,24 @@ #include #include +namespace sl +{ + +// Define a concept for STL containers +template +concept ConceptBasicContainer = requires(T t) +{ + // Type must have an iterator type + typename T::iterator; + + // Must have a begin() method returning an iterator + { t.begin() } -> std::same_as; + + // Must have an end() method returning an iterator + { t.end() } -> std::same_as; +}; + + template void AssignInitlistToCSizedArray(outT* dest, const size_t dest_size, std::initializer_list initializer) noexcept { @@ -17,4 +35,39 @@ void AssignInitlistToCSizedArray(outT* dest, const size_t dest_size, std::initia } } +template +bool ContainerIsSorted(T const& cont) +{ + return std::is_sorted(cont.begin(), cont.end()); +} + +template +bool SortedContainerHasDuplicates(T const& cont) +{ + return std::adjacent_find(cont.begin(), cont.end()) != cont.end(); +} + +/* +template +bool SortedContainerHasDuplicatesPred(ContT const& cont, PredT const& predicate) +{ + return std::adjacent_find(cont.begin(), cont.end(), predicate) == cont.end(); +} +*/ + +template +bool UnsortedContainerHasDuplicates(const T& cont) noexcept +{ + for (size_t i = 0; i < cont.size(); ++i) { + for (size_t j = i + 1; j < cont.size(); ++j) { + if ((i != j) && (cont[i] == cont[j])) { + return true; // Found a duplicate + } + } + } + return false; // No duplicates found +} + +} // namespace sl + #endif // INC_CONTAINER_OPS_H diff --git a/src/game/CObjBase.cpp b/src/game/CObjBase.cpp index 7cef3e945..d4360268b 100644 --- a/src/game/CObjBase.cpp +++ b/src/game/CObjBase.cpp @@ -225,7 +225,7 @@ void CObjBase::DeletePrepare() RemoveSelf(); if (fTopLevel) - _uiInternalStateFlags |= SF_TOPLEVEL; + _uiInternalStateFlags |= SF_TOPLEVEL; } void CObjBase::DeleteCleanup(bool fForce) @@ -765,6 +765,7 @@ bool CObjBase::MoveNear(CPointMap pt, ushort iSteps ) // Move to nearby this other object. // Actually move it within +/- iSteps + // TODO: WUT?? CPointMap ptOld(pt); for ( uint i = 0; i < iSteps; ++i ) { diff --git a/src/game/CTimedObject.cpp b/src/game/CTimedObject.cpp index ae4a6b771..94d2f8fca 100644 --- a/src/game/CTimedObject.cpp +++ b/src/game/CTimedObject.cpp @@ -62,6 +62,7 @@ void CTimedObject::_SetTimeout(int64 iDelayInMsecs) { ADDTOCALLSTACK_DEBUG("CTimedObject::_SetTimeout"); // Assume we have the mutex already locked here + g_Log.EventDebug("[%p] _SetTimeout to %" PRId64 ".\n", (void*)this, iDelayInMsecs); const ProfileTask timersTask(PROFILE_TIMERS); // profile the settimeout proccess. if (_IsDeleted()) //prevent deleted objects from setting new timers to avoid nullptr calls diff --git a/src/game/CWorldMap.cpp b/src/game/CWorldMap.cpp index 3ca2e35ac..365146c45 100644 --- a/src/game/CWorldMap.cpp +++ b/src/game/CWorldMap.cpp @@ -332,9 +332,8 @@ CPointMap CWorldMap::FindTypeNear_Top( const CPointMap & pt, IT_TYPE iType, int const CItemBase * pItemDef = nullptr; const CItemBaseDupe * pDupeDef = nullptr; CItem * pItem = nullptr; - height_t Height = 0; - byte z = 0; - CPointMap ptTest; + height_t Height = 0; + CPointMap ptTest; uint iRetElem = 4; @@ -351,9 +350,9 @@ CPointMap CWorldMap::FindTypeNear_Top( const CPointMap & pt, IT_TYPE iType, int Area->SetAllShow( true ); for (;;) { - z = 0; + schar z = 0; Height = 0; - pItem = Area->GetItem(); + pItem = Area->GetItem(); if ( pItem == nullptr ) break; @@ -361,14 +360,14 @@ CPointMap CWorldMap::FindTypeNear_Top( const CPointMap & pt, IT_TYPE iType, int if ( pt.GetDist( ptItemTop ) > iDistance ) continue; - pItemDef = CItemBase::FindItemBase( pItem->GetDispID() ); + pItemDef = CItemBase::FindItemBase( pItem->GetDispID() ); if ( pItemDef == nullptr ) continue; Height = pItemDef->GetHeight(); if ( pItemDef->GetID() != pItem->GetDispID() ) //not a parent item { - pDupeDef = CItemBaseDupe::GetDupeRef((ITEMID_TYPE)(pItem->GetDispID())); + pDupeDef = CItemBaseDupe::GetDupeRef((ITEMID_TYPE)(pItem->GetDispID())); if ( ! pDupeDef ) { g_Log.EventDebug("Failed to get non-parent reference (dynamic) (DispID 0%x) (X: %d Y: %d Z: %d)\n",pItem->GetDispID(),ptTest.m_x,ptTest.m_y,ptTest.m_z); diff --git a/src/game/CWorldTicker.cpp b/src/game/CWorldTicker.cpp index 3efc97708..82c5adf8d 100644 --- a/src/game/CWorldTicker.cpp +++ b/src/game/CWorldTicker.cpp @@ -1,3 +1,4 @@ +#include "../common/sphere_library/scontainer_ops.h" #include "../common/CException.h" #include "../sphere/threads.h" #include "../sphere/ProfileTask.h" @@ -8,6 +9,17 @@ #include "CWorldGameTime.h" #include "CWorldTicker.h" +#ifdef _DEBUG +# define DEBUG_CTIMEDOBJ_TIMED_TICKING +# define DEBUG_CCHAR_PERIODIC_TICKING +//# define DEBUG_STATUSUPDATES +# define DEBUG_LIST_OPS + +# define DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE +# define DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + +//# define BENCHMARK_LISTS // TODO +#endif CWorldTicker::CWorldTicker(CWorldClock *pClock) { @@ -16,9 +28,9 @@ CWorldTicker::CWorldTicker(CWorldClock *pClock) _iLastTickDone = 0; - _vecObjs.reserve(50); - _vecWorldObjsToEraseFromList.reserve(50); - _vecPeriodicCharsToEraseFromList.reserve(25); + _vecGenericObjsToTick.reserve(50); + _vecWorldObjsEraseRequests.reserve(50); + _vecPeriodicCharsEraseRequests.reserve(25); } @@ -26,81 +38,192 @@ CWorldTicker::CWorldTicker(CWorldClock *pClock) void CWorldTicker::_InsertTimedObject(const int64 iTimeout, CTimedObject* pTimedObject) { + ASSERT(pTimedObject); ASSERT(iTimeout != 0); -/* -#ifdef _DEBUG - for (auto& elemList : _mWorldTickList) +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + ASSERT(sl::ContainerIsSorted(_mWorldTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); +#endif + + const auto fnFindEntryByObj = [pTimedObject](TickingTimedObjEntry const& rhs) noexcept { + return pTimedObject == rhs.second; + }; +#if MT_ENGINES + std::unique_lock lock(_mWorldTickList.MT_CMUTEX); +#endif + + const auto itEntryInAddList = std::find_if( + _vecWorldObjsAddRequests.begin(), + _vecWorldObjsAddRequests.end(), + fnFindEntryByObj); + if (_vecWorldObjsAddRequests.end() != itEntryInAddList) { - for (auto& elem : elemList.second) +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: CTimedObj insertion into ticking list already requested with %s timer. OVERWRITING.\n", + (void*)pTimedObject, + ((itEntryInAddList->first == iTimeout) ? "same" : "different")); +#endif + itEntryInAddList->first = iTimeout; + return; // Already requested the addition. + } + + /* + const auto itFoundEraseRequest = std::find( + _vecWorldObjsEraseRequested.begin(), + _vecWorldObjsEraseRequested.end(), + pTimedObject); + if (_vecWorldObjsEraseRequested.end() != itFoundEraseRequest) + { +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[%p] WARN: Stopped attempt of inserting a CTimedObj which removal from ticking list has been requested before!\n", (void*)pTimedObject); +#endif + return; // Already requested the addition. + }*/ + + const auto itEntryInTickList = std::find_if( + _mWorldTickList.begin(), + _mWorldTickList.end(), + fnFindEntryByObj); + if (_mWorldTickList.end() != itEntryInTickList) + { + if (itEntryInTickList->first == iTimeout) + { +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested insertion of a CTimedObj in the main ticking list with the same timeout, skipping.\n", (void*)pTimedObject); +#endif + return; + } + +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested insertion of a CTimedObj already in the main ticking list.\n", (void*)pTimedObject); +# endif + + const auto itEntryInEraseList = std::find( + _vecWorldObjsEraseRequests.begin(), + _vecWorldObjsEraseRequests.end(), + pTimedObject); + if (_vecWorldObjsEraseRequests.end() == itEntryInEraseList) { - ASSERT(elem != pTimedObject); +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: But i didn't even requested to remove that!\n", (void*)pTimedObject); +# endif + + ASSERT(false); + return; } + +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: But that's fine, because i already have requested to remove that!\n", (void*)pTimedObject); +# endif +#endif + } +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + else + { + const auto itEntryInEraseList = std::find( + _vecWorldObjsEraseRequests.begin(), + _vecWorldObjsEraseRequests.end(), + pTimedObject); + ASSERT(_vecWorldObjsEraseRequests.end() == itEntryInEraseList); } #endif -*/ -#if MT_ENGINES - std::unique_lock lock(_mWorldTickList.MT_CMUTEX); + _vecWorldObjsAddRequests.emplace_back(iTimeout, pTimedObject); + +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Done adding CTimedObj in the ticking list add buffer with timeout %" PRId64 ".\n", (void*)pTimedObject, iTimeout); #endif - _mWorldTickList.emplace(iTimeout, pTimedObject); } -void CWorldTicker::_RemoveTimedObject(const int64 iOldTimeout, CTimedObject* pTimedObject) +void CWorldTicker::_RemoveTimedObject(CTimedObject* pTimedObject) { - ASSERT(iOldTimeout != 0); +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mWorldTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); +#endif - //g_Log.EventDebug("Trying to erase TimedObject 0x%p with old timeout %ld.\n", pTimedObject, iOldTimeout); + const auto fnFindEntryByObj = [pTimedObject](TickingTimedObjEntry const& rhs) noexcept { + return pTimedObject == rhs.second; + }; #if MT_ENGINES std::unique_lock lock(_mWorldTickList.MT_CMUTEX); #endif - const auto itMap = _mWorldTickList.equal_range(iOldTimeout); - decltype(_mWorldTickList)::const_iterator itFound = itMap.second; // first element greater than the key we look for - for (auto it = itMap.first; it != itMap.second; ++it) + + // TODO: use binary search here? + const auto itEntryInTickList = std::find_if( + _mWorldTickList.begin(), + _mWorldTickList.end(), + fnFindEntryByObj); + + const auto itEntryInAddList = std::find_if( + _vecWorldObjsAddRequests.begin(), + _vecWorldObjsAddRequests.end(), + fnFindEntryByObj); + + const auto itEntryInRemoveList = std::find( + _vecWorldObjsEraseRequests.begin(), + _vecWorldObjsEraseRequests.end(), + pTimedObject); + + if (itEntryInAddList != _vecWorldObjsAddRequests.end()) { - // I have a pair of iterators for a range of the elements (all the elements with the same key) - if (it->second == pTimedObject) - { - if (itFound != itMap.second) - { - g_Log.EventDebug("The same TimedObject is inserted multiple times in mWorldTickList. This shouldn't happen. Removing only the first one.\n"); - } - itFound = it; -#if !defined(_DEBUG) - break; -#endif + _vecWorldObjsAddRequests.erase(itEntryInAddList); + +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Removing CTimedObj from the ticking list add buffer.\n", (void*)pTimedObject); + + if (itEntryInRemoveList == _vecWorldObjsEraseRequests.end()) { + ASSERT(itEntryInTickList == _mWorldTickList.end()); + } + else if (itEntryInRemoveList != _vecWorldObjsEraseRequests.end()) { + ASSERT(itEntryInTickList != _mWorldTickList.end()); } +# endif +#endif + + return; } - if (itFound == itMap.second) + + if (_vecWorldObjsEraseRequests.end() != itEntryInRemoveList) { - // Not found. The object might have a timeout while being in a non-tickable state, so it isn't in the list. -/* -#ifdef _DEBUG - g_Log.EventDebug("Requested erasure of TimedObject in mWorldTickList, but it wasn't found.\n"); + // I have already requested to remove this from the main ticking list. +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: CTimedObj removal from the main ticking list already requested.\n", (void*)pTimedObject); +# endif + ASSERT(itEntryInAddList == _vecWorldObjsAddRequests.end()); #endif -*/ + return; // Already requested the removal. + } -/* -#ifdef _DEBUG - for (auto& elemList : _mWorldTickList) - { - for (auto& elem : elemList.second) - { - ASSERT(elem != pTimedObject); - } - } + if (itEntryInTickList == _mWorldTickList.end()) + { + // Not found. The object might have a timeout while being in a non-tickable state, so it isn't in the list. +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested erasure of TimedObject in mWorldTickList, but it wasn't found.\n", (void*)pTimedObject); #endif -*/ return; } - _mWorldTickList.erase(itFound); + + _vecWorldObjsEraseRequests.emplace_back(pTimedObject); + +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Done adding CTimedObj to the ticking list remove buffer.\n", (void*)pTimedObject); +#endif } void CWorldTicker::AddTimedObject(const int64 iTimeout, CTimedObject* pTimedObject, bool fForce) { - //if (iTimeout < CWorldGameTime::GetCurrentTime().GetTimeRaw()) // We do that to get them tick as sooner as possible + //if (iTimeout < CWorldGameTime::GetCurrentTime().GetTimeRaw()) // We do that to get them tick as sooner as possible; don't uncomment. // return; +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Trying to add CTimedObj to the ticking list.\n", (void*)pTimedObject); +#endif + EXC_TRY("AddTimedObject"); const ProfileTask timersTask(PROFILE_TIMERS); @@ -110,7 +233,7 @@ void CWorldTicker::AddTimedObject(const int64 iTimeout, CTimedObject* pTimedObje { // Adding an object already on the list? Am i setting a new timeout without deleting the previous one? EXC_SET_BLOCK("Remove"); - _RemoveTimedObject(iTickOld, pTimedObject); + _RemoveTimedObject(pTimedObject); } EXC_SET_BLOCK("Insert"); @@ -138,22 +261,68 @@ void CWorldTicker::AddTimedObject(const int64 iTimeout, CTimedObject* pTimedObje { _InsertTimedObject(iTimeout, pTimedObject); } +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + else + g_Log.EventDebug("[WorldTicker][%p] INFO: Cannot tick and no force, so i'm not adding CTimedObj to the ticking list.\n", (void*)pTimedObject); +#endif EXC_CATCH; } void CWorldTicker::DelTimedObject(CTimedObject* pTimedObject) { +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Trying to remove CTimedObj from the ticking list.\n", (void*)pTimedObject); +#endif + EXC_TRY("DelTimedObject"); const ProfileTask timersTask(PROFILE_TIMERS); EXC_SET_BLOCK("Not ticking?"); const int64 iTickOld = pTimedObject->_GetTimeoutRaw(); if (iTickOld == 0) + { +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested deletion of CTimedObj, but Timeout is 0, so it shouldn't be in the list, or just queued to be removed.\n", (void*)pTimedObject); +# endif + + const auto itEntryInRemoveList = std::find( + _vecWorldObjsEraseRequests.begin(), + _vecWorldObjsEraseRequests.end(), + pTimedObject); + if (itEntryInRemoveList != _vecWorldObjsEraseRequests.end()) + { +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: though, found it already in the removal list, so it's fine..\n", (void*)pTimedObject); +# endif + + //ASSERT(false); + return; + } + + const auto itTickList = std::find_if( + _mWorldTickList.begin(), + _mWorldTickList.end(), + [pTimedObject](const TickingTimedObjEntry& entry) { + return entry.second == pTimedObject; + }); + if (itTickList != _mWorldTickList.end()) { +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: But i have found it in the list! With Timeout %" PRId64 ".\n", (void*)pTimedObject, itTickList->first); +# endif + ASSERT(false); + } +# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + else + g_Log.EventDebug("[WorldTicker][%p] WARN: (rightfully) i haven't found it in the list.\n", (void*)pTimedObject); +# endif +#endif return; + } EXC_SET_BLOCK("Remove"); - _RemoveTimedObject(iTickOld, pTimedObject); + _RemoveTimedObject(pTimedObject); EXC_CATCH; } @@ -163,46 +332,161 @@ void CWorldTicker::DelTimedObject(CTimedObject* pTimedObject) void CWorldTicker::_InsertCharTicking(const int64 iTickNext, CChar* pChar) { + ASSERT(iTickNext != 0); + ASSERT(pChar); + #if MT_ENGINES std::unique_lock lock(_mCharTickList.MT_CMUTEX); #endif - _mCharTickList.emplace(iTickNext, pChar); +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + const auto fnFindEntryByChar = [pChar](TickingPeriodicCharEntry const& rhs) noexcept { + return pChar == rhs.second; + }; + + const auto itEntryInAddList = std::find_if( + _vecPeriodicCharsAddRequests.begin(), + _vecPeriodicCharsAddRequests.end(), + fnFindEntryByChar); + if (_vecPeriodicCharsAddRequests.end() != itEntryInAddList) + { +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Periodic char insertion into ticking list already requested " + "(requesting tick %" PRId64 ", previous requested tick %" PRId64 ").\n", + (void*)pChar, iTickNext, itEntryInAddList->first); +#endif + + ASSERT(false); + return; // Already requested the addition. + } + + const auto itEntryInEraseList = std::find( + _vecPeriodicCharsEraseRequests.begin(), + _vecPeriodicCharsEraseRequests.end(), + pChar); + if (_vecPeriodicCharsEraseRequests.end() != itEntryInEraseList) + { +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Stopped insertion attempt of a CChar which removal from periodic ticking list has been requested!\n", (void*)pChar); +# endif + + ASSERT(false); + return; // Already requested the removal. + } + + const auto itEntryInTickList = std::find_if( + _mCharTickList.begin(), + _mCharTickList.end(), + fnFindEntryByChar); + ASSERT(_mCharTickList.end() == itEntryInTickList); +#endif + + _vecPeriodicCharsAddRequests.emplace_back(iTickNext, pChar); pChar->_iTimePeriodicTick = iTickNext; + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Done adding the CChar to the periodic ticking list add buffer.\n", (void*)pChar); +#endif } -void CWorldTicker::_RemoveCharTicking(const int64 iOldTimeout, CChar* pChar) +bool CWorldTicker::_RemoveCharTicking(CChar* pChar) { // I'm reasonably sure that the element i'm trying to remove is present in this container. + ASSERT(pChar); + #if MT_ENGINES std::unique_lock lock(_mCharTickList.MT_CMUTEX); #endif - const auto itMap = _mCharTickList.equal_range(iOldTimeout); - decltype(_mCharTickList)::const_iterator itFound = itMap.second; // first element greater than the key we look for - for (auto it = itMap.first; it != itMap.second; ++it) + const auto fnFindEntryByChar = [pChar](TickingPeriodicCharEntry const& rhs) noexcept { + return pChar == rhs.second; + }; + + const auto itEntryInTickList = std::find_if( + _mCharTickList.begin(), + _mCharTickList.end(), + fnFindEntryByChar); + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + const auto itEntryInRemoveList = std::find( + _vecPeriodicCharsEraseRequests.begin(), + _vecPeriodicCharsEraseRequests.end(), + pChar); + if (_vecPeriodicCharsEraseRequests.end() != itEntryInRemoveList) { - // I have a pair of iterators for a range of the elements (all the elements with the same key) - if (it->second == pChar) - { - if (itFound != itMap.second) - { - g_Log.EventDebug("The same CChar is inserted multiple times in mCharTickList. This shouldn't happen. Removing only the first one.\n"); - } - itFound = it; -#if !_DEBUG - break; +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: TickingPeriodicChar erasure from ticking list already requested.\n", (void*)pChar); +# endif + + ASSERT(false); + return false; // Already requested the removal. + } #endif + + const auto itEntryInAddList = std::find_if( + _vecPeriodicCharsAddRequests.begin(), + _vecPeriodicCharsAddRequests.end(), + fnFindEntryByChar); + if (_vecPeriodicCharsAddRequests.end() != itEntryInAddList) + { +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Erasing TickingPeriodicChar from periodic char ticking list add buffer.\n", (void*)pChar); +#endif + + _vecPeriodicCharsAddRequests.erase(itEntryInAddList); + pChar->_iTimePeriodicTick = 0; + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + if (itEntryInRemoveList == _vecPeriodicCharsEraseRequests.end()) { + ASSERT(itEntryInTickList == _mCharTickList.end()); + } + else { + ASSERT(itEntryInTickList != _mCharTickList.end()); } +#endif + return true; } - if (itFound == itMap.second) + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + // Check if it's in the ticking list. + if (itEntryInTickList == _mCharTickList.end()) { - return; +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested TickingPeriodicChar removal from ticking list, but not found.\n", (void*)pChar); +# endif + + ASSERT(false); + return false; } - _mCharTickList.erase(itFound); + if (_vecPeriodicCharsEraseRequests.end() != itEntryInRemoveList) + { + // I have already requested to remove this from the main ticking list. +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: TickingPeriodicChar removal from the main ticking list already requested.\n", (void*)pChar); +# endif + ASSERT(itEntryInAddList == _vecPeriodicCharsAddRequests.end()); + return false; // Already requested the removal. + } +#endif + + if (itEntryInTickList == _mCharTickList.end()) + { + // Not found. The object might have a timeout while being in a non-tickable state, so it isn't in the list. +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested erasure of TimedObject in mWorldTickList, but it wasn't found.\n", (void*)pChar); +#endif + return false; + } + + _vecPeriodicCharsEraseRequests.emplace_back(pChar); pChar->_iTimePeriodicTick = 0; + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Done adding the CChar to the periodic ticking list remove buffer.\n", (void*)pChar); +#endif + return true; } void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) @@ -226,8 +510,13 @@ void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) iTickOld = pChar->_iTimePeriodicTick; } - //if (iTickNext == iTickOld) - //{ +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Trying to add CChar to the periodic ticking list. Tickold: %" PRId64 ". Ticknext: %" PRId64 ".\n", + (void*)pChar, iTickOld, iTickNext); +#endif + + if (iTickNext == iTickOld) + { /* #ifdef _DEBUG auto it = std::find_if(_mCharTickList.begin(), _mCharTickList.end(), @@ -237,8 +526,11 @@ void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) DEBUG_ASSERT(it == _mCharTickList.end()); #endif */ - // return; - //} +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Stop, tickold == ticknext.\n", (void*)pChar); +#endif + return; + } //if (iTickNext < CWorldGameTime::GetCurrentTime().GetTimeRaw()) // We do that to get them tick as sooner as possible // return; @@ -247,20 +539,13 @@ void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) { // Adding an object already on the list? Am i setting a new timeout without deleting the previous one? EXC_SET_BLOCK("Remove"); - _RemoveCharTicking(iTickOld, pChar); - } + const bool fRet = _RemoveCharTicking(pChar); + UnreferencedParameter(fRet); -/* -#ifdef _DEBUG - for (auto& elemList : _mCharTickList) - { - for (auto& elemChar : elemList.second) - { - ASSERT(elemChar != pChar); - } - } +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + ASSERT(fRet); #endif -*/ + } EXC_SET_BLOCK("Insert"); _InsertCharTicking(iTickNext, pChar); @@ -285,25 +570,98 @@ void CWorldTicker::DelCharTicking(CChar* pChar, bool fNeedsLock) { iTickOld = pChar->_iTimePeriodicTick; } + +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: Trying to remove CChar from the periodic ticking list. Tickold: %" PRId64 ".\n", + (void*)pChar, iTickOld); +#endif + if (iTickOld == 0) + { +#ifdef DEBUG_CCHAR_PERIODIC_TICKING +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: Requested deletion of Periodic char, but Timeout is 0. It shouldn't be in the list, or just queued to be removed.\n", (void*)pChar); +# endif + auto fnFindEntryByChar = [pChar](const TickingPeriodicCharEntry& entry) noexcept { + return entry.second == pChar; + }; + + const auto itEntryInEraseList = std::find( + _vecPeriodicCharsEraseRequests.begin(), + _vecPeriodicCharsEraseRequests.end(), + pChar); + if (itEntryInEraseList != _vecPeriodicCharsEraseRequests.end()) + { +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: though, found it already in the removal list, so it's fine..\n", (void*)pChar); +# endif + + //ASSERT(false); + return; + } + + const auto itEntryInTickList = std::find_if( + _mCharTickList.begin(), + _mCharTickList.end(), + fnFindEntryByChar); + if (itEntryInTickList != _mCharTickList.end()) + { +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: But i have found it in the list! With Timeout %" PRId64 ".\n", (void*)pChar, itEntryInTickList->first); +# endif + + ASSERT(false); + return; + } + +# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] WARN: (rightfully) i haven't found it in any list.\n", (void*)pChar); +# endif +#endif return; + } EXC_SET_BLOCK("Remove"); - _RemoveCharTicking(iTickOld, pChar); + _RemoveCharTicking(pChar); EXC_CATCH; } void CWorldTicker::AddObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static { +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] INFO: Trying to add CObjBase to the status update list.\n", (void*)pObj); +#endif EXC_TRY("AddObjStatusUpdate"); + const ProfileTask timersTask(PROFILE_TIMERS); UnreferencedParameter(fNeedsLock); { #if MT_ENGINES std::unique_lock lock(_ObjStatusUpdates.MT_CMUTEX); #endif - _ObjStatusUpdates.insert(pObj); + + // Here i don't need to use an "add" buffer, like with CTimedObj, because this container isn't meant + // to be ordered and i can just push back stuff. + const auto itDuplicate = std::find( + _ObjStatusUpdates.begin(), + _ObjStatusUpdates.end(), + pObj + ); + if (_ObjStatusUpdates.end() != itDuplicate) + { +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] WARN: Trying to add status update for duplicate CObjBase.\n", (void*)pObj); + ASSERT(false); +#endif + return; + } + + _ObjStatusUpdates.emplace_back(pObj); + +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] INFO: Done adding CObjBase to the status update list.\n", (void*)pObj); +#endif } EXC_CATCH; @@ -311,21 +669,429 @@ void CWorldTicker::AddObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static void CWorldTicker::DelObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static { +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] INFO: Trying to remove CObjBase from the status update list.\n", (void*)pObj); +#endif EXC_TRY("DelObjStatusUpdate"); + const ProfileTask timersTask(PROFILE_TIMERS); UnreferencedParameter(fNeedsLock); { #if MT_ENGINES std::unique_lock lock(_ObjStatusUpdates.MT_CMUTEX); #endif - _ObjStatusUpdates.erase(pObj); + //_ObjStatusUpdates.erase(pObj); + + const auto itMissing = std::find( + _ObjStatusUpdates.begin(), + _ObjStatusUpdates.end(), + pObj + ); + if (_ObjStatusUpdates.end() == itMissing) + { +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] WARN: Requested erasure of CObjBase from the status update list, but it wasn't found.\n", (void*)pObj); +#endif + return; + } + + const auto itRepeated = std::find( + _vecObjStatusUpdateEraseRequested.begin(), + _vecObjStatusUpdateEraseRequested.end(), + pObj + ); + if (_vecObjStatusUpdateEraseRequested.end() != itRepeated) + { +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("WARN [%p]: CObjBase erasure from the status update list already requested.\n", (void*)pObj); +#endif + return; + } + + _vecObjStatusUpdateEraseRequested.emplace_back(pObj); +#ifdef DEBUG_STATUSUPDATES + g_Log.EventDebug("[%p] INFO: Done adding CObjBase to the status update remove buffer.\n", (void*)pObj); +#endif } EXC_CATCH; } -// Check timeouts and do ticks +template +static void sortedVecRemoveElementsByIndices(std::vector& vecMain, const std::vector& vecIndicesToRemove) +{ + // Erase in chunks, call erase the least times possible. + if (vecIndicesToRemove.empty()) + return; + + if (vecMain.empty()) + return; + + const size_t sz = vecMain.size(); + +#ifdef DEBUG_LIST_OPS + ASSERT(std::is_sorted(vecMain.begin(), vecMain.end())); + ASSERT(std::is_sorted(vecIndicesToRemove.begin(), vecIndicesToRemove.end())); + // Check that those sorted vectors do not have duplicated values. + ASSERT(std::adjacent_find(vecMain.begin(), vecMain.end()) == vecMain.end()); + ASSERT(std::adjacent_find(vecIndicesToRemove.begin(), vecIndicesToRemove.end()) == vecIndicesToRemove.end()); + + //g_Log.EventDebug("Starting sortedVecRemoveElementsByIndices.\n"); +#endif + // Copy the original vector to check against later + std::vector originalVecMain = vecMain; + + // Start from the end of vecIndicesToRemove, working backwards with normal iterators + auto itRemoveFirst = vecIndicesToRemove.end(); // Points one past the last element + while (itRemoveFirst != vecIndicesToRemove.begin()) + { + // Move the iterator back to point to a valid element. + --itRemoveFirst; + + auto itRemoveLast = itRemoveFirst; // Marks the start of a contiguous block + // Find contiguous block by working backwards and finding consecutive indices + // This loop identifies a contiguous block of indices to remove. + // A block is contiguous if the current index *itRemoveFirst is exactly 1 greater than the previous index. + while (itRemoveFirst != vecIndicesToRemove.begin() && *(itRemoveFirst - 1) + 1 == *itRemoveFirst) + { + --itRemoveFirst; + } + +#ifdef DEBUG_LIST_OPS + /* + g_Log.EventDebug("Removing contiguous indices: %" PRIuSIZE_T " to %" PRIuSIZE_T " (total sizes vecMain: %" PRIuSIZE_T ", vecIndices: %" PRIuSIZE_T ").\n", + *itRemoveFirst, *itRemoveLast, vecMain.size(), vecIndicesToRemove.size()); + */ +#endif + + // Once we find a contiguous block, we erase that block from vecMain. + auto itRemoveLastPast = (*itRemoveLast == vecMain.size() - 1) ? vecMain.end() : (vecMain.begin() + *itRemoveLast + 1); + vecMain.erase(vecMain.begin() + *itRemoveFirst, itRemoveLastPast); + } + +#ifdef DEBUG_LIST_OPS + // Sanity Check: Verify that the removed elements are no longer present in vecMain + for (auto index : vecIndicesToRemove) { + UnreferencedParameter(index); + ASSERT(index < originalVecMain.size()); + ASSERT(std::find(vecMain.begin(), vecMain.end(), originalVecMain[index]) == vecMain.end()); + } + g_Log.EventDebug("Sizes: new vec %" PRIuSIZE_T ", old vec %" PRIuSIZE_T ", remove vec %" PRIuSIZE_T ".\n", + vecMain.size(), sz, vecIndicesToRemove.size()); +#endif + + UnreferencedParameter(sz); + ASSERT(vecMain.size() == sz - vecIndicesToRemove.size()); + + + /* + // Alternative implementation: + // We cannot use a vector with the indices but we need a vector with a copy of the elements to remove. + // std::remove_if in itself is more efficient than multiple erase calls, because the number of the element shifts is lesser. + // Though, we need to consider the memory overhead of reading through an std::pair of two types, which is bigger than just an index. + // Also, jumping across the vector in a non-contiguous way with the binary search can add additional memory overhead by itself, and + // this will be greater the bigger are the elements in the vector.. + // The bottom line is that we need to run some benchmarks between the two algorithms, and possibly also for two versions of this algorithm, + // one using binary search and another with linear search. + // The latter might actually be faster for a small number of elements, since it's more predictable for the prefetcher. + + // Use std::remove_if to shift elements not marked for removal to the front + auto it = std::remove_if(vecMain.begin(), vecMain.end(), + [&](const T& element) { + // Check if the current element is in the valuesToRemove vector using binary search + return std::binary_search(valuesToRemove.begin(), valuesToRemove.end(), element); + }); + + // Erase the removed elements in one go + vecMain.erase(it, vecMain.end()); + */ +} + +template +static void unsortedVecRemoveElementsByValues(std::vector& vecMain, const std::vector & vecValuesToRemove) +{ + if (vecValuesToRemove.empty()) + return; + +#ifdef DEBUG_LIST_OPS + ASSERT(!sl::UnsortedContainerHasDuplicates(vecMain)); + ASSERT(!sl::UnsortedContainerHasDuplicates(vecValuesToRemove)); +#endif + + // Sort valuesToRemove for binary search + //std::sort(vecValuesToRemove.begin(), vecValuesToRemove.end()); + + // Use std::remove_if to shift elements not marked for removal to the front + auto it = std::remove_if(vecMain.begin(), vecMain.end(), + [&](const T& element) { + // Use binary search to check if the element should be removed + //return std::binary_search(vecValuesToRemove.begin(), vecValuesToRemove.end(), element); + return std::find(vecValuesToRemove.begin(), vecValuesToRemove.end(), element) != vecValuesToRemove.end(); + }); + + // Erase the removed elements in one go + vecMain.erase(it, vecMain.end()); +} + + +/* +// To be tested and benchmarked. +template +static void sortedVecRemoveElementsByValues(std::vector& vecMain, const std::vector& toRemove) +{ + if (toRemove.empty() || vecMain.empty()) + return; + + auto mainIt = vecMain.begin(); + auto removeIt = toRemove.begin(); + + // Destination pointer for in-place shifting + auto destIt = mainIt; + + while (mainIt != vecMain.end() && removeIt != toRemove.end()) { + // Skip over elements in the main vector that are smaller than the current element to remove + auto nextRangeEnd = std::lower_bound(mainIt, vecMain.end(), *removeIt); + + // Batch copy the range of elements not marked for removal + std::move(mainIt, nextRangeEnd, destIt); + destIt += std::distance(mainIt, nextRangeEnd); + + // Advance main iterator and remove iterator + mainIt = nextRangeEnd; + + // Skip the elements that need to be removed + if (mainIt != vecMain.end() && *mainIt == *removeIt) { + ++mainIt; + ++removeIt; + } + } + + // Copy the remaining elements if there are any left + std::move(mainIt, vecMain.end(), destIt); + + // Resize the vector to remove the now extraneous elements at the end + vecMain.resize(destIt - vecMain.begin()); +} +*/ + +/* +template +static void sortedVecDifference( + const std::vector& vecMain, const std::vector& vecToRemove, std::vector& vecElemBuffer + ) +{ + auto itMain = vecMain.begin(); // Iterator for vecMain + auto start = itMain; // Start marker for non-matching ranges in vecMain + for (auto& elem : vecToRemove) { + g_Log.EventDebug("Should remove %p.\n", (void*)elem); + } + for (auto& elem : vecMain) { + g_Log.EventDebug("VecMain %p.\n", (void*)elem.second); + } + + // Iterate through each element in vecToRemove to locate and exclude its matches in vecMain + for (const auto& removePtr : vecToRemove) { + // Binary search to find the start of the block where vecMain.second == removePtr + itMain = std::lower_bound(itMain, vecMain.end(), removePtr, + [](const TPair& lhs, const T* rhs) noexcept { + return lhs.second < rhs; // Compare TPair.second with T* + }); + + // Insert all elements from `start` up to `itMain` (non-matching elements) + vecElemBuffer.insert(vecElemBuffer.end(), start, itMain); + + // Skip over all contiguous elements in vecMain that match removePtr + while (itMain != vecMain.end() && itMain->second == removePtr) { + ++itMain; + } + + // Update `start` to the new non-matching range after any matching elements are skipped + start = itMain; + } + + // Insert remaining elements from vecMain after the last matched element + vecElemBuffer.insert(vecElemBuffer.end(), start, vecMain.end()); + + for (auto& elem : vecMain) { + g_Log.EventDebug("VecMain %p.\n", (void*)elem.second); + } +} +*/ + +template +static void unsortedVecDifference( + const std::vector& vecMain, const std::vector& vecToRemove, std::vector& vecElemBuffer + ) +{ + /* + // Iterate through vecMain, adding elements to vecElemBuffer only if they aren't in vecToRemove + for (const auto& elem : vecMain) { + if (std::find(vecToRemove.begin(), vecToRemove.end(), elem.second) == vecToRemove.end()) { + vecElemBuffer.push_back(elem); + } + } + */ + // vecMain is sorted by timeout (first elem of the pair), not by pointer (second elem) + // vecToRemove is sorted by its contents (pointer) + + // Reserve space in vecElemBuffer to avoid reallocations + vecElemBuffer.reserve(vecMain.size() - vecToRemove.size()); + + // Use an iterator to store the position for bulk insertion + auto itCopyFromThis = vecMain.begin(); + /* + auto itFindBegin = vecToRemove.begin(); + + // Iterate through vecMain, copying elements that are not in vecToRemove + for (auto itMain = vecMain.begin(); itMain != vecMain.end(); ++itMain) + { + // Perform a linear search for the current element's pointer in vecToRemove + auto itTemp = std::find(itFindBegin, vecToRemove.end(), itMain->second); + if (itTemp != vecToRemove.end()) + { + // If the element is found in vecToRemove, copy elements before it + vecElemBuffer.insert(vecElemBuffer.end(), itCopyFromThis, itMain); // Copy up to but not including itMain + + // Move itCopyFromThis forward to the next element + itCopyFromThis = itMain + 1; + //itFindBegin = itTemp + 1; + + // Update itFindBegin to continue searching for the next instance in vecToRemove + // We do not change itFindBegin here, since we want to keep searching for this pointer + // in vecToRemove for subsequent elements in vecMain + } + else + { + // If itTemp is not found, we can still copy the current element + // Check if itCopyFromThis is equal to itMain to avoid double-copying in case of duplicates + if (itCopyFromThis != itMain) + { + // Move itCopyFromThis forward to the next element + itCopyFromThis = itMain + 1; + } + } + }*/ + + // Iterate through vecMain, copying elements that are not in vecToRemove + for (auto itMain = vecMain.begin(); itMain != vecMain.end(); ++itMain) { + // Perform a linear search for the current element's pointer in vecToRemove + auto itTemp = std::find(vecToRemove.begin(), vecToRemove.end(), itMain->second); + if (itTemp != vecToRemove.end()) { + // If the element is found in vecToRemove, copy elements before it + vecElemBuffer.insert(vecElemBuffer.end(), itCopyFromThis, itMain); // Copy up to but not including itMain + + // Move itCopyFromThis forward to the next element + itCopyFromThis = itMain + 1; // Move to the next element after itMain + } + } + + // Copy any remaining elements in vecMain after the last found element + vecElemBuffer.insert(vecElemBuffer.end(), itCopyFromThis, vecMain.end()); + +#ifdef DEBUG_LIST_OPS + g_Log.EventDebug("Sizes: new vec %" PRIuSIZE_T ", old vec %" PRIuSIZE_T ", remove vec %" PRIuSIZE_T ".\n", + vecElemBuffer.size(), vecMain.size(), vecToRemove.size()); + + for (auto& elem : vecToRemove) { + g_Log.EventDebug("Should remove %p.\n", (void*)elem); + } + for (auto& elem : vecMain) { + g_Log.EventDebug("VecMain %p.\n", (void*)elem.second); + } + for (auto& elem : vecElemBuffer) { + g_Log.EventDebug("NewVec %p.\n", (void*)elem.second); + } + ASSERT(vecElemBuffer.size() == vecMain.size() - vecToRemove.size()); +#endif +} + +template +static void sortedVecRemoveAddQueued( + std::vector &vecMain, std::vector &vecToRemove, std::vector &vecToAdd, std::vector &vecElemBuffer + ) +{ +#ifdef DEBUG_LIST_OPS + ASSERT(std::is_sorted(vecMain.begin(), vecMain.end())); + ASSERT(std::adjacent_find(vecMain.begin(), vecMain.end()) == vecMain.end()); // no duplicate values +#endif + + //EXC_TRY("vecRemoveAddQueued"); + //EXC_SET_BLOCK("Sort intermediate lists"); + std::sort(vecToAdd.begin(), vecToAdd.end()); + std::sort(vecToRemove.begin(), vecToRemove.end()); + +#ifdef DEBUG_LIST_OPS + ASSERT(std::adjacent_find(vecToAdd.begin(), vecToAdd.end()) == vecToAdd.end()); // no duplicate values + ASSERT(std::adjacent_find(vecToRemove.begin(), vecToRemove.end()) == vecToRemove.end()); // no duplicate values +#endif + + //EXC_SET_BLOCK("Ordered remove"); + if (!vecToRemove.empty()) + { + if (vecMain.empty()) + ASSERT(false); // Shouldn't ever happen. + + // TODO: test and benchmark if the approach of the above function (sortedVecRemoveElementsInPlace) might be faster. + vecElemBuffer.clear(); + vecElemBuffer.reserve(vecMain.size() / 2); + unsortedVecDifference(vecMain, vecToRemove, vecElemBuffer); + +#ifdef DEBUG_LIST_OPS + for (auto& elem : vecToRemove) { + auto it = std::find_if(vecElemBuffer.begin(), vecElemBuffer.end(), [elem](auto &rhs) {return elem == rhs.second;}); + UnreferencedParameter(it); + ASSERT (it == vecElemBuffer.end()); + } + + ASSERT(vecElemBuffer.size() == vecMain.size() - vecToRemove.size()); + ASSERT(std::is_sorted(vecElemBuffer.begin(), vecElemBuffer.end())); + ASSERT(std::adjacent_find(vecElemBuffer.begin(), vecElemBuffer.end()) == vecElemBuffer.end()); // no duplicate values +#endif + + vecMain.swap(vecElemBuffer); + + //vecMain = std::move(vecElemBuffer); + vecElemBuffer.clear(); + vecToRemove.clear(); + +#ifdef DEBUG_LIST_OPS + g_Log.EventDebug("[GLOBAL] STATUS: Nonempty tick list remove buffer processed.\n"); +#endif + } + + //EXC_SET_BLOCK("Mergesort"); + if (!vecToAdd.empty()) + { + vecElemBuffer.clear(); + vecElemBuffer.reserve(vecMain.size() + vecToAdd.size()); + std::merge( + vecMain.begin(), vecMain.end(), + vecToAdd.begin(), vecToAdd.end(), + std::back_inserter(vecElemBuffer) + ); + +#ifdef DEBUG_LIST_OPS + ASSERT(vecElemBuffer.size() == vecMain.size() + vecToAdd.size()); + ASSERT(std::is_sorted(vecElemBuffer.begin(), vecElemBuffer.end())); + ASSERT(std::adjacent_find(vecElemBuffer.begin(), vecElemBuffer.end()) == vecElemBuffer.end()); // no duplicate values +#endif + + vecMain.swap(vecElemBuffer); + //vecMain = std::move(vecElemBuffer); + vecElemBuffer.clear(); + vecToAdd.clear(); + +#ifdef DEBUG_LIST_OPS + g_Log.EventDebug("[GLOBAL] STATUS: Nonempty tick list add buffer processed.\n"); +#endif + } + + //EXC_CATCH: +} + + +// Check timeouts and do ticks void CWorldTicker::Tick() { ADDTOCALLSTACK("CWorldTicker::Tick"); @@ -346,32 +1112,40 @@ void CWorldTicker::Tick() * TODO: implement a new class inheriting from CTimedObject to get rid of this code? */ { - EXC_TRYSUB("StatusUpdates"); { - EXC_SETSUB_BLOCK("Selection"); #if MT_ENGINES std::unique_lock lock_su(_ObjStatusUpdates.MT_CMUTEX); #endif + EXC_TRYSUB("StatusUpdates"); + EXC_SETSUB_BLOCK("Remove requested"); + unsortedVecRemoveElementsByValues(_ObjStatusUpdates, _vecObjStatusUpdateEraseRequested); + + EXC_SETSUB_BLOCK("Selection"); if (!_ObjStatusUpdates.empty()) { for (CObjBase* pObj : _ObjStatusUpdates) { if (pObj && !pObj->_IsBeingDeleted()) - _vecObjs.emplace_back(static_cast(pObj)); + _vecGenericObjsToTick.emplace_back(static_cast(pObj)); } - _ObjStatusUpdates.clear(); } + EXC_CATCHSUB(""); + //EXC_DEBUGSUB_START; + //EXC_DEBUGSUB_END; + _ObjStatusUpdates.clear(); + _vecObjStatusUpdateEraseRequested.clear(); } + EXC_TRYSUB("StatusUpdates"); EXC_SETSUB_BLOCK("Loop"); - for (void* pObjVoid : _vecObjs) + for (void* pObjVoid : _vecGenericObjsToTick) { CObjBase* pObj = static_cast(pObjVoid); pObj->OnTickStatusUpdate(); } EXC_CATCHSUB(""); - _vecObjs.clear(); + _vecGenericObjsToTick.clear(); } } @@ -381,455 +1155,350 @@ void CWorldTicker::Tick() // Items, Chars ... Everything relying on CTimedObject (excepting CObjBase, which inheritance is only virtual) int64 iCurTime = CWorldGameTime::GetCurrentTime().GetTimeRaw(); // Current timestamp, a few msecs will advance in the current tick ... avoid them until the following tick(s). - EXC_SET_BLOCK("TimedObjects"); { + // Need this new scope to give the right lifetime to ProfileTask. + //g_Log.EventDebug("Start ctimedobj section.\n"); + EXC_SET_BLOCK("TimedObjects"); const ProfileTask timersTask(PROFILE_TIMERS); { - // Need here a new, inner scope to get rid of EXC_TRYSUB variables and for the unique_lock - EXC_TRYSUB("Timed Objects Selection"); + // Need here another scope to give the right lifetime to the unique_lock. #if MT_ENGINES std::unique_lock lock(_mWorldTickList.MT_CMUTEX); #endif +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mWorldTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); +#endif + { + // New requests done during the world loop. + EXC_TRYSUB("Update main list"); +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + g_Log.EventDebug("[GLOBAL] STATUS: Updating WorldTickList.\n"); +#endif + sortedVecRemoveAddQueued(_mWorldTickList, _vecWorldObjsEraseRequests, _vecWorldObjsAddRequests, _vecWorldObjsElementBuffer); + EXC_CATCHSUB(""); + } - WorldTickList::iterator itMap = _mWorldTickList.begin(); - WorldTickList::iterator itMapEnd = _mWorldTickList.end(); +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mWorldTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); +#endif - size_t uiProgressive = 0; - int64 iTime; - while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) + // Need here a new, inner scope to get rid of EXC_TRYSUB variables + if (!_mWorldTickList.empty()) { - CTimedObject* pTimedObj = itMap->second; - if (pTimedObj->_IsTimerSet() && pTimedObj->_CanTick()) { - if (pTimedObj->_GetTimeoutRaw() <= iCurTime) + EXC_TRYSUB("Selection"); + _vecIndexMiscBuffer.clear(); + WorldTickList::iterator itMap = _mWorldTickList.begin(); + WorldTickList::iterator itMapEnd = _mWorldTickList.end(); + + size_t uiProgressive = 0; + int64 iTime; + while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) { - if (auto pObjBase = dynamic_cast(pTimedObj)) + CTimedObject* pTimedObj = itMap->second; +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + g_Log.EventDebug("Checking if CTimedObject %p should tick.\n", reinterpret_cast(pTimedObj)); +#endif + if (pTimedObj->_IsTimerSet() && pTimedObj->_CanTick()) { - if (pObjBase->_IsBeingDeleted()) - continue; + if (pTimedObj->_GetTimeoutRaw() <= iCurTime) + { + if (auto pObjBase = dynamic_cast(pTimedObj)) + { + if (pObjBase->_IsBeingDeleted()) + continue; + } + +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + g_Log.EventDebug("Yes it should (%p).\n", reinterpret_cast(pTimedObj)); +#endif + _vecGenericObjsToTick.emplace_back(static_cast(pTimedObj)); + _vecIndexMiscBuffer.emplace_back(uiProgressive); + } + //else + //{ + // // This shouldn't happen... If it does, get rid of the entry on the list anyways, + // // it got desynchronized in some way and might be an invalid or even deleted and deallocated object! + //} } + ++itMap; + ++uiProgressive; + } + EXC_CATCHSUB(""); + } - _vecObjs.emplace_back(static_cast(pTimedObj)); - _vecWorldObjsToEraseFromList.emplace_back(uiProgressive); +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_vecIndexMiscBuffer)); + ASSERT(!sl::SortedContainerHasDuplicates(_vecIndexMiscBuffer)); + ASSERT(!sl::UnsortedContainerHasDuplicates(_vecGenericObjsToTick)); +#endif - pTimedObj->_ClearTimeout(); + { + EXC_TRYSUB("Delete from List"); + sortedVecRemoveElementsByIndices(_mWorldTickList, _vecIndexMiscBuffer); + +#ifdef DEBUG_LIST_OPS + for (void* obj : _vecGenericObjsToTick) { + auto itit = std::find_if(_mWorldTickList.begin(), _mWorldTickList.end(), + [obj](TickingTimedObjEntry const& lhs) { + return static_cast(obj) == lhs.second; + }); + UnreferencedParameter(itit); + ASSERT(itit == _mWorldTickList.end()); } - //else - //{ - // // This shouldn't happen... If it does, get rid of the entry on the list anyways, - // // it got desynchronized in some way and might be an invalid or even deleted and deallocated object! - //} +#endif + EXC_CATCHSUB(""); } - ++itMap; - ++uiProgressive; - } - EXC_CATCHSUB("AddToSubLists"); - } +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mWorldTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); +#endif - { - EXC_TRYSUB("Timed Objects Delete from List"); + // Done working with _mWorldTickList, we don't need the lock from now on. - // Erase in chunks, call erase the least times possible. - if (!_vecWorldObjsToEraseFromList.empty()) - { - /* - g_Log.EventDebug("-- Start WORLDTICK. I need to remove %lu items:\n", _vecWorldObjsToEraseFromList.size()); - std::stringstream ss; - for (size_t elem : _vecWorldObjsToEraseFromList) + lpctstr ptcSubDesc; + for (void* pObjVoid : _vecGenericObjsToTick) // Loop through all msecs stored, unless we passed the timestamp. { - ss << elem << ' '; - } - g_Log.EventDebug("%s\n", ss.str().c_str()); - */ + ptcSubDesc = "Generic"; - if (_vecWorldObjsToEraseFromList.size() > 1) - { - size_t uiCurMapElemIdx = 0; - size_t uiCurVecElemIdx = 0; - //size_t uiSubRangeStartIdx = 0; - WorldTickList::iterator itSubRangeStart = _mWorldTickList.begin(); - WorldTickList::iterator itMap = _mWorldTickList.begin(); - bool fContiguous = true; - bool fFirstMatch = false; - while ((itMap != _mWorldTickList.end()) && - (uiCurMapElemIdx <= _vecWorldObjsToEraseFromList.back()) && - (uiCurVecElemIdx < _vecWorldObjsToEraseFromList.size())) - { - if (!fFirstMatch) - { - if (uiCurMapElemIdx == _vecWorldObjsToEraseFromList[uiCurVecElemIdx]) - { - //uiSubRangeStartIdx = uiCurMapElemIdx; - itSubRangeStart = itMap; - fFirstMatch = true; + EXC_TRYSUB("Tick"); + EXC_SETSUB_BLOCK("Elapsed"); - ++uiCurVecElemIdx; - } + CTimedObject* pTimedObj = static_cast(pObjVoid); + pTimedObj->_ClearTimeout(); - ++itMap; - ++uiCurMapElemIdx; - continue; - } +//#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +// g_Log.EventDebug("Ticking CTimedObject %p.\n", reinterpret_cast(pTimedObj)); +//#endif +#if MT_ENGINES + std::unique_lock lockTimeObj(pTimedObj->MT_CMUTEX); +#endif + const PROFILE_TYPE profile = pTimedObj->_GetProfileType(); + const ProfileTask profileTask(profile); + + // Default to true, so if any error occurs it gets deleted for safety + // (valid only for classes having the Delete method, which, for everyone to know, does NOT destroy the object). + bool fDelete = true; - if (uiCurMapElemIdx == _vecWorldObjsToEraseFromList[uiCurVecElemIdx]) + switch (profile) + { + case PROFILE_ITEMS: { - // Matches. I want to delete this. - if (uiCurMapElemIdx == _vecWorldObjsToEraseFromList[uiCurVecElemIdx - 1] + 1) + CItem* pItem = dynamic_cast(pTimedObj); + ASSERT(pItem); + if (pItem->IsItemEquipped()) { - // I want to delete it and it's contiguous, go on - ASSERT(fContiguous); + ptcSubDesc = "ItemEquipped"; + CObjBaseTemplate* pObjTop = pItem->GetTopLevelObj(); + ASSERT(pObjTop); + + CChar* pChar = dynamic_cast(pObjTop); + if (pChar) + { + fDelete = !pChar->OnTickEquip(pItem); + break; + } + + ptcSubDesc = "Item (fallback)"; + g_Log.Event(LOGL_CRIT, "Item equipped, but not contained in a character? (UID: 0%" PRIx32 ")\n.", pItem->GetUID().GetObjUID()); } else { - // It isn't contiguous. Go below. - ASSERT(!fContiguous); - // This is the first one that matches after previous mismatches. We start this chunk from here. - //uiSubRangeStartIdx = uiCurMapElemIdx; - fContiguous = true; + ptcSubDesc = "Item"; } - - ++itMap; - ++uiCurMapElemIdx; - ++uiCurVecElemIdx; - continue; + fDelete = (pItem->_OnTick() == false); + break; } + break; - // Not contiguous to the next element to be erased (stored in the vector). - // What to do? - if (uiCurMapElemIdx != _vecWorldObjsToEraseFromList[uiCurVecElemIdx]) + case PROFILE_CHARS: { - // I don't want to erase this. - if (!fContiguous) + ptcSubDesc = "Char"; + CChar* pChar = dynamic_cast(pTimedObj); + ASSERT(pChar); + fDelete = !pChar->_OnTick(); + if (!fDelete && pChar->m_pNPC && !pTimedObj->_IsTimerSet()) { - // This is an element after the first one successive to the previous contiguous block (2nd, 3rd...) - // Ignore it. - //g_Log.EventDebug("Skip this %lu\n", uiCurMapElemIdx); - ++itMap; + pTimedObj->_SetTimeoutS(3); //3 seconds timeout to keep NPCs 'alive' } - else - { - // This is the first element after the previous contiguous block - // I want to erase until the previous one - // erase doesn't delete the last element in the range - itMap = _mWorldTickList.erase(itSubRangeStart, itMap); - //g_Log.EventDebug("Skip this %lu, not to be deleted, and...\n", uiCurMapElemIdx); - //g_Log.EventDebug("Erasing %lu items starting from pos %lu\n", (uiCurMapElemIdx - uiSubRangeStartIdx), uiSubRangeStartIdx); - - ++itMap; - itSubRangeStart = itMap; - //uiSubRangeStartIdx = uiCurMapElemIdx; // Not really needed - fContiguous = false; - } - ++uiCurMapElemIdx; - continue; } + break; - ASSERT(false); // Shouldn't really be here. - } - if (fFirstMatch && fContiguous) - { - /*itMap =*/ _mWorldTickList.erase(itSubRangeStart, itMap); // last range to erase - //g_Log.EventDebug("(End cycle) Erasing %lu items starting from pos %lu\n", (uiCurMapElemIdx - uiSubRangeStartIdx), uiSubRangeStartIdx); - } - } - else - { - _mWorldTickList.erase(std::next(_mWorldTickList.begin(), _vecWorldObjsToEraseFromList.front())); - //g_Log.EventDebug("Erasing 1 item.\n"); - } - } - - EXC_CATCHSUB("DeleteFromList"); - _vecWorldObjsToEraseFromList.clear(); - } - - lpctstr ptcSubDesc; - for (void* pObjVoid : _vecObjs) // Loop through all msecs stored, unless we passed the timestamp. - { - ptcSubDesc = "Generic"; - - EXC_TRYSUB("Timed Object Tick"); - EXC_SETSUB_BLOCK("Elapsed"); - - CTimedObject* pTimedObj = static_cast(pObjVoid); - -#if MT_ENGINES - std::unique_lock lockTimeObj(pTimedObj->MT_CMUTEX); -#endif - - const PROFILE_TYPE profile = pTimedObj->_GetProfileType(); - const ProfileTask profileTask(profile); + case PROFILE_SECTORS: + { + ptcSubDesc = "Sector"; + fDelete = false; // sectors should NEVER be deleted. + pTimedObj->_OnTick(); + } + break; - // Default to true, so if any error occurs it gets deleted for safety - // (valid only for classes having the Delete method, which, for everyone to know, does NOT destroy the object). - bool fDelete = true; + case PROFILE_MULTIS: + { + ptcSubDesc = "Multi"; + fDelete = !pTimedObj->_OnTick(); + } + break; - switch (profile) - { - case PROFILE_ITEMS: - { - CItem* pItem = dynamic_cast(pTimedObj); - ASSERT(pItem); - if (pItem->IsItemEquipped()) - { - ptcSubDesc = "ItemEquipped"; - CObjBaseTemplate* pObjTop = pItem->GetTopLevelObj(); - ASSERT(pObjTop); + case PROFILE_SHIPS: + { + ptcSubDesc = "ItemShip"; + fDelete = !pTimedObj->_OnTick(); + } + break; - CChar* pChar = dynamic_cast(pObjTop); - if (pChar) + case PROFILE_TIMEDFUNCTIONS: { - fDelete = !pChar->OnTickEquip(pItem); - break; + ptcSubDesc = "TimedFunction"; + fDelete = false; + pTimedObj->_OnTick(); } + break; - ptcSubDesc = "Item (fallback)"; - g_Log.Event(LOGL_CRIT, "Item equipped, but not contained in a character? (UID: 0%" PRIx32 ")\n.", pItem->GetUID().GetObjUID()); - } - else - { - ptcSubDesc = "Item"; + default: + { + ptcSubDesc = "Default"; + fDelete = !pTimedObj->_OnTick(); + } + break; } - fDelete = (pItem->_OnTick() == false); - break; - } - break; - case PROFILE_CHARS: - { - ptcSubDesc = "Char"; - CChar* pChar = dynamic_cast(pTimedObj); - ASSERT(pChar); - fDelete = !pChar->_OnTick(); - if (!fDelete && pChar->m_pNPC && !pTimedObj->_IsTimerSet()) + if (fDelete) { - pTimedObj->_SetTimeoutS(3); //3 seconds timeout to keep NPCs 'alive' + EXC_SETSUB_BLOCK("Delete"); + CObjBase* pObjBase = dynamic_cast(pTimedObj); + ASSERT(pObjBase); // Only CObjBase-derived objects have the Delete method, and should be Delete-d. + pObjBase->Delete(); } - } - break; - - case PROFILE_SECTORS: - { - ptcSubDesc = "Sector"; - fDelete = false; // sectors should NEVER be deleted. - pTimedObj->_OnTick(); - } - break; - - case PROFILE_MULTIS: - { - ptcSubDesc = "Multi"; - fDelete = !pTimedObj->_OnTick(); - } - break; - - case PROFILE_SHIPS: - { - ptcSubDesc = "ItemShip"; - fDelete = !pTimedObj->_OnTick(); - } - break; - case PROFILE_TIMEDFUNCTIONS: - { - ptcSubDesc = "TimedFunction"; - fDelete = false; - pTimedObj->_OnTick(); - } - break; - - default: - { - ptcSubDesc = "Default"; - fDelete = !pTimedObj->_OnTick(); + EXC_CATCHSUB(ptcSubDesc); } - break; - } - - if (fDelete) - { - EXC_SETSUB_BLOCK("Delete"); - CObjBase* pObjBase = dynamic_cast(pTimedObj); - ASSERT(pObjBase); // Only CObjBase-derived objects have the Delete method, and should be Delete-d. - pObjBase->Delete(); } - - EXC_CATCHSUB(ptcSubDesc); +/* +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + else + g_Log.EventDebug("No ctimedobj ticks for this loop.\n"); +#endif +*/ } } - _vecObjs.clear(); + _vecGenericObjsToTick.clear(); + //g_Log.EventDebug("END ctimedobj section.\n"); // ---- /* Periodic, automatic ticking for every char */ + // No need another scope here to encapsulate this ProfileTask, because from now on, to the end of this method, + // everything we do is related to char-only stuff. + //g_Log.EventDebug("Start periodic ticks section.\n"); + EXC_SET_BLOCK("Char Periodic Ticks"); const ProfileTask taskChars(PROFILE_CHARS); - { - // Need here a new, inner scope to get rid of EXC_TRYSUB variables, and for the unique_lock - EXC_TRYSUB("Char Periodic Ticks Selection"); + // Need here another scope to give the right lifetime to the unique_lock. #if MT_ENGINES std::unique_lock lock(_mCharTickList.MT_CMUTEX); #endif - - CharTickList::iterator itMap = _mCharTickList.begin(); - CharTickList::iterator itMapEnd = _mCharTickList.end(); - - size_t uiProgressive = 0; - int64 iTime; - while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) { - CChar* pChar = itMap->second; - - if ((pChar->_iTimePeriodicTick != 0) && pChar->_CanTick() && !pChar->_IsBeingDeleted()) - { - if (pChar->_iTimePeriodicTick <= iCurTime) - { - _vecObjs.emplace_back(static_cast(pChar)); - _vecPeriodicCharsToEraseFromList.emplace_back(uiProgressive); +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mCharTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mCharTickList)); +#endif - pChar->_iTimePeriodicTick = 0; - } - //else - //{ - // // This shouldn't happen... If it does, get rid of the entry on the list anyways, - // // it got desynchronized in some way and might be an invalid or even deleted and deallocated object! - //} + // New requests done during the world loop. + EXC_TRYSUB("Update main list"); - } - ++itMap; - ++uiProgressive; +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + g_Log.EventDebug("[GLOBAL] STATUS: Updating CharTickList.\n"); +#endif + sortedVecRemoveAddQueued(_mCharTickList, _vecPeriodicCharsEraseRequests, _vecPeriodicCharsAddRequests, _vecPeriodicCharsElementBuffer); + EXC_CATCHSUB(""); + ASSERT(sl::ContainerIsSorted(_mCharTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mCharTickList)); } - EXC_CATCHSUB(""); - } - - { - EXC_TRYSUB("Periodic Chars Delete from List"); -//#if MT_ENGINES -// std::unique_lock lockTimeObj(pTimedObj->MT_CMUTEX); -//#endif - // Erase in chunks, call erase the least times possible. - if (!_vecPeriodicCharsToEraseFromList.empty()) + if (!_mCharTickList.empty()) + { { - /* - g_Log.EventDebug("-- Start CHARPERIODICTICK. I need to remove %lu items:\n", _vecPeriodicCharsToEraseFromList.size()); - std::stringstream ss; - for (size_t elem : _vecPeriodicCharsToEraseFromList) - { - ss << elem << ' '; - } - g_Log.EventDebug("%s\n", ss.str().c_str()); - */ + EXC_TRYSUB("Selection"); +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + //g_Log.EventDebug("Start looping through char periodic ticks.\n"); +#endif + _vecIndexMiscBuffer.clear(); + CharTickList::iterator itMap = _mCharTickList.begin(); + CharTickList::iterator itMapEnd = _mCharTickList.end(); - if (_vecPeriodicCharsToEraseFromList.size() > 1) + size_t uiProgressive = 0; + int64 iTime; + while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) { - size_t uiCurMapElemIdx = 0; - size_t uiCurVecElemIdx = 0; - // size_t uiSubRangeStartIdx = 0; - CharTickList::iterator itSubRangeStart = _mCharTickList.begin(); - CharTickList::iterator itMap = _mCharTickList.begin(); - bool fContiguous = true; - bool fFirstMatch = false; - while ((itMap != _mCharTickList.end()) && - (uiCurMapElemIdx <= _vecPeriodicCharsToEraseFromList.back()) && - (uiCurVecElemIdx < _vecPeriodicCharsToEraseFromList.size())) + CChar* pChar = itMap->second; +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + g_Log.EventDebug("Executing char periodic tick: %p. Registered time: %" PRId64 ". pChar->_iTimePeriodicTick: %" PRId64 "\n", + (void*)pChar, itMap->first, pChar->_iTimePeriodicTick); + ASSERT(itMap->first == pChar->_iTimePeriodicTick); +#endif + if ((pChar->_iTimePeriodicTick != 0) && pChar->_CanTick() && !pChar->_IsBeingDeleted()) { - if (!fFirstMatch) + if (pChar->_iTimePeriodicTick <= iCurTime) { - if (uiCurMapElemIdx == _vecPeriodicCharsToEraseFromList[uiCurVecElemIdx]) - { - //uiSubRangeStartIdx = uiCurMapElemIdx; - itSubRangeStart = itMap; - fFirstMatch = true; - - ++uiCurVecElemIdx; - } - - ++itMap; - ++uiCurMapElemIdx; - continue; + _vecGenericObjsToTick.emplace_back(static_cast(pChar)); + _vecIndexMiscBuffer.emplace_back(uiProgressive); } + //else + //{ + // // This shouldn't happen... If it does, get rid of the entry on the list anyways, + // // it got desynchronized in some way and might be an invalid or even deleted and deallocated object! + //} - if (uiCurMapElemIdx == _vecPeriodicCharsToEraseFromList[uiCurVecElemIdx]) - { - // Matches. I want to delete this. - if (uiCurMapElemIdx == _vecPeriodicCharsToEraseFromList[uiCurVecElemIdx - 1] + 1) - { - // I want to delete it and it's contiguous, go on - ASSERT(fContiguous); - } - else - { - // It isn't contiguous. Go below. - ASSERT(!fContiguous); - // This is the first one that matches after previous mismatches. We start this chunk from here. - //uiSubRangeStartIdx = uiCurMapElemIdx; - fContiguous = true; - } + } + ++itMap; + ++uiProgressive; + } + EXC_CATCHSUB(""); - ++itMap; - ++uiCurMapElemIdx; - ++uiCurVecElemIdx; - continue; - } +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_vecIndexMiscBuffer)); + ASSERT(!sl::UnsortedContainerHasDuplicates(_vecGenericObjsToTick)); + ASSERT(!sl::SortedContainerHasDuplicates(_vecIndexMiscBuffer)); +#endif - // Not contiguous to the next element to be erased (stored in the vector). - // What to do? - if (uiCurMapElemIdx != _vecPeriodicCharsToEraseFromList[uiCurVecElemIdx]) - { - // I don't want to erase this. - if (!fContiguous) - { - // This is an element after the first one successive to the previous contiguous block (2nd, 3rd...) - // Ignore it. - //g_Log.EventDebug("Skip this %lu\n", uiCurMapElemIdx); - ++itMap; - } - else - { - // This is the first element after the previous contiguous block - // I want to erase until the previous one - // erase doesn't delete the last element in the range - //g_Log.EventDebug("Skip this %lu, not to be deleted, and...\n", uiCurMapElemIdx); - //g_Log.EventDebug("Erasing %lu items starting from pos %lu\n", (uiCurMapElemIdx - uiSubRangeStartIdx), uiSubRangeStartIdx); - - itMap = _mCharTickList.erase(itSubRangeStart, itMap); - ++itMap; - itSubRangeStart = itMap; - //uiSubRangeStartIdx = uiCurMapElemIdx; // Not really needed - fContiguous = false; - } - ++uiCurMapElemIdx; - continue; - } +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + //g_Log.EventDebug("Done looping through char periodic ticks. Need to tick n %" PRIuSIZE_T " objs.\n", _vecGenericObjsToTick.size()); +#endif + } - ASSERT(false); // Shouldn't really be here. - } - if (fFirstMatch && fContiguous) - { - /*itMap =*/ _mCharTickList.erase(itSubRangeStart, itMap); // last range to erase - //g_Log.EventDebug("(End cycle) Erasing %lu items starting from pos %lu\n", (uiCurMapElemIdx - uiSubRangeStartIdx), uiSubRangeStartIdx); - } - } - else - { - _mCharTickList.erase(std::next(_mCharTickList.begin(), _vecPeriodicCharsToEraseFromList.front())); - //g_Log.EventDebug("Erasing 1 item.\n"); - } + { + EXC_TRYSUB("Delete from List"); + // Erase in chunks, call erase the least times possible. + sortedVecRemoveElementsByIndices(_mCharTickList, _vecIndexMiscBuffer); + EXC_CATCHSUB("DeleteFromList"); + + _vecIndexMiscBuffer.clear(); } - EXC_CATCHSUB("DeleteFromList"); - _vecPeriodicCharsToEraseFromList.clear(); +#ifdef DEBUG_LIST_OPS + ASSERT(sl::ContainerIsSorted(_mCharTickList)); + ASSERT(!sl::SortedContainerHasDuplicates(_mCharTickList)); +#endif + + // Done working with _mCharTickList, we don't need the lock from now on. + } +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + else + {}; //g_Log.EventDebug("No char periodic ticks for this loop.\n"); +#endif } { EXC_TRYSUB("Char Periodic Ticks Loop"); - for (void* pObjVoid : _vecObjs) // Loop through all msecs stored, unless we passed the timestamp. + for (void* pObjVoid : _vecGenericObjsToTick) // Loop through all msecs stored, unless we passed the timestamp. { CChar* pChar = static_cast(pObjVoid); + pChar->_iTimePeriodicTick = 0; if (pChar->OnTickPeriodic()) { AddCharTicking(pChar, false); @@ -840,8 +1509,11 @@ void CWorldTicker::Tick() } } EXC_CATCHSUB(""); - _vecObjs.clear(); + + _vecGenericObjsToTick.clear(); } + //g_Log.EventDebug("END periodic ticks section.\n"); + EXC_CATCH; } diff --git a/src/game/CWorldTicker.h b/src/game/CWorldTicker.h index 22729e3b2..a0d02a413 100644 --- a/src/game/CWorldTicker.h +++ b/src/game/CWorldTicker.h @@ -8,7 +8,12 @@ #include "CTimedFunctionHandler.h" #include "CTimedObject.h" -/* Include phmap.h */ +/* + * #include +*/ + +/* +//--- Include phmap.h #ifdef ADDRESS_SANITIZER #define MYASAN_ #endif @@ -36,20 +41,8 @@ #ifdef __GNUC__ #pragma GCC diagnostic pop #endif -/* End of phmap.h inclusion */ - -/* Include btree.h */ -#if NON_MSVC_COMPILER - #pragma GCC diagnostic push - #pragma GCC diagnostic ignored "-Wzero-as-null-pointer-constant" -#endif - -#include - -#if NON_MSVC_COMPILER - #pragma GCC diagnostic pop -#endif -/* End of btree.h inclusion */ +//--- End of phmap.h inclusion +*/ class CObjBase; @@ -64,18 +57,25 @@ class CWorldTicker ~CWorldTicker() = default; private: - struct WorldTickList : public phmap::btree_multimap + // Generic Timers. Calls to OnTick. + using TickingTimedObjEntry = std::pair; + struct WorldTickList : public std::vector { MT_CMUTEX_DEF; }; - struct CharTickList : public phmap::btree_multimap + // Calls to OnTickPeriodic. Regens and periodic checks. + using TickingPeriodicCharEntry = std::pair; + struct CharTickList : public std::vector { MT_CMUTEX_DEF; }; - struct StatusUpdatesList : public phmap::parallel_flat_hash_set + // Calls to OnTickStatusUpdate. Periodically send updated infos to the clients. + //struct StatusUpdatesList : public phmap::parallel_flat_hash_set + //struct StatusUpdatesList : public fc::flat_set //struct StatusUpdatesList : public std::unordered_set + struct StatusUpdatesList : public std::vector { MT_CMUTEX_DEF; }; @@ -85,13 +85,19 @@ class CWorldTicker friend class CWorldTickingList; StatusUpdatesList _ObjStatusUpdates; // objects that need OnTickStatusUpdate called + std::vector _vecObjStatusUpdateEraseRequested; // Reuse the same container (using void pointers statically casted) to avoid unnecessary reallocations. - std::vector _vecObjs; - // "Index" in the multimap - std::vector _vecWorldObjsToEraseFromList; - // "Index" in the multimap - std::vector _vecPeriodicCharsToEraseFromList; + std::vector _vecGenericObjsToTick; + std::vector _vecIndexMiscBuffer; + + std::vector _vecWorldObjsAddRequests; + std::vector _vecWorldObjsEraseRequests; + std::vector _vecWorldObjsElementBuffer; + + std::vector _vecPeriodicCharsAddRequests; + std::vector _vecPeriodicCharsEraseRequests; + std::vector _vecPeriodicCharsElementBuffer; //---- @@ -113,10 +119,10 @@ class CWorldTicker void DelObjStatusUpdate(CObjBase* pObj, bool fNeedsLock); private: - void _InsertTimedObject(const int64 iTimeout, CTimedObject* pTimedObject); - void _RemoveTimedObject(const int64 iOldTimeout, CTimedObject* pTimedObject); - void _InsertCharTicking(const int64 iTickNext, CChar* pChar); - void _RemoveCharTicking(const int64 iOldTimeout, CChar* pChar); + void _InsertTimedObject(int64 iTimeout, CTimedObject* pTimedObject); + void _RemoveTimedObject(CTimedObject* pTimedObject); + void _InsertCharTicking(int64 iTickNext, CChar* pChar); + bool _RemoveCharTicking(CChar* pChar); }; #endif // _INC_CWORLDTICKER_H diff --git a/src/game/chars/CChar.cpp b/src/game/chars/CChar.cpp index 693ec86fa..6c755d6de 100644 --- a/src/game/chars/CChar.cpp +++ b/src/game/chars/CChar.cpp @@ -340,7 +340,7 @@ CChar::CChar( CREID_TYPE baseID ) : CChar::~CChar() { ADDTOCALLSTACK("CChar::~CChar"); - EXC_TRY("Cleanup in destructor"); + EXC_TRY("Cleanup in destructor"); CChar::DeletePrepare(); CChar::DeleteCleanup(true); @@ -397,7 +397,7 @@ void CChar::DeleteCleanup(bool fForce) } } -// Called before Delete() +// Called before Delete(). Notify the world/scripts that i'm going to delete this char. // @Destroy or f_onchar_delete can prevent the deletion bool CChar::NotifyDelete(bool fForce) { @@ -1117,7 +1117,7 @@ bool CChar::DupeFrom(const CChar * pChar, bool fNewbieItems ) m_atUnk.m_dwArg2 = pChar->m_atUnk.m_dwArg2; m_atUnk.m_dwArg3 = pChar->m_atUnk.m_dwArg3; - _iTimeNextRegen = pChar->_iTimeNextRegen; + //_iTimeNextRegen = pChar->_iTimeNextRegen; _iTimeCreate = pChar->_iTimeCreate; _iTimeLastHitsUpdate = pChar->_iTimeLastHitsUpdate; @@ -1262,7 +1262,7 @@ bool CChar::DupeFrom(const CChar * pChar, bool fNewbieItems ) FixWeight(); - if (!pChar->IsSleeping()) + if (!pChar->IsSleeping() && !IsSleeping()) { _GoAwake(); } @@ -4680,7 +4680,7 @@ bool CChar::r_Verb( CScript &s, CTextConsole * pSrc ) // Execute command from sc Effect( EFFECT_LIGHTNING, ITEMID_NOTHING, pCharSrc ); OnTakeDamage( 10000, pCharSrc, DAMAGE_GOD ); Stat_SetVal( STAT_STR, 0 ); - g_Log.Event( LOGL_EVENT|LOGM_KILLS|LOGM_GM_CMDS, "'%s' was KILLed by '%s'\n", GetName(), pSrc->GetName()); + g_Log.Event( LOGL_EVENT|LOGM_KILLS|LOGM_GM_CMDS, "'%s' (ptr %p) was KILLed by '%s'\n", GetName(), (void*)this, pSrc->GetName()); } break; case CHV_MAKEITEM: diff --git a/src/game/chars/CCharNPCAct.cpp b/src/game/chars/CCharNPCAct.cpp index 9f7f81d58..27f573784 100644 --- a/src/game/chars/CCharNPCAct.cpp +++ b/src/game/chars/CCharNPCAct.cpp @@ -639,7 +639,7 @@ int CChar::NPC_WalkToPoint( bool fRun ) else if (iTickNext > 5 * MSECS_PER_SEC) // neither more than 5 seconds. iTickNext = 5 * MSECS_PER_SEC; - _SetTimeout(iTickNext); + _SetTimeout(iTickNext); EXC_CATCH; return 1; } diff --git a/src/game/chars/CCharStatus.cpp b/src/game/chars/CCharStatus.cpp index 80f9c7bb7..503077605 100644 --- a/src/game/chars/CCharStatus.cpp +++ b/src/game/chars/CCharStatus.cpp @@ -2026,7 +2026,7 @@ CRegion *CChar::CheckValidMove( CPointMap &ptDest, uint64 *uiBlockFlags, DIR_TYP // Fill in the proper ptDest.m_z value for this location. (if walking) // pdwBlockFlags = what is blocking me. (can be null = don't care) - // test diagonal dirs by two others *only* when already having a normal location + // test diagonal dirs by two others *only* when already having a normal location (not diagonal) { const CPointMap ptOld(GetTopPoint()); if (!fPathFinding && ptOld.IsValidPoint() && (dir % 2)) From f20a80c72b6ce9015585df7242c0464b742c6760 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 30 Jan 2025 18:32:03 +0100 Subject: [PATCH 05/13] Set nonblocking input if not running inside a terminal into linux --- src/sphere/UnixTerminal.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/sphere/UnixTerminal.cpp b/src/sphere/UnixTerminal.cpp index f11449411..4c1819970 100644 --- a/src/sphere/UnixTerminal.cpp +++ b/src/sphere/UnixTerminal.cpp @@ -28,6 +28,17 @@ UnixTerminal::~UnixTerminal() //_thread_selfTerminateAfterThisTick = true; // just to be sure } +/* +bool isInputAvailable() +{ + fd_set fds; + struct timeval tv = {0, 0}; // Non-blocking, immediate return + FD_ZERO(&fds); + FD_SET(STDIN_FILENO, &fds); + return select(STDIN_FILENO + 1, &fds, NULL, NULL, &tv) > 0; +} +*/ + bool UnixTerminal::isReady() { ADDTOCALLSTACK("UnixTerminal::isReady"); @@ -176,6 +187,13 @@ void UnixTerminal::prepare() if (tcsetattr(STDIN_FILENO, TCSANOW, &term_caps) < 0) throw CSError(LOGL_WARN, 0, "failed to set terminal attributes"); + } + else + { +//#ifdef _POSIX_VERSION + int flags = fcntl(STDIN_FILENO, F_GETFL, 0); + fcntl(STDIN_FILENO, F_SETFL, flags | O_NONBLOCK); +//#endif } setbuf(stdin, nullptr); #endif From daa4d5f98c8d6bb928347364ae8b97e467c12be7 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 30 Jan 2025 22:24:55 +0100 Subject: [PATCH 06/13] cleanup --- src/game/CObjBase.cpp | 2 +- src/game/CTimedObject.cpp | 1 - src/game/CWorldTicker.cpp | 4 ++-- src/game/chars/CChar.cpp | 4 ++-- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/game/CObjBase.cpp b/src/game/CObjBase.cpp index d4360268b..bf8e28dde 100644 --- a/src/game/CObjBase.cpp +++ b/src/game/CObjBase.cpp @@ -765,7 +765,7 @@ bool CObjBase::MoveNear(CPointMap pt, ushort iSteps ) // Move to nearby this other object. // Actually move it within +/- iSteps - // TODO: WUT?? + // TODO: check this again... CPointMap ptOld(pt); for ( uint i = 0; i < iSteps; ++i ) { diff --git a/src/game/CTimedObject.cpp b/src/game/CTimedObject.cpp index 94d2f8fca..ae4a6b771 100644 --- a/src/game/CTimedObject.cpp +++ b/src/game/CTimedObject.cpp @@ -62,7 +62,6 @@ void CTimedObject::_SetTimeout(int64 iDelayInMsecs) { ADDTOCALLSTACK_DEBUG("CTimedObject::_SetTimeout"); // Assume we have the mutex already locked here - g_Log.EventDebug("[%p] _SetTimeout to %" PRId64 ".\n", (void*)this, iDelayInMsecs); const ProfileTask timersTask(PROFILE_TIMERS); // profile the settimeout proccess. if (_IsDeleted()) //prevent deleted objects from setting new timers to avoid nullptr calls diff --git a/src/game/CWorldTicker.cpp b/src/game/CWorldTicker.cpp index 82c5adf8d..811f63cbb 100644 --- a/src/game/CWorldTicker.cpp +++ b/src/game/CWorldTicker.cpp @@ -15,8 +15,8 @@ //# define DEBUG_STATUSUPDATES # define DEBUG_LIST_OPS -# define DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE -# define DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE +//# define DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE +//# define DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE //# define BENCHMARK_LISTS // TODO #endif diff --git a/src/game/chars/CChar.cpp b/src/game/chars/CChar.cpp index 6c755d6de..030d48164 100644 --- a/src/game/chars/CChar.cpp +++ b/src/game/chars/CChar.cpp @@ -1117,7 +1117,7 @@ bool CChar::DupeFrom(const CChar * pChar, bool fNewbieItems ) m_atUnk.m_dwArg2 = pChar->m_atUnk.m_dwArg2; m_atUnk.m_dwArg3 = pChar->m_atUnk.m_dwArg3; - //_iTimeNextRegen = pChar->_iTimeNextRegen; + _iTimeNextRegen = pChar->_iTimeNextRegen; _iTimeCreate = pChar->_iTimeCreate; _iTimeLastHitsUpdate = pChar->_iTimeLastHitsUpdate; @@ -4680,7 +4680,7 @@ bool CChar::r_Verb( CScript &s, CTextConsole * pSrc ) // Execute command from sc Effect( EFFECT_LIGHTNING, ITEMID_NOTHING, pCharSrc ); OnTakeDamage( 10000, pCharSrc, DAMAGE_GOD ); Stat_SetVal( STAT_STR, 0 ); - g_Log.Event( LOGL_EVENT|LOGM_KILLS|LOGM_GM_CMDS, "'%s' (ptr %p) was KILLed by '%s'\n", GetName(), (void*)this, pSrc->GetName()); + g_Log.Event( LOGL_EVENT|LOGM_KILLS|LOGM_GM_CMDS, "'%s' was KILLed by '%s'\n", GetName(), pSrc->GetName()); } break; case CHV_MAKEITEM: From 14cf3dc0f0f36c16ccea99bd693b6d3f9d3e753a Mon Sep 17 00:00:00 2001 From: cbnolok Date: Mon, 3 Feb 2025 17:37:57 +0100 Subject: [PATCH 07/13] More CWorldTicker cleanup, removed some redundant tick list add ops. --- src/game/CObjBase.cpp | 15 +-- src/game/CTimedObject.cpp | 9 +- src/game/CWorldTicker.cpp | 223 +++++++++++++---------------------- src/game/CWorldTicker.h | 39 ------ src/game/chars/CCharAct.cpp | 3 +- src/game/chars/CCharStat.cpp | 5 +- 6 files changed, 96 insertions(+), 198 deletions(-) diff --git a/src/game/CObjBase.cpp b/src/game/CObjBase.cpp index bf8e28dde..3c7110030 100644 --- a/src/game/CObjBase.cpp +++ b/src/game/CObjBase.cpp @@ -3145,13 +3145,13 @@ void CObjBase::UpdatePropertyFlag() if (!(g_Cfg.m_iFeatureAOS & FEATURE_AOS_UPDATE_B) || g_Serv.IsLoading()) return; - m_fStatusUpdate |= SU_UPDATE_TOOLTIP; + m_fStatusUpdate |= SU_UPDATE_TOOLTIP; // Items equipped, inside containers or with timer expired doesn't receive ticks and need to be added to a list of items to be processed separately - if (!IsTopLevel() || _IsTimerExpired()) + if (!IsTopLevel() || _IsTimerExpired()) { CWorldTickingList::AddObjStatusUpdate(this, false); - } + } } dword CObjBase::GetPropertyHash() const @@ -3189,12 +3189,7 @@ void CObjBase::_GoAwake() if (auto pContainer = dynamic_cast(this)) { pContainer->_GoAwake(); // This method isn't virtual - } - - if (_IsTimerSet()) - { - CWorldTickingList::AddObjSingle(_GetTimeoutRaw(), this, true); - } + } // CWorldTickingList::AddObjStatusUpdate(this, false); // Don't! It's done when needed in UpdatePropertyFlag() } @@ -3207,6 +3202,8 @@ void CObjBase::_GoSleep() { CWorldTickingList::DelObjSingle(this); } + + // Most objects won't be into the status update list, but we have to check anyways. CWorldTickingList::DelObjStatusUpdate(this, false); } diff --git a/src/game/CTimedObject.cpp b/src/game/CTimedObject.cpp index ae4a6b771..fe8832d98 100644 --- a/src/game/CTimedObject.cpp +++ b/src/game/CTimedObject.cpp @@ -28,13 +28,10 @@ CTimedObject::~CTimedObject() void CTimedObject::_GoAwake() { ADDTOCALLSTACK("CTimedObject::_GoAwake"); - /* - * if the timeout did expire then it got ignored on its tick and removed from the tick's map so we add it again, - * otherwise it's not needed since the timer is already there. - */ - if ((_iTimeout > 0) && (_iTimeout < CWorldGameTime::GetCurrentTime().GetTimeRaw())) + + if (_IsTimerSet()) { - _SetTimeout(1); // set to 1 msec to tick it ASAP. + CWorldTickingList::AddObjSingle(_GetTimeoutRaw(), this, true); } _fIsSleeping = false; } diff --git a/src/game/CWorldTicker.cpp b/src/game/CWorldTicker.cpp index 811f63cbb..002228c28 100644 --- a/src/game/CWorldTicker.cpp +++ b/src/game/CWorldTicker.cpp @@ -9,14 +9,16 @@ #include "CWorldGameTime.h" #include "CWorldTicker.h" -#ifdef _DEBUG +#if defined _DEBUG || defined _NIGHTLY # define DEBUG_CTIMEDOBJ_TIMED_TICKING # define DEBUG_CCHAR_PERIODIC_TICKING -//# define DEBUG_STATUSUPDATES +# define DEBUG_STATUSUPDATES # define DEBUG_LIST_OPS //# define DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE //# define DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE +//# define DEBUG_STATUSUPDATES_VERBOSE +//# define DEBUG_LIST_OPS_VERBOSE //# define BENCHMARK_LISTS // TODO #endif @@ -59,45 +61,35 @@ void CWorldTicker::_InsertTimedObject(const int64 iTimeout, CTimedObject* pTimed fnFindEntryByObj); if (_vecWorldObjsAddRequests.end() != itEntryInAddList) { -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING g_Log.EventDebug("[WorldTicker][%p] WARN: CTimedObj insertion into ticking list already requested with %s timer. OVERWRITING.\n", (void*)pTimedObject, ((itEntryInAddList->first == iTimeout) ? "same" : "different")); #endif itEntryInAddList->first = iTimeout; - return; // Already requested the addition. - } - /* - const auto itFoundEraseRequest = std::find( - _vecWorldObjsEraseRequested.begin(), - _vecWorldObjsEraseRequested.end(), - pTimedObject); - if (_vecWorldObjsEraseRequested.end() != itFoundEraseRequest) - { -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[%p] WARN: Stopped attempt of inserting a CTimedObj which removal from ticking list has been requested before!\n", (void*)pTimedObject); -#endif return; // Already requested the addition. - }*/ + } +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING const auto itEntryInTickList = std::find_if( _mWorldTickList.begin(), _mWorldTickList.end(), fnFindEntryByObj); if (_mWorldTickList.end() != itEntryInTickList) { + /* if (itEntryInTickList->first == iTimeout) { #ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested insertion of a CTimedObj in the main ticking list with the same timeout, skipping.\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested insertion of a CTimedObj in the main ticking list with the same timeout, skipping.\n", (void*)pTimedObject); #endif return; } + */ -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested insertion of a CTimedObj already in the main ticking list.\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested insertion of a CTimedObj already in the main ticking list.\n", (void*)pTimedObject); # endif const auto itEntryInEraseList = std::find( @@ -115,11 +107,9 @@ void CWorldTicker::_InsertTimedObject(const int64 iTimeout, CTimedObject* pTimed } # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: But that's fine, because i already have requested to remove that!\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: But that's fine, because i already have requested to remove that!\n", (void*)pTimedObject); # endif -#endif } -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING else { const auto itEntryInEraseList = std::find( @@ -174,14 +164,13 @@ void CWorldTicker::_RemoveTimedObject(CTimedObject* pTimedObject) #ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE g_Log.EventDebug("[WorldTicker][%p] INFO: Removing CTimedObj from the ticking list add buffer.\n", (void*)pTimedObject); - +# endif if (itEntryInRemoveList == _vecWorldObjsEraseRequests.end()) { ASSERT(itEntryInTickList == _mWorldTickList.end()); } else if (itEntryInRemoveList != _vecWorldObjsEraseRequests.end()) { ASSERT(itEntryInTickList != _mWorldTickList.end()); } -# endif #endif return; @@ -190,10 +179,11 @@ void CWorldTicker::_RemoveTimedObject(CTimedObject* pTimedObject) if (_vecWorldObjsEraseRequests.end() != itEntryInRemoveList) { // I have already requested to remove this from the main ticking list. + // It can happen and it's legit. Example: we call this method via CObjBase::Delete and ~CObjBase. +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE + g_Log.EventDebug("[WorldTicker][%p] INFO: CTimedObj removal from the main ticking list already requested.\n", (void*)pTimedObject); +#endif #ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING -# ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: CTimedObj removal from the main ticking list already requested.\n", (void*)pTimedObject); -# endif ASSERT(itEntryInAddList == _vecWorldObjsAddRequests.end()); #endif return; // Already requested the removal. @@ -201,9 +191,9 @@ void CWorldTicker::_RemoveTimedObject(CTimedObject* pTimedObject) if (itEntryInTickList == _mWorldTickList.end()) { - // Not found. The object might have a timeout while being in a non-tickable state, so it isn't in the list. -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested erasure of TimedObject in mWorldTickList, but it wasn't found.\n", (void*)pTimedObject); + // Not found. The object might have a timeout while being in a non-tickable state (like at server startup), so it isn't in the list. +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested erasure of TimedObject in mWorldTickList, but it wasn't found there.\n", (void*)pTimedObject); #endif return; } @@ -231,7 +221,9 @@ void CWorldTicker::AddTimedObject(const int64 iTimeout, CTimedObject* pTimedObje const int64 iTickOld = pTimedObject->_GetTimeoutRaw(); if (iTickOld != 0) { - // Adding an object already on the list? Am i setting a new timeout without deleting the previous one? + // Adding an object that might already be in the list? + // Or, am i setting a new timeout without deleting the previous one? + // I can have iTickOld != 0 but not be in the list in some cases, like when duping an obj with an active timer. EXC_SET_BLOCK("Remove"); _RemoveTimedObject(pTimedObject); } @@ -284,7 +276,7 @@ void CWorldTicker::DelTimedObject(CTimedObject* pTimedObject) { #ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested deletion of CTimedObj, but Timeout is 0, so it shouldn't be in the list, or just queued to be removed.\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested deletion of CTimedObj, but Timeout is 0, so it shouldn't be in the list, or just queued to be removed.\n", (void*)pTimedObject); # endif const auto itEntryInRemoveList = std::find( @@ -294,10 +286,8 @@ void CWorldTicker::DelTimedObject(CTimedObject* pTimedObject) if (itEntryInRemoveList != _vecWorldObjsEraseRequests.end()) { # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: though, found it already in the removal list, so it's fine..\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: though, found it already in the removal list, so it's fine..\n", (void*)pTimedObject); # endif - - //ASSERT(false); return; } @@ -315,7 +305,7 @@ void CWorldTicker::DelTimedObject(CTimedObject* pTimedObject) } # ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE else - g_Log.EventDebug("[WorldTicker][%p] WARN: (rightfully) i haven't found it in the list.\n", (void*)pTimedObject); + g_Log.EventDebug("[WorldTicker][%p] INFO: (rightfully) i haven't found it in the list.\n", (void*)pTimedObject); # endif #endif return; @@ -350,11 +340,9 @@ void CWorldTicker::_InsertCharTicking(const int64 iTickNext, CChar* pChar) fnFindEntryByChar); if (_vecPeriodicCharsAddRequests.end() != itEntryInAddList) { -#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE g_Log.EventDebug("[WorldTicker][%p] WARN: Periodic char insertion into ticking list already requested " "(requesting tick %" PRId64 ", previous requested tick %" PRId64 ").\n", (void*)pChar, iTickNext, itEntryInAddList->first); -#endif ASSERT(false); return; // Already requested the addition. @@ -366,10 +354,7 @@ void CWorldTicker::_InsertCharTicking(const int64 iTickNext, CChar* pChar) pChar); if (_vecPeriodicCharsEraseRequests.end() != itEntryInEraseList) { -# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE g_Log.EventDebug("[WorldTicker][%p] WARN: Stopped insertion attempt of a CChar which removal from periodic ticking list has been requested!\n", (void*)pChar); -# endif - ASSERT(false); return; // Already requested the removal. } @@ -414,7 +399,7 @@ bool CWorldTicker::_RemoveCharTicking(CChar* pChar) pChar); if (_vecPeriodicCharsEraseRequests.end() != itEntryInRemoveList) { -# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE +# ifdef DEBUG_CCHAR_PERIODIC_TICKING g_Log.EventDebug("[WorldTicker][%p] INFO: TickingPeriodicChar erasure from ticking list already requested.\n", (void*)pChar); # endif @@ -451,9 +436,7 @@ bool CWorldTicker::_RemoveCharTicking(CChar* pChar) // Check if it's in the ticking list. if (itEntryInTickList == _mCharTickList.end()) { -# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE g_Log.EventDebug("[WorldTicker][%p] WARN: Requested TickingPeriodicChar removal from ticking list, but not found.\n", (void*)pChar); -# endif ASSERT(false); return false; @@ -462,9 +445,7 @@ bool CWorldTicker::_RemoveCharTicking(CChar* pChar) if (_vecPeriodicCharsEraseRequests.end() != itEntryInRemoveList) { // I have already requested to remove this from the main ticking list. -# ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE g_Log.EventDebug("[WorldTicker][%p] WARN: TickingPeriodicChar removal from the main ticking list already requested.\n", (void*)pChar); -# endif ASSERT(itEntryInAddList == _vecPeriodicCharsAddRequests.end()); return false; // Already requested the removal. @@ -474,8 +455,8 @@ bool CWorldTicker::_RemoveCharTicking(CChar* pChar) if (itEntryInTickList == _mCharTickList.end()) { // Not found. The object might have a timeout while being in a non-tickable state, so it isn't in the list. -#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested erasure of TimedObject in mWorldTickList, but it wasn't found.\n", (void*)pChar); +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested erasure of TimedObject in mWorldTickList, but it wasn't found there.\n", (void*)pChar); #endif return false; } @@ -517,17 +498,8 @@ void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) if (iTickNext == iTickOld) { -/* -#ifdef _DEBUG - auto it = std::find_if(_mCharTickList.begin(), _mCharTickList.end(), - [pChar](const std::pair& elem) { - return elem.second == pChar; - }); - DEBUG_ASSERT(it == _mCharTickList.end()); -#endif -*/ -#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Stop, tickold == ticknext.\n", (void*)pChar); +#ifdef DEBUG_CCHAR_PERIODIC_TICKING + g_Log.EventDebug("[WorldTicker][%p] INFO: Do not add (again) char periodic tick, tickold == ticknext.\n", (void*)pChar); #endif return; } @@ -539,12 +511,7 @@ void CWorldTicker::AddCharTicking(CChar* pChar, bool fNeedsLock) { // Adding an object already on the list? Am i setting a new timeout without deleting the previous one? EXC_SET_BLOCK("Remove"); - const bool fRet = _RemoveCharTicking(pChar); - UnreferencedParameter(fRet); - -#ifdef DEBUG_CCHAR_PERIODIC_TICKING - ASSERT(fRet); -#endif + _RemoveCharTicking(pChar); } EXC_SET_BLOCK("Insert"); @@ -580,7 +547,7 @@ void CWorldTicker::DelCharTicking(CChar* pChar, bool fNeedsLock) { #ifdef DEBUG_CCHAR_PERIODIC_TICKING # ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: Requested deletion of Periodic char, but Timeout is 0. It shouldn't be in the list, or just queued to be removed.\n", (void*)pChar); + g_Log.EventDebug("[WorldTicker][%p] INFO: Requested deletion of Periodic char, but Timeout is 0. It shouldn't be in the list, or just queued to be removed.\n", (void*)pChar); # endif auto fnFindEntryByChar = [pChar](const TickingPeriodicCharEntry& entry) noexcept { return entry.second == pChar; @@ -593,10 +560,9 @@ void CWorldTicker::DelCharTicking(CChar* pChar, bool fNeedsLock) if (itEntryInEraseList != _vecPeriodicCharsEraseRequests.end()) { # ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: though, found it already in the removal list, so it's fine..\n", (void*)pChar); + g_Log.EventDebug("[WorldTicker][%p] INFO: though, found it already in the removal list, so it's fine..\n", (void*)pChar); # endif - //ASSERT(false); return; } @@ -615,7 +581,7 @@ void CWorldTicker::DelCharTicking(CChar* pChar, bool fNeedsLock) } # ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE - g_Log.EventDebug("[WorldTicker][%p] WARN: (rightfully) i haven't found it in any list.\n", (void*)pChar); + g_Log.EventDebug("[WorldTicker][%p] INFO: (rightfully) i haven't found it in any list.\n", (void*)pChar); # endif #endif return; @@ -629,7 +595,7 @@ void CWorldTicker::DelCharTicking(CChar* pChar, bool fNeedsLock) void CWorldTicker::AddObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static { -#ifdef DEBUG_STATUSUPDATES +#ifdef DEBUG_STATUSUPDATES_VERBOSE g_Log.EventDebug("[%p] INFO: Trying to add CObjBase to the status update list.\n", (void*)pObj); #endif EXC_TRY("AddObjStatusUpdate"); @@ -650,16 +616,15 @@ void CWorldTicker::AddObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static ); if (_ObjStatusUpdates.end() != itDuplicate) { -#ifdef DEBUG_STATUSUPDATES - g_Log.EventDebug("[%p] WARN: Trying to add status update for duplicate CObjBase.\n", (void*)pObj); - ASSERT(false); +#ifdef DEBUG_STATUSUPDATES_VERBOSE + g_Log.EventDebug("[%p] WARN: Trying to add status update for duplicate CObjBase. Blocked.\n", (void*)pObj); #endif return; } _ObjStatusUpdates.emplace_back(pObj); -#ifdef DEBUG_STATUSUPDATES +#ifdef DEBUG_STATUSUPDATES_VERBOSE g_Log.EventDebug("[%p] INFO: Done adding CObjBase to the status update list.\n", (void*)pObj); #endif } @@ -669,7 +634,7 @@ void CWorldTicker::AddObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static void CWorldTicker::DelObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static { -#ifdef DEBUG_STATUSUPDATES +#ifdef DEBUG_STATUSUPDATES_VERBOSE g_Log.EventDebug("[%p] INFO: Trying to remove CObjBase from the status update list.\n", (void*)pObj); #endif EXC_TRY("DelObjStatusUpdate"); @@ -689,8 +654,8 @@ void CWorldTicker::DelObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static ); if (_ObjStatusUpdates.end() == itMissing) { -#ifdef DEBUG_STATUSUPDATES - g_Log.EventDebug("[%p] WARN: Requested erasure of CObjBase from the status update list, but it wasn't found.\n", (void*)pObj); +#ifdef DEBUG_STATUSUPDATES_VERBOSE + g_Log.EventDebug("[%p] INFO: Requested erasure of CObjBase from the status update list, but it wasn't found there.\n", (void*)pObj); #endif return; } @@ -702,14 +667,15 @@ void CWorldTicker::DelObjStatusUpdate(CObjBase* pObj, bool fNeedsLock) // static ); if (_vecObjStatusUpdateEraseRequested.end() != itRepeated) { -#ifdef DEBUG_STATUSUPDATES - g_Log.EventDebug("WARN [%p]: CObjBase erasure from the status update list already requested.\n", (void*)pObj); +#ifdef DEBUG_STATUSUPDATES_VERBOSE + // It can happen and it's legit. Example: we call this method via CObjBase::Delete and ~CObjBase. + g_Log.EventDebug("INFO [%p]: CObjBase erasure from the status update list already requested.\n", (void*)pObj); #endif return; } _vecObjStatusUpdateEraseRequested.emplace_back(pObj); -#ifdef DEBUG_STATUSUPDATES +#ifdef DEBUG_STATUSUPDATES_VERBOSE g_Log.EventDebug("[%p] INFO: Done adding CObjBase to the status update remove buffer.\n", (void*)pObj); #endif } @@ -730,14 +696,13 @@ static void sortedVecRemoveElementsByIndices(std::vector& vecMain, const std: const size_t sz = vecMain.size(); #ifdef DEBUG_LIST_OPS - ASSERT(std::is_sorted(vecMain.begin(), vecMain.end())); - ASSERT(std::is_sorted(vecIndicesToRemove.begin(), vecIndicesToRemove.end())); - // Check that those sorted vectors do not have duplicated values. - ASSERT(std::adjacent_find(vecMain.begin(), vecMain.end()) == vecMain.end()); - ASSERT(std::adjacent_find(vecIndicesToRemove.begin(), vecIndicesToRemove.end()) == vecIndicesToRemove.end()); - + ASSERT(sl::ContainerIsSorted(vecMain)); + ASSERT(sl::ContainerIsSorted(vecIndicesToRemove)); + ASSERT(!sl::SortedContainerHasDuplicates(vecMain)); + ASSERT(!sl::SortedContainerHasDuplicates(vecIndicesToRemove)); //g_Log.EventDebug("Starting sortedVecRemoveElementsByIndices.\n"); #endif + // Copy the original vector to check against later std::vector originalVecMain = vecMain; @@ -757,12 +722,12 @@ static void sortedVecRemoveElementsByIndices(std::vector& vecMain, const std: --itRemoveFirst; } -#ifdef DEBUG_LIST_OPS /* +#ifdef DEBUG_LIST_OPS g_Log.EventDebug("Removing contiguous indices: %" PRIuSIZE_T " to %" PRIuSIZE_T " (total sizes vecMain: %" PRIuSIZE_T ", vecIndices: %" PRIuSIZE_T ").\n", *itRemoveFirst, *itRemoveLast, vecMain.size(), vecIndicesToRemove.size()); - */ #endif + */ // Once we find a contiguous block, we erase that block from vecMain. auto itRemoveLastPast = (*itRemoveLast == vecMain.size() - 1) ? vecMain.end() : (vecMain.begin() + *itRemoveLast + 1); @@ -776,8 +741,10 @@ static void sortedVecRemoveElementsByIndices(std::vector& vecMain, const std: ASSERT(index < originalVecMain.size()); ASSERT(std::find(vecMain.begin(), vecMain.end(), originalVecMain[index]) == vecMain.end()); } +# ifdef DEBUG_LIST_OPS_VERBOSE g_Log.EventDebug("Sizes: new vec %" PRIuSIZE_T ", old vec %" PRIuSIZE_T ", remove vec %" PRIuSIZE_T ".\n", vecMain.size(), sz, vecIndicesToRemove.size()); +# endif #endif UnreferencedParameter(sz); @@ -973,6 +940,7 @@ static void unsortedVecDifference( } }*/ + // TODO: maybe optimize this algorithm. // Iterate through vecMain, copying elements that are not in vecToRemove for (auto itMain = vecMain.begin(); itMain != vecMain.end(); ++itMain) { // Perform a linear search for the current element's pointer in vecToRemove @@ -989,7 +957,7 @@ static void unsortedVecDifference( // Copy any remaining elements in vecMain after the last found element vecElemBuffer.insert(vecElemBuffer.end(), itCopyFromThis, vecMain.end()); -#ifdef DEBUG_LIST_OPS +#ifdef DEBUG_LIST_OPS_VERBOSE g_Log.EventDebug("Sizes: new vec %" PRIuSIZE_T ", old vec %" PRIuSIZE_T ", remove vec %" PRIuSIZE_T ".\n", vecElemBuffer.size(), vecMain.size(), vecToRemove.size()); @@ -1012,8 +980,8 @@ static void sortedVecRemoveAddQueued( ) { #ifdef DEBUG_LIST_OPS - ASSERT(std::is_sorted(vecMain.begin(), vecMain.end())); - ASSERT(std::adjacent_find(vecMain.begin(), vecMain.end()) == vecMain.end()); // no duplicate values + ASSERT(sl::ContainerIsSorted(vecMain)); + ASSERT(!sl::SortedContainerHasDuplicates(vecMain)); #endif //EXC_TRY("vecRemoveAddQueued"); @@ -1022,15 +990,16 @@ static void sortedVecRemoveAddQueued( std::sort(vecToRemove.begin(), vecToRemove.end()); #ifdef DEBUG_LIST_OPS - ASSERT(std::adjacent_find(vecToAdd.begin(), vecToAdd.end()) == vecToAdd.end()); // no duplicate values - ASSERT(std::adjacent_find(vecToRemove.begin(), vecToRemove.end()) == vecToRemove.end()); // no duplicate values + //ASSERT(sl::ContainerIsSorted(vecMain)); + ASSERT(!sl::SortedContainerHasDuplicates(vecMain)); #endif //EXC_SET_BLOCK("Ordered remove"); if (!vecToRemove.empty()) { - if (vecMain.empty()) + if (vecMain.empty()) { ASSERT(false); // Shouldn't ever happen. + } // TODO: test and benchmark if the approach of the above function (sortedVecRemoveElementsInPlace) might be faster. vecElemBuffer.clear(); @@ -1038,15 +1007,16 @@ static void sortedVecRemoveAddQueued( unsortedVecDifference(vecMain, vecToRemove, vecElemBuffer); #ifdef DEBUG_LIST_OPS - for (auto& elem : vecToRemove) { + for (auto& elem : vecToRemove) + { auto it = std::find_if(vecElemBuffer.begin(), vecElemBuffer.end(), [elem](auto &rhs) {return elem == rhs.second;}); UnreferencedParameter(it); ASSERT (it == vecElemBuffer.end()); } ASSERT(vecElemBuffer.size() == vecMain.size() - vecToRemove.size()); - ASSERT(std::is_sorted(vecElemBuffer.begin(), vecElemBuffer.end())); - ASSERT(std::adjacent_find(vecElemBuffer.begin(), vecElemBuffer.end()) == vecElemBuffer.end()); // no duplicate values + ASSERT(sl::ContainerIsSorted(vecElemBuffer)); + ASSERT(!sl::SortedContainerHasDuplicates(vecElemBuffer)); #endif vecMain.swap(vecElemBuffer); @@ -1054,10 +1024,6 @@ static void sortedVecRemoveAddQueued( //vecMain = std::move(vecElemBuffer); vecElemBuffer.clear(); vecToRemove.clear(); - -#ifdef DEBUG_LIST_OPS - g_Log.EventDebug("[GLOBAL] STATUS: Nonempty tick list remove buffer processed.\n"); -#endif } //EXC_SET_BLOCK("Mergesort"); @@ -1073,8 +1039,8 @@ static void sortedVecRemoveAddQueued( #ifdef DEBUG_LIST_OPS ASSERT(vecElemBuffer.size() == vecMain.size() + vecToAdd.size()); - ASSERT(std::is_sorted(vecElemBuffer.begin(), vecElemBuffer.end())); - ASSERT(std::adjacent_find(vecElemBuffer.begin(), vecElemBuffer.end()) == vecElemBuffer.end()); // no duplicate values + ASSERT(sl::ContainerIsSorted(vecElemBuffer)); + ASSERT(!sl::SortedContainerHasDuplicates(vecElemBuffer)); #endif vecMain.swap(vecElemBuffer); @@ -1082,7 +1048,7 @@ static void sortedVecRemoveAddQueued( vecElemBuffer.clear(); vecToAdd.clear(); -#ifdef DEBUG_LIST_OPS +#ifdef DEBUG_LIST_OPS_VERBOSE g_Log.EventDebug("[GLOBAL] STATUS: Nonempty tick list add buffer processed.\n"); #endif } @@ -1164,25 +1130,17 @@ void CWorldTicker::Tick() // Need here another scope to give the right lifetime to the unique_lock. #if MT_ENGINES std::unique_lock lock(_mWorldTickList.MT_CMUTEX); -#endif -#ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_mWorldTickList)); - ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); #endif { // New requests done during the world loop. EXC_TRYSUB("Update main list"); -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE g_Log.EventDebug("[GLOBAL] STATUS: Updating WorldTickList.\n"); #endif sortedVecRemoveAddQueued(_mWorldTickList, _vecWorldObjsEraseRequests, _vecWorldObjsAddRequests, _vecWorldObjsElementBuffer); EXC_CATCHSUB(""); } -#ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_mWorldTickList)); - ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); -#endif // Need here a new, inner scope to get rid of EXC_TRYSUB variables if (!_mWorldTickList.empty()) @@ -1198,7 +1156,7 @@ void CWorldTicker::Tick() while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) { CTimedObject* pTimedObj = itMap->second; -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE g_Log.EventDebug("Checking if CTimedObject %p should tick.\n", reinterpret_cast(pTimedObj)); #endif if (pTimedObj->_IsTimerSet() && pTimedObj->_CanTick()) @@ -1211,7 +1169,7 @@ void CWorldTicker::Tick() continue; } -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE g_Log.EventDebug("Yes it should (%p).\n", reinterpret_cast(pTimedObj)); #endif _vecGenericObjsToTick.emplace_back(static_cast(pTimedObj)); @@ -1230,17 +1188,16 @@ void CWorldTicker::Tick() } #ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_vecIndexMiscBuffer)); - ASSERT(!sl::SortedContainerHasDuplicates(_vecIndexMiscBuffer)); ASSERT(!sl::UnsortedContainerHasDuplicates(_vecGenericObjsToTick)); #endif - { EXC_TRYSUB("Delete from List"); sortedVecRemoveElementsByIndices(_mWorldTickList, _vecIndexMiscBuffer); #ifdef DEBUG_LIST_OPS - for (void* obj : _vecGenericObjsToTick) { + // Ensure + for (void* obj : _vecGenericObjsToTick) + { auto itit = std::find_if(_mWorldTickList.begin(), _mWorldTickList.end(), [obj](TickingTimedObjEntry const& lhs) { return static_cast(obj) == lhs.second; @@ -1252,10 +1209,6 @@ void CWorldTicker::Tick() EXC_CATCHSUB(""); } -#ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_mWorldTickList)); - ASSERT(!sl::SortedContainerHasDuplicates(_mWorldTickList)); -#endif // Done working with _mWorldTickList, we don't need the lock from now on. @@ -1270,9 +1223,6 @@ void CWorldTicker::Tick() CTimedObject* pTimedObj = static_cast(pObjVoid); pTimedObj->_ClearTimeout(); -//#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING -// g_Log.EventDebug("Ticking CTimedObject %p.\n", reinterpret_cast(pTimedObj)); -//#endif #if MT_ENGINES std::unique_lock lockTimeObj(pTimedObj->MT_CMUTEX); #endif @@ -1386,7 +1336,6 @@ void CWorldTicker::Tick() } _vecGenericObjsToTick.clear(); - //g_Log.EventDebug("END ctimedobj section.\n"); // ---- @@ -1403,15 +1352,10 @@ void CWorldTicker::Tick() std::unique_lock lock(_mCharTickList.MT_CMUTEX); #endif { -#ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_mCharTickList)); - ASSERT(!sl::SortedContainerHasDuplicates(_mCharTickList)); -#endif - // New requests done during the world loop. EXC_TRYSUB("Update main list"); -#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING +#ifdef DEBUG_CTIMEDOBJ_TIMED_TICKING_VERBOSE g_Log.EventDebug("[GLOBAL] STATUS: Updating CharTickList.\n"); #endif sortedVecRemoveAddQueued(_mCharTickList, _vecPeriodicCharsEraseRequests, _vecPeriodicCharsAddRequests, _vecPeriodicCharsElementBuffer); @@ -1424,7 +1368,7 @@ void CWorldTicker::Tick() { { EXC_TRYSUB("Selection"); -#ifdef DEBUG_CCHAR_PERIODIC_TICKING +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE //g_Log.EventDebug("Start looping through char periodic ticks.\n"); #endif _vecIndexMiscBuffer.clear(); @@ -1436,7 +1380,7 @@ void CWorldTicker::Tick() while ((itMap != itMapEnd) && (iCurTime > (iTime = itMap->first))) { CChar* pChar = itMap->second; -#ifdef DEBUG_CCHAR_PERIODIC_TICKING +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE g_Log.EventDebug("Executing char periodic tick: %p. Registered time: %" PRId64 ". pChar->_iTimePeriodicTick: %" PRId64 "\n", (void*)pChar, itMap->first, pChar->_iTimePeriodicTick); ASSERT(itMap->first == pChar->_iTimePeriodicTick); @@ -1466,8 +1410,8 @@ void CWorldTicker::Tick() ASSERT(!sl::SortedContainerHasDuplicates(_vecIndexMiscBuffer)); #endif -#ifdef DEBUG_CCHAR_PERIODIC_TICKING - //g_Log.EventDebug("Done looping through char periodic ticks. Need to tick n %" PRIuSIZE_T " objs.\n", _vecGenericObjsToTick.size()); +#ifdef DEBUG_CCHAR_PERIODIC_TICKING_VERBOSE + g_Log.EventDebug("Done looping through char periodic ticks. Need to tick n %" PRIuSIZE_T " objs.\n", _vecGenericObjsToTick.size()); #endif } @@ -1480,11 +1424,6 @@ void CWorldTicker::Tick() _vecIndexMiscBuffer.clear(); } -#ifdef DEBUG_LIST_OPS - ASSERT(sl::ContainerIsSorted(_mCharTickList)); - ASSERT(!sl::SortedContainerHasDuplicates(_mCharTickList)); -#endif - // Done working with _mCharTickList, we don't need the lock from now on. } #ifdef DEBUG_CCHAR_PERIODIC_TICKING diff --git a/src/game/CWorldTicker.h b/src/game/CWorldTicker.h index a0d02a413..ef6fba8b6 100644 --- a/src/game/CWorldTicker.h +++ b/src/game/CWorldTicker.h @@ -8,42 +8,6 @@ #include "CTimedFunctionHandler.h" #include "CTimedObject.h" -/* - * #include -*/ - -/* -//--- Include phmap.h -#ifdef ADDRESS_SANITIZER - #define MYASAN_ -#endif - -#ifdef _WIN32 -// #define MYSRWLOCK_ - #undef SRWLOCK_INIT -#endif -#ifdef __GNUC__ - #pragma GCC diagnostic push - #pragma GCC diagnostic ignored "-Wshift-count-overflow" -#endif - -// TODO: undef is TEMPORARY !! There's a bug that needs to be solved -#undef ADDRESS_SANITIZER -#include - -#ifdef MYASAN_ - #define ADDRESS_SANITIZER -#endif -//#ifdef MYSRWLOCK_ -//# define SRWLOCK_INIT -//#endif - -#ifdef __GNUC__ - #pragma GCC diagnostic pop -#endif -//--- End of phmap.h inclusion -*/ - class CObjBase; class CChar; @@ -72,9 +36,6 @@ class CWorldTicker }; // Calls to OnTickStatusUpdate. Periodically send updated infos to the clients. - //struct StatusUpdatesList : public phmap::parallel_flat_hash_set - //struct StatusUpdatesList : public fc::flat_set - //struct StatusUpdatesList : public std::unordered_set struct StatusUpdatesList : public std::vector { MT_CMUTEX_DEF; diff --git a/src/game/chars/CCharAct.cpp b/src/game/chars/CCharAct.cpp index 384e4b3c3..754242f2d 100644 --- a/src/game/chars/CCharAct.cpp +++ b/src/game/chars/CCharAct.cpp @@ -5796,7 +5796,8 @@ void CChar::_GoAwake() CWorldTickingList::AddCharPeriodic(this, false); - _SetTimeout(g_Rand.GetValFast(1 * MSECS_PER_SEC)); // make it tick randomly in the next sector, so all awaken NPCs get a different tick time. + if (!_IsTimerSet()) + _SetTimeout(g_Rand.GetValFast(1 * MSECS_PER_SEC)); // make it tick randomly in the next sector, so all awaken NPCs get a different tick time. } void CChar::_GoSleep() diff --git a/src/game/chars/CCharStat.cpp b/src/game/chars/CCharStat.cpp index 9bd654df9..0f5528875 100644 --- a/src/game/chars/CCharStat.cpp +++ b/src/game/chars/CCharStat.cpp @@ -167,7 +167,10 @@ void CChar::Stat_SetVal( STAT_TYPE i, ushort uiVal ) m_Stat[i].m_val = uiVal; if ((i == STAT_STR) && (uiVal == 0)) - { // Ensure this char will tick and die + { + // Ensure this char will tick and die. + // It's a redundant operation and safety check, as very probably it already is in the periodic ticking list. + // TODO: add methods to assert that a char/cobjbase/etc is contained in a specific ticking list. CWorldTickingList::AddCharPeriodic(this, false); } } From 28f6013ebc9290a72cf0be04ef05f3a64ac6a7fb Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 18:33:48 +0100 Subject: [PATCH 08/13] Moving pre-allocated string buffers from static to dynamic allocation, and reduce the amount of pre-allocated string buffers. --- src/common/sphere_library/CSString.cpp | 6 +- src/common/sphere_library/CSString.h | 2 +- src/common/sphere_library/sstringobjs.cpp | 28 ++++++--- src/common/sphere_library/sstringobjs.h | 19 +++--- src/sphere/threads.cpp | 77 +++++++++++++---------- 5 files changed, 74 insertions(+), 58 deletions(-) diff --git a/src/common/sphere_library/CSString.cpp b/src/common/sphere_library/CSString.cpp index a0cc3b665..009bf68bb 100644 --- a/src/common/sphere_library/CSString.cpp +++ b/src/common/sphere_library/CSString.cpp @@ -9,7 +9,7 @@ #include // for vsnprintf #endif -#define STRING_DEFAULT_SIZE 42 +#define CSTRING_DEFAULT_SIZE 42 //#define DEBUG_STRINGS #ifdef DEBUG_STRINGS @@ -67,7 +67,7 @@ CSString::CSString(const CSString& s) : // private void CSString::Init() { - m_iMaxLength = STRING_DEFAULT_SIZE; + m_iMaxLength = CSTRING_DEFAULT_SIZE; #ifdef DEBUG_STRINGS gMemAmount += m_iMaxLength; #endif @@ -117,7 +117,7 @@ int CSString::Resize(int iNewLength, bool fPreciseSize) } else { - //m_iMaxLength = iNewLength + (STRING_DEFAULT_SIZE >> 1); // Probably too much... + //m_iMaxLength = iNewLength + (CSTRING_DEFAULT_SIZE >> 1); // Probably too much... m_iMaxLength = (iNewLength * 3) >> 1; // >> 2 is equal to doing / 2 } diff --git a/src/common/sphere_library/CSString.h b/src/common/sphere_library/CSString.h index 26aedb428..8c7bb0dc9 100644 --- a/src/common/sphere_library/CSString.h +++ b/src/common/sphere_library/CSString.h @@ -30,7 +30,7 @@ class CSString /** * @brief Initializes internal data. * - * Allocs STRING_DEFAULT_SIZE by default. If DEBUG_STRINGS setted, updates statistical information (total memory allocated). + * Allocs CSTRING_DEFAULT_SIZE by default. If DEBUG_STRINGS setted, updates statistical information (total memory allocated). */ void Init(); diff --git a/src/common/sphere_library/sstringobjs.cpp b/src/common/sphere_library/sstringobjs.cpp index 765aeb009..e07785026 100644 --- a/src/common/sphere_library/sstringobjs.cpp +++ b/src/common/sphere_library/sstringobjs.cpp @@ -3,28 +3,35 @@ #include "sstringobjs.h" -#define MAX_TEMP_LINES_NO_CONTEXT 512 -#define STRING_DEFAULT_SIZE 40 - +// temporary string storage +#define NO_CONTEXT_TEMPSTRING_MAX_LINES 256 +//#define STRINGOBJ_DEFAULT_SIZE 48 struct TemporaryStringUnsafeStateHolder { // NOT thread safe size_t m_tempPosition; - char m_tempStrings[MAX_TEMP_LINES_NO_CONTEXT][THREAD_STRING_LENGTH]; + std::unique_ptr m_tempStrings; static TemporaryStringUnsafeStateHolder& get() { static TemporaryStringUnsafeStateHolder instance; return instance; } -}; +private: + TemporaryStringUnsafeStateHolder() + { + m_tempStrings = std::make_unique(NO_CONTEXT_TEMPSTRING_MAX_LINES * THREAD_STRING_LENGTH); + } +}; +[[nodiscard]] static tchar* getUnsafeStringBuffer() noexcept { auto& unsafe_state_holder = TemporaryStringUnsafeStateHolder::get(); - tchar* unsafe_buffer = unsafe_state_holder.m_tempStrings[unsafe_state_holder.m_tempPosition++]; - if (unsafe_state_holder.m_tempPosition >= MAX_TEMP_LINES_NO_CONTEXT) + tchar* unsafe_buffer = &(unsafe_state_holder.m_tempStrings[unsafe_state_holder.m_tempPosition * THREAD_STRING_LENGTH]); + unsafe_state_holder.m_tempPosition += 1; + if (unsafe_state_holder.m_tempPosition >= NO_CONTEXT_TEMPSTRING_MAX_LINES) { unsafe_state_holder.m_tempPosition = 0; } @@ -32,6 +39,7 @@ static tchar* getUnsafeStringBuffer() noexcept } +[[nodiscard]] tchar* Str_GetTemp() noexcept { AbstractThread *pThreadState = ThreadHolder::get().current(); @@ -184,17 +192,17 @@ size_t AbstractString::lastIndexOf(char c) const noexcept /* HeapString::HeapString() { - resize(STRING_DEFAULT_SIZE); + resize(STRINGOBJ_DEFAULT_SIZE); } HeapString::~HeapString() { - destroyHeap(); + destroyHeap(); } void HeapString::resize(size_t newLength) { - ensureLengthHeap(newLength); + ensureLengthHeap(newLength); } */ diff --git a/src/common/sphere_library/sstringobjs.h b/src/common/sphere_library/sstringobjs.h index fddd7e55e..86e591715 100644 --- a/src/common/sphere_library/sstringobjs.h +++ b/src/common/sphere_library/sstringobjs.h @@ -8,26 +8,21 @@ #include "../common.h" -// temporary string storage -#define THREAD_STRING_STORAGE 4096 -#define THREAD_TSTRING_STORAGE 2048 -#define THREAD_STRING_LENGTH 4096 - //-- -tchar* Str_GetTemp() noexcept; +#define THREAD_STRING_LENGTH 4096 -#if __cplusplus >= 202002L -// C++20 (and later) code +[[nodiscard]] consteval -#else -constexpr -#endif -size_t Str_TempLength() noexcept { +size_t Str_TempLength() noexcept +{ return (size_t)(THREAD_STRING_LENGTH); } +[[nodiscard]] +tchar* Str_GetTemp() noexcept; + //-- // Base abstract class for strings, provides basic information of what should be available diff --git a/src/sphere/threads.cpp b/src/sphere/threads.cpp index aacad6db1..096b03f99 100644 --- a/src/sphere/threads.cpp +++ b/src/sphere/threads.cpp @@ -1,5 +1,7 @@ -// this thing is somehow required to be able to initialise OLE -#define _WIN32_DCOM +#ifdef _WIN32 + // this thing is somehow required to be able to initialise OLE + #define _WIN32_DCOM +#endif #include "../common/basic_threading.h" #include "../common/CException.h" @@ -12,7 +14,7 @@ #include #include #elif !defined(_BSD) && !defined(__APPLE__) - #include + #include // to set thread name #endif #include @@ -26,32 +28,42 @@ // number of milliseconds to wait for a thread to close #define THREADJOIN_TIMEOUT 60000 +// temporary string storage +#define THREAD_TEMPSTRING_C_STORAGE 2048 +#define THREAD_TEMPSTRING_OBJ_STORAGE 1024 + struct TemporaryStringThreadSafeStateHolder { - // Normal Buffer + // C-style string Buffer (char array) SimpleMutex g_tmpStringMutex; - std::atomic g_tmpStringIndex = 0; - char g_tmpStrings[THREAD_TSTRING_STORAGE][THREAD_STRING_LENGTH]; + std::atomic g_tmpStringIndex; + std::unique_ptr g_tmpStrings; // TemporaryString Buffer SimpleMutex g_tmpTemporaryStringMutex; - std::atomic g_tmpTemporaryStringIndex = 0; - - + std::atomic g_tmpTemporaryStringIndex; struct TemporaryStringStorage { char m_buffer[THREAD_STRING_LENGTH]; char m_state; - } g_tmpTemporaryStringStorage[THREAD_STRING_STORAGE]; + }; + std::unique_ptr g_tmpTemporaryStringStorage; +private: + TemporaryStringThreadSafeStateHolder() : + g_tmpStringIndex(0), g_tmpTemporaryStringIndex(0) + { + g_tmpStrings = std::make_unique(THREAD_TEMPSTRING_C_STORAGE * THREAD_STRING_LENGTH); + g_tmpTemporaryStringStorage = std::make_unique(THREAD_TEMPSTRING_OBJ_STORAGE); + } public: - static TemporaryStringThreadSafeStateHolder* get() noexcept - { - static TemporaryStringThreadSafeStateHolder instance{}; - return &instance; - } + static TemporaryStringThreadSafeStateHolder& get() noexcept + { + static TemporaryStringThreadSafeStateHolder instance; + return instance; + } }; @@ -786,18 +798,19 @@ AbstractSphereThread::~AbstractSphereThread() char *AbstractSphereThread::allocateBuffer() noexcept { - auto* tsholder = TemporaryStringThreadSafeStateHolder::get(); - SimpleThreadLock stlBuffer(tsholder->g_tmpStringMutex); + auto& tsholder = TemporaryStringThreadSafeStateHolder::get(); + SimpleThreadLock stlBuffer(tsholder.g_tmpStringMutex); char * buffer = nullptr; - tsholder->g_tmpStringIndex += 1; + tsholder.g_tmpStringIndex += 1; - if (tsholder->g_tmpStringIndex >= THREAD_TSTRING_STORAGE ) + if (tsholder.g_tmpStringIndex >= THREAD_TEMPSTRING_C_STORAGE ) { - tsholder->g_tmpStringIndex = tsholder->g_tmpStringIndex % THREAD_TSTRING_STORAGE; + tsholder.g_tmpStringIndex = tsholder.g_tmpStringIndex % THREAD_TEMPSTRING_C_STORAGE; } - buffer = tsholder->g_tmpStrings[tsholder->g_tmpStringIndex]; + //buffer = tsholder->g_tmpStrings.get() + (tsholder->g_tmpStringIndex * THREAD_STRING_LENGTH); + buffer = &(tsholder.g_tmpStrings[tsholder.g_tmpStringIndex * THREAD_TEMPSTRING_C_STORAGE]); *buffer = '\0'; return buffer; @@ -807,24 +820,24 @@ static TemporaryStringThreadSafeStateHolder::TemporaryStringStorage * getThreadRawStringBuffer() { - auto* tsholder = TemporaryStringThreadSafeStateHolder::get(); - SimpleThreadLock stlBuffer(tsholder->g_tmpStringMutex); + auto& tsholder = TemporaryStringThreadSafeStateHolder::get(); + SimpleThreadLock stlBuffer(tsholder.g_tmpStringMutex); - int initialPosition = tsholder->g_tmpTemporaryStringIndex; + int initialPosition = tsholder.g_tmpTemporaryStringIndex; int index; for (;;) { - index = tsholder->g_tmpTemporaryStringIndex += 1; - if(tsholder->g_tmpTemporaryStringIndex >= THREAD_STRING_STORAGE ) + index = tsholder.g_tmpTemporaryStringIndex += 1; + if(tsholder.g_tmpTemporaryStringIndex >= THREAD_TEMPSTRING_OBJ_STORAGE ) { - const int inc = tsholder->g_tmpTemporaryStringIndex % THREAD_STRING_STORAGE; - tsholder->g_tmpTemporaryStringIndex = inc; + const int inc = tsholder.g_tmpTemporaryStringIndex % THREAD_TEMPSTRING_OBJ_STORAGE; + tsholder.g_tmpTemporaryStringIndex = inc; index = inc; } - if(tsholder->g_tmpTemporaryStringStorage[index].m_state == 0 ) + if(tsholder.g_tmpTemporaryStringStorage[index].m_state == 0 ) { - auto* store = &tsholder->g_tmpTemporaryStringStorage[index]; + auto* store = &(tsholder.g_tmpTemporaryStringStorage[index]); *store->m_buffer = '\0'; return store; } @@ -846,8 +859,8 @@ getThreadRawStringBuffer() void AbstractSphereThread::getStringBuffer(TemporaryString &string) noexcept { ADDTOCALLSTACK("alloc"); - auto* tsholder = TemporaryStringThreadSafeStateHolder::get(); - SimpleThreadLock stlBuffer(tsholder->g_tmpTemporaryStringMutex); + auto& tsholder = TemporaryStringThreadSafeStateHolder::get(); + SimpleThreadLock stlBuffer(tsholder.g_tmpTemporaryStringMutex); auto* store = getThreadRawStringBuffer(); string.init(store->m_buffer, &store->m_state); From 2f412edfdd3bb62a3863c8ff564a69f1cff72877 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 18:35:01 +0100 Subject: [PATCH 09/13] Reducing binary size on non-Windows OSes, by stripping unused symbols in non-debug builds after compilation. Issue 1345 --- CMakeLists.txt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 445f0334b..eb5d6a9ab 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -197,6 +197,11 @@ if(SINGLE_TARGET) #set_target_properties(spheresvr_release PROPERTIES CXX_EXTENSIONS OFF) #target_compile_features(spheresvr_release PUBLIC cxx_std_20) target_precompile_headers(spheresvr_release ${pch_options}) + + add_custom_command(TARGET spheresvr_release POST_BUILD + COMMAND ${CMAKE_STRIP} $ + COMMENT "Stripping executable" + ) endif() if(("${CMAKE_BUILD_TYPE}" STREQUAL "") OR (${CMAKE_BUILD_TYPE} MATCHES "(N|n?)ightly")) set(TARGETS ${TARGETS} spheresvr_nightly) @@ -210,6 +215,11 @@ if(SINGLE_TARGET) #set_target_properties(spheresvr_nightly PROPERTIES CXX_EXTENSIONS OFF) #target_compile_features(spheresvr_nightly PUBLIC cxx_std_20) target_precompile_headers(spheresvr_nightly ${pch_options}) + + add_custom_command(TARGET spheresvr_nightly POST_BUILD + COMMAND ${CMAKE_STRIP} $ + COMMENT "Stripping executable" + ) endif() if(("${CMAKE_BUILD_TYPE}" STREQUAL "") OR (${CMAKE_BUILD_TYPE} MATCHES "(D|d?)ebug")) set(TARGETS ${TARGETS} spheresvr_debug) @@ -233,6 +243,13 @@ else(SINGLE_TARGET) #set_target_properties(spheresvr PROPERTIES CXX_EXTENSIONS OFF) #target_compile_features(spheresvr PUBLIC cxx_std_20) target_precompile_headers(spheresvr ${pch_options}) + + # To test + #add_custom_command(TARGET spheresvr POST_BUILD + # $<$:COMMAND ${CMAKE_STRIP} $> + # $<$:COMMAND ${CMAKE_STRIP} $> + # COMMENT "Stripping executable" + #) endif(SINGLE_TARGET) # -------------------- From e3544bc084c1e809b3d2f5f756f20c23ba4d59d7 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 18:41:27 +0100 Subject: [PATCH 10/13] Fixed: @(Item)Step trigger not instantly firing. Issue #1366 --- src/game/chars/CCharAct.cpp | 2 +- src/game/chars/CCharNPCAct.cpp | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/game/chars/CCharAct.cpp b/src/game/chars/CCharAct.cpp index 754242f2d..e2e699eee 100644 --- a/src/game/chars/CCharAct.cpp +++ b/src/game/chars/CCharAct.cpp @@ -4912,7 +4912,7 @@ TRIGRET_TYPE CChar::CheckLocationEffects(bool fStanding) { if (_uiRecursingItemStep >= _kuiRecursingItemStepLimit) { - g_Log.EventError("Calling recursively @ITEMSTEP for more than %u times. Skipping trigger call.\n", _kuiRecursingStepLimit); + g_Log.EventError("Calling recursively @ITEMSTEP for more than %u times. Skipping trigger call.\n", _kuiRecursingItemStepLimit); } else { diff --git a/src/game/chars/CCharNPCAct.cpp b/src/game/chars/CCharNPCAct.cpp index 27f573784..84d51e238 100644 --- a/src/game/chars/CCharNPCAct.cpp +++ b/src/game/chars/CCharNPCAct.cpp @@ -577,8 +577,7 @@ int CChar::NPC_WalkToPoint( bool fRun ) CheckRevealOnMove(); EXC_SET_BLOCK("MoveToChar"); - //if (!MoveToChar(pMe, false, true)) - if (!MoveToChar(pMe, false, false)) + if (!MoveToChar(pMe, false, true)) return 2; EXC_SET_BLOCK("Move Update"); From 21428ff66caa473b8c343fc843200402040c868e Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 19:10:36 +0100 Subject: [PATCH 11/13] Fixed Issue #1362: Crash during startup garbage collector when spawned npc memory is mislinked. --- src/game/CWorld.cpp | 3 +++ src/game/items/CItemMemory.cpp | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/game/CWorld.cpp b/src/game/CWorld.cpp index fe7ba2d57..08c9d1f44 100644 --- a/src/game/CWorld.cpp +++ b/src/game/CWorld.cpp @@ -184,6 +184,9 @@ static lpctstr GetReasonForGarbageCode(int iCode = -1) noexcept break; case 0x4226: pStr = "Old Spawn memory item conversion"; + break; + case 0x4227: + pStr = "Old Spawn memory item conversion (mislinked)"; break; case 0xFFFF: diff --git a/src/game/items/CItemMemory.cpp b/src/game/items/CItemMemory.cpp index 5ef7ea9a5..2df3f81fe 100644 --- a/src/game/items/CItemMemory.cpp +++ b/src/game/items/CItemMemory.cpp @@ -104,8 +104,16 @@ int CItemMemory::FixWeirdness() dword dwFlags = GetHue(); if (dwFlags & MEMORY_LEGACY_ISPAWNED) { - CCSpawn *pSpawn = static_cast(m_uidLink.ItemFind()->GetComponent(COMP_SPAWN)); - if (pSpawn && pChar) + CItem *pSpawnItem = m_uidLink.ItemFind(); + if (!pSpawnItem) + { + iResultCode = 0x4227; + return iResultCode; + } + + CCSpawn *pSpawn = static_cast(pSpawnItem->GetComponent(COMP_SPAWN)); + ASSERT(pSpawn); + if (pSpawn) { pSpawn->AddObj(pChar->GetUID()); } From 805bc1682261c654d5038270a889f2621aa35adc Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 19:12:30 +0100 Subject: [PATCH 12/13] Fixed Issue #1307: Crash on boot if an NPC is destroyed in the middle of its initialization. --- src/game/CWorld.cpp | 7 +++++-- src/game/clients/CAccount.cpp | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/game/CWorld.cpp b/src/game/CWorld.cpp index 08c9d1f44..b7b4996b5 100644 --- a/src/game/CWorld.cpp +++ b/src/game/CWorld.cpp @@ -411,9 +411,12 @@ void CWorldThread::AddIdleObj(CSObjContRec* obj) void CWorldThread::ScheduleObjDeletion(CSObjContRec* obj) { - const auto servMode = g_Serv.GetServerMode(); - const bool fDestroy = (servMode == SERVMODE_Exiting || servMode == SERVMODE_Loading); // If the world is being destroyed, do not schedule the object for deletion but delete it right away. + const auto servMode = g_Serv.GetServerMode(); + // I can't destroy it while SERVMODE_Loading, because the script parser can't know (without creating a global state holder, TODO) that this + // object was deleted/destroyed. The object pointer will become invalid, and if something uses it, even for calling a method, Sphere will crash. + //const bool fDestroy = (servMode == SERVMODE_Exiting || servMode == SERVMODE_Loading); + const bool fDestroy = (servMode == SERVMODE_Exiting); if (fDestroy) { diff --git a/src/game/clients/CAccount.cpp b/src/game/clients/CAccount.cpp index 7e2027853..6e292947e 100644 --- a/src/game/clients/CAccount.cpp +++ b/src/game/clients/CAccount.cpp @@ -1062,7 +1062,7 @@ void CAccount::SetNewPassword( lpctstr pszPassword ) ADDTOCALLSTACK("CAccount::SetNewPassword"); if ( !pszPassword || !pszPassword[0] ) // no password given, auto-generate password { - static tchar const passwdChars[] = "ABCDEFGHJKLMNPQRTUVWXYZ2346789"; + static constexpr tchar passwdChars[] = "ABCDEFGHJKLMNPQRTUVWXYZ2346789"; int len = (int)strlen(passwdChars); int charsCnt = g_Rand.GetVal(4) + 6; // 6 - 10 chars if ( charsCnt > (MAX_ACCOUNT_PASSWORD_ENTER - 1) ) From b5a59945bd08e099d83f3893af870a109b64ac10 Mon Sep 17 00:00:00 2001 From: cbnolok Date: Thu, 6 Feb 2025 19:29:57 +0100 Subject: [PATCH 13/13] Moved fixed GCC/Clang compilation flags to separate cmake files. Fixed a CMake parsing error causing optimizations not being applied correctly to Nightly and Release build. --- CMakeLists.txt | 24 +---- cmake/CommonCompilerFlags.cmake | 22 +++++ cmake/CompilerFlagsBase.cmake | 98 +++++++++++++++++++ cmake/CompilerFlagsChecker.cmake | 37 +++---- .../include/Linux-Clang_common.inc.cmake | 36 ++++--- .../include/Linux-GNU_common.inc.cmake | 27 ++--- .../include/OSX-AppleClang_common.inc.cmake | 32 +++--- .../include/Windows-Clang_common.inc.cmake | 63 ++++++------ .../include/Windows-GNU_common.inc.cmake | 27 ++--- lib/lib_build_flags_common_c.cmake | 6 +- 10 files changed, 228 insertions(+), 144 deletions(-) create mode 100644 cmake/CommonCompilerFlags.cmake create mode 100644 cmake/CompilerFlagsBase.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index eb5d6a9ab..1cc439403 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -111,7 +111,7 @@ option(USE_UBSAN "Enable Undefined Behavior Sanitizer." FALSE) option(USE_LSAN "Enable LeakSanitizer." FALSE) option(USE_MSAN "Enable MemorySanitizer." FALSE) -set(ENABLED_SANITIZER CACHE INTERNAL BOOL FALSE) +set(ENABLED_SANITIZER FALSE CACHE INTERNAL BOOL "Am i using a sanitizer?") if(USE_ASAN OR USE_MSAN OR USE_LSAN OR USE_MSAN) set(ENABLED_SANITIZER TRUE) endif() @@ -244,7 +244,7 @@ else(SINGLE_TARGET) #target_compile_features(spheresvr PUBLIC cxx_std_20) target_precompile_headers(spheresvr ${pch_options}) - # To test + # To be tested #add_custom_command(TARGET spheresvr POST_BUILD # $<$:COMMAND ${CMAKE_STRIP} $> # $<$:COMMAND ${CMAKE_STRIP} $> @@ -255,25 +255,7 @@ endif(SINGLE_TARGET) # -------------------- message(STATUS) -include("${CMAKE_SOURCE_DIR}/cmake/CompilerFlagsChecker.cmake") - -# -------------------- - -if(USE_ASAN) - set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} ADDRESS_SANITIZER) -endif() -if(USE_UBSAN) - set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} UNDEFINED_BEHAVIOR_SANITIZER) -endif() -if(USE_LSAN) - set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} LEAK_SANITIZER) -endif() -if(USE_MSAN) - set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} MEMORY_SANITIZER) -endif() -if(ENABLED_SANITIZER) - set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} _SANITIZERS) -endif() +include("${CMAKE_SOURCE_DIR}/cmake/CommonCompilerFlags.cmake") # -------------------- diff --git a/cmake/CommonCompilerFlags.cmake b/cmake/CommonCompilerFlags.cmake new file mode 100644 index 000000000..2fae79e31 --- /dev/null +++ b/cmake/CommonCompilerFlags.cmake @@ -0,0 +1,22 @@ +set(list_explicit_compiler_options_all CACHE INTERNAL STRING) +set(list_explicit_linker_options_all CACHE INTERNAL STRING) +include("${CMAKE_SOURCE_DIR}/cmake/CompilerFlagsBase.cmake") +include("${CMAKE_SOURCE_DIR}/cmake/CompilerFlagsChecker.cmake") + +# -------------------- + +if(USE_ASAN) + set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} ADDRESS_SANITIZER) +endif() +if(USE_UBSAN) + set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} UNDEFINED_BEHAVIOR_SANITIZER) +endif() +if(USE_LSAN) + set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} LEAK_SANITIZER) +endif() +if(USE_MSAN) + set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} MEMORY_SANITIZER) +endif() +if(ENABLED_SANITIZER) + set(PREPROCESSOR_DEFS_EXTRA ${PREPROCESSOR_DEFS_EXTRA} _SANITIZERS) +endif() diff --git a/cmake/CompilerFlagsBase.cmake b/cmake/CompilerFlagsBase.cmake new file mode 100644 index 000000000..fddb2c124 --- /dev/null +++ b/cmake/CompilerFlagsBase.cmake @@ -0,0 +1,98 @@ +message(STATUS "Setting base compiler flags...") + +if(NOT MSVC) + # Compiler option flags (minimum). + list( + APPEND + compiler_options_base + -pthread + -fexceptions + -fnon-call-exceptions + -pipe + -ffast-math + # Place each function and variable into its own section, allowing the linker to remove unused ones. + #-ffunction-sections + #-fdata-sections + #-flto # Supported even by ancient compilers... also needed to benefit the most from the two flags above. + ) + list( + APPEND + compiler_options_warning_base + -Werror + -Wall + -Wextra + -Wpedantic + ) + + # Linker option flags (minimum). + list( + APPEND + linker_options_base + -pthread + -dynamic + #-flto + ) + + message(STATUS "Adding the following base compiler options: ${compiler_options_base}.") + message(STATUS "Adding the following base compiler warning options: ${compiler_options_warning_base}.") + message(STATUS "Adding the following base linker options: ${linker_options_base}.") + + #-- Store once the compiler flags, only the ones specific per build type. + + if("${ENABLED_SANITIZER}" STREQUAL "TRUE") + # -fno-omit-frame-pointer disables a good optimization which might corrupt the debugger stack trace. + set(local_compile_options_nondebug -ggdb3 -Og -fno-omit-frame-pointer -fno-inline) + set(local_link_options_nondebug) + else() + # If using ThinLTO: -flto=thin -fsplit-lto-unit + # If using classic/monolithic LTO: -flto=full -fvirtual-function-elimination + # Probably useful when using lto: -ffunction-sections -fdata-sections + set(local_compile_options_nondebug -O3)# -flto) + set(local_link_options_nondebug)#-flto) + endif() + + set(custom_compile_options_release + ${local_compile_options_nondebug} + -flto=full + -fvirtual-function-elimination + -ffunction-sections + -fdata-sections + CACHE INTERNAL STRING + ) + set(custom_compile_options_nightly + ${local_compile_options_nondebug} + CACHE INTERNAL STRING + ) + set(custom_compile_options_debug + -ggdb3 -O0 -fno-omit-frame-pointer -fno-inline + CACHE INTERNAL STRING + ) + + set(custom_link_options_release + ${local_link_options_nondebug} + -flto=full + CACHE INTERNAL STRING + ) + set(custom_link_options_nightly + ${local_link_options_nondebug} + CACHE INTERNAL STRING + ) + set(custom_link_options_debug + "" + CACHE INTERNAL STRING + ) + + if(TARGET spheresvr_release) + message(STATUS "Adding the following compiler options specific to 'Release' build: ${custom_compile_options_release}.") + message(STATUS "Adding the following linker options specific to 'Release' build: ${custom_link_options_release}.") + endif() + if(TARGET spheresvr_nightly) + message(STATUS "Adding the following compiler options specific to 'Nightly' build: ${custom_compile_options_nightly}.") + message(STATUS "Adding the following linker options specific to 'Nightly' build: ${custom_link_options_nightly}.") + endif() + if(TARGET spheresvr_debug) + message(STATUS "Adding the following compiler options specific to 'Debug' build: ${custom_compile_options_debug}.") + message(STATUS "Adding the following linker options specific to 'Debug' build: ${custom_link_options_debug}.") + endif() + +endif() diff --git a/cmake/CompilerFlagsChecker.cmake b/cmake/CompilerFlagsChecker.cmake index b0e183dc0..768b6a9da 100644 --- a/cmake/CompilerFlagsChecker.cmake +++ b/cmake/CompilerFlagsChecker.cmake @@ -4,19 +4,7 @@ message(STATUS "Checking available compiler flags...") if(NOT MSVC) message(STATUS "-- Compilation options:") - # Compiler option flags. - list( - APPEND - compiler_options_base - -pthread - -fexceptions - -fnon-call-exceptions - -pipe - -ffast-math - ) - list(APPEND base_compiler_options_warning -Werror;-Wall;-Wextra;-Wpedantic) - - # Linker flags (warnings) + # Linker flags (warnings). #check_cxx_compiler_flag("-Wl,--fatal-warnings" COMP_LINKER_HAS_FATAL_WARNINGS_V1) #check_cxx_compiler_flag("-Wl,-fatal_warnings" COMP_LINKER_HAS_FATAL_WARNINGS_V2) if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang") @@ -213,6 +201,10 @@ if(NOT MSVC) list(APPEND checked_linker_options_all "-Wl,-fatal_warnings") endif() + #if(COMP_HAS_LTO) + # list(APPEND checked_compiler_options "-flto") + #endif() + if(COMP_HAS_FNO_EXPENSIVE_OPTIMIZATIONS) list(APPEND checked_compiler_options "-fno-expensive-optimizations") endif() @@ -456,8 +448,6 @@ if(NOT MSVC) list( APPEND list_explicit_compiler_options_all - ${compiler_options_base} - ${base_compiler_options_warning} ${checked_compiler_options} ${checked_compiler_options_asan} ${checked_compiler_options_ubsan} @@ -468,18 +458,14 @@ if(NOT MSVC) ${checked_compiler_warnings_disabled} ) - list(APPEND list_explicit_linker_options_all ${checked_linker_options_all};-pthread;-dynamic) - - #string(JOIN " " string_checked_compiler_options_all ${list_checked_compiler_options_all}) - #set(string_checked_compiler_options_all CACHE INTERNAL STRING) - set(list_explicit_compiler_options_all CACHE INTERNAL STRING) - set(list_explicit_linker_options_all CACHE INTERNAL STRING) + list( + APPEND + list_explicit_linker_options_all + ${checked_linker_options_all} + ) # -- - message(STATUS "Adding the following base compiler options: ${compiler_options_base}") - message(STATUS "Adding the following base compiler warning options: ${base_compiler_options_warning}") - message(STATUS "Adding the following conditional compiler options: ${checked_compiler_options}.") if(COMP_HAS_ASAN) message(STATUS "Adding the following conditional compiler options for ASan: ${checked_compiler_options_asan}.") @@ -507,13 +493,12 @@ if(NOT MSVC) STATUS "Adding the following conditional compiler warnings ignore options: ${checked_compiler_warnings_disabled}." ) - message(STATUS "Adding the following linker options: ${list_explicit_linker_options_all}.") + message(STATUS "Adding the following conditional linker options: ${checked_linker_options_all}.") elseif(MSVC) list( APPEND list_explicit_compiler_options_all - ${base_compiler_options_msvc} ${checked_compiler_options_msvc} ) diff --git a/cmake/toolchains/include/Linux-Clang_common.inc.cmake b/cmake/toolchains/include/Linux-Clang_common.inc.cmake index a7304c79c..bb1bf34a7 100644 --- a/cmake/toolchains/include/Linux-Clang_common.inc.cmake +++ b/cmake/toolchains/include/Linux-Clang_common.inc.cmake @@ -18,7 +18,7 @@ function(toolchain_exe_stuff_common) #-- Validate sanitizers options and store them between the common compiler flags. # From https://clang.llvm.org/docs/ClangCommandLineReference.html - # -static-libsan Statically link the sanitizer runtime (Not supported for ASan, TSan or UBSan on darwin) + # -static-libsan Statically link the sanitizer runtime (Not supported for ASan, TSan or UBSan on Darwin) #string(REPLACE ";" " " CXX_FLAGS_EXTRA "${CXX_FLAGS_EXTRA}") @@ -27,29 +27,14 @@ function(toolchain_exe_stuff_common) #-- Apply compiler flags, only the ones specific per build type. - # -fno-omit-frame-pointer disables a good optimization which may corrupt the debugger stack trace. - set(local_compile_options_extra) - if(ENABLED_SANITIZER OR TARGET spheresvr_debug) - set(local_compile_options_extra -fno-omit-frame-pointer -fno-inline) - endif() if(TARGET spheresvr_release) - target_compile_options( - spheresvr_release - PUBLIC -O3 -flto=full -fvirtual-function-elimination ${local_compile_options_extra} - ) + target_compile_options(spheresvr_release PUBLIC ${custom_compile_options_release}) endif() if(TARGET spheresvr_nightly) - if(ENABLED_SANITIZER) - target_compile_options(spheresvr_nightly PUBLIC -ggdb3 -Og ${local_compile_options_extra}) - else() - target_compile_options( - spheresvr_nightly - PUBLIC -O3 -flto=full -fvirtual-function-elimination ${local_compile_options_extra} - ) - endif() + target_compile_options(spheresvr_nightly PUBLIC ${custom_compile_options_nightly}) endif() if(TARGET spheresvr_debug) - target_compile_options(spheresvr_debug PUBLIC -ggdb3 -O0 ${local_compile_options_extra}) + target_compile_options(spheresvr_debug PUBLIC ${custom_compile_options_debug}) endif() #-- Store common linker flags. @@ -62,6 +47,19 @@ function(toolchain_exe_stuff_common) -static-libgcc> # no way to safely statically link against libc ) + #-- Apply linker flags, only the ones specific per build type. + + if(TARGET spheresvr_release) + target_link_options(spheresvr_release PUBLIC ${custom_link_options_release}) + endif() + if(TARGET spheresvr_nightly) + target_link_options(spheresvr_nightly PUBLIC ${custom_link_options_nightly}) + endif() + if(TARGET spheresvr_debug) + target_link_options(spheresvr_debug PUBLIC ${custom_link_options_debug}) + endif() + + #-- Store common define macros. set(cxx_compiler_definitions_common diff --git a/cmake/toolchains/include/Linux-GNU_common.inc.cmake b/cmake/toolchains/include/Linux-GNU_common.inc.cmake index 19e8c3a71..b890cf8d2 100644 --- a/cmake/toolchains/include/Linux-GNU_common.inc.cmake +++ b/cmake/toolchains/include/Linux-GNU_common.inc.cmake @@ -21,23 +21,14 @@ function(toolchain_exe_stuff_common) #-- Apply compiler flags, only the ones specific per build type. - # -fno-omit-frame-pointer disables a good optimization which may corrupt the debugger stack trace. - set(local_compile_options_extra) - if(ENABLED_SANITIZER OR TARGET spheresvr_debug) - set(local_compile_options_extra -fno-omit-frame-pointer -fno-inline) - endif() if(TARGET spheresvr_release) - target_compile_options(spheresvr_release PUBLIC -s -O3 ${local_compile_options_extra}) + target_compile_options(spheresvr_release PUBLIC ${custom_compile_options_release}) endif() if(TARGET spheresvr_nightly) - if(ENABLED_SANITIZER) - target_compile_options(spheresvr_nightly PUBLIC -ggdb3 -Og ${local_compile_options_extra}) - else() - target_compile_options(spheresvr_nightly PUBLIC -O3 ${local_compile_options_extra}) - endif() + target_compile_options(spheresvr_nightly PUBLIC ${custom_compile_options_nightly}) endif() if(TARGET spheresvr_debug) - target_compile_options(spheresvr_debug PUBLIC -ggdb3 -O0 ${local_compile_options_extra}) + target_compile_options(spheresvr_debug PUBLIC ${custom_compile_options_debug}) endif() #-- Store common linker flags. @@ -52,6 +43,18 @@ function(toolchain_exe_stuff_common) > ) + #-- Apply linker flags, only the ones specific per build type. + + if(TARGET spheresvr_release) + target_link_options(spheresvr_release PUBLIC ${custom_link_options_release}) + endif() + if(TARGET spheresvr_nightly) + target_link_options(spheresvr_nightly PUBLIC ${custom_link_options_nightly}) + endif() + if(TARGET spheresvr_debug) + target_link_options(spheresvr_debug PUBLIC ${custom_link_options_debug}) + endif() + #-- Store common define macros. set(cxx_compiler_definitions_common diff --git a/cmake/toolchains/include/OSX-AppleClang_common.inc.cmake b/cmake/toolchains/include/OSX-AppleClang_common.inc.cmake index efc752811..0103c0e61 100644 --- a/cmake/toolchains/include/OSX-AppleClang_common.inc.cmake +++ b/cmake/toolchains/include/OSX-AppleClang_common.inc.cmake @@ -30,23 +30,14 @@ function(toolchain_exe_stuff_common) #-- Apply compiler flags, only the ones specific per build type. - # -fno-omit-frame-pointer disables a good optimization which may corrupt the debugger stack trace. - set(local_compile_options_extra) - if(ENABLED_SANITIZER OR TARGET spheresvr_debug) - set(local_compile_options_extra -fno-omit-frame-pointer -fno-inline) - endif() if(TARGET spheresvr_release) - target_compile_options(spheresvr_release PUBLIC -O3 ${local_compile_options_extra}) + target_compile_options(spheresvr_release PUBLIC ${custom_compile_options_release}) endif() if(TARGET spheresvr_nightly) - if(ENABLED_SANITIZER) - target_compile_options(spheresvr_nightly PUBLIC -ggdb3 -Og ${local_compile_options_extra}) - else() - target_compile_options(spheresvr_nightly PUBLIC -O3 ${local_compile_options_extra}) - endif() + target_compile_options(spheresvr_nightly PUBLIC ${custom_compile_options_nightly}) endif() if(TARGET spheresvr_debug) - target_compile_options(spheresvr_debug PUBLIC -ggdb3 -O0 ${local_compile_options_extra}) + target_compile_options(spheresvr_debug PUBLIC ${custom_compile_options_debug}) endif() #-- Store common linker flags. @@ -60,13 +51,16 @@ function(toolchain_exe_stuff_common) ) #-- Apply linker flags, only the ones specific per build type. - # -s got deprecated, we could directly call 'strip' - #IF (TARGET spheresvr_release) - # target_link_options (spheresvr_release PRIVATE -s) - #ENDIF () - #IF (TARGET spheresvr_nightly AND NOT ${ENABLED_SANITIZER}) - # target_link_options (spheresvr_nightly PRIVATE -s) - #ENDIF () + + if(TARGET spheresvr_release) + target_link_options(spheresvr_release PUBLIC ${custom_link_options_release}) + endif() + if(TARGET spheresvr_nightly) + target_link_options(spheresvr_nightly PUBLIC ${custom_link_options_nightly}) + endif() + if(TARGET spheresvr_debug) + target_link_options(spheresvr_debug PUBLIC ${custom_link_options_debug}) + endif() #-- Store common define macros. diff --git a/cmake/toolchains/include/Windows-Clang_common.inc.cmake b/cmake/toolchains/include/Windows-Clang_common.inc.cmake index bf2094af3..3d399b54a 100644 --- a/cmake/toolchains/include/Windows-Clang_common.inc.cmake +++ b/cmake/toolchains/include/Windows-Clang_common.inc.cmake @@ -35,16 +35,8 @@ function(toolchain_exe_stuff_common) #-- Apply compiler flags, only the ones specific per build type. - # -fno-omit-frame-pointer disables a good optimization which may corrupt the debugger stack trace. - set(local_compile_options_extra) - if(ENABLED_SANITIZER OR TARGET spheresvr_debug) - set(local_compile_options_extra -fno-omit-frame-pointer -fno-inline) - endif() if(TARGET spheresvr_release) - target_compile_options( - spheresvr_release - PUBLIC -O3 -flto=full -fvirtual-function-elimination ${local_compile_options_extra} - ) + if(TARGET spheresvr_release) #[[ IF (NOT ${CLANG_USE_GCC_LINKER}) if (${RUNTIME_STATIC_LINK}) @@ -56,14 +48,7 @@ function(toolchain_exe_stuff_common) ]] endif() if(TARGET spheresvr_nightly) - if(ENABLED_SANITIZER) - target_compile_options(spheresvr_nightly PUBLIC -ggdb3 -Og ${local_compile_options_extra}) - else() - target_compile_options( - spheresvr_nightly - PUBLIC -O3 -flto=full -fvirtual-function-elimination ${local_compile_options_extra} - ) - endif() + target_compile_options(spheresvr_nightly PUBLIC ${custom_compile_options_nightly}) #[[ IF (NOT ${CLANG_USE_GCC_LINKER}) if (${RUNTIME_STATIC_LINK}) @@ -75,7 +60,7 @@ function(toolchain_exe_stuff_common) ]] endif() if(TARGET spheresvr_debug) - target_compile_options(spheresvr_debug PUBLIC -ggdb3 -O0) + target_compile_options(spheresvr_debug PUBLIC ${custom_compile_options_debug}) #[[ IF (NOT ${CLANG_USE_GCC_LINKER}) if (${RUNTIME_STATIC_LINK}) @@ -95,16 +80,40 @@ function(toolchain_exe_stuff_common) if(${RUNTIME_STATIC_LINK}) set(cxx_linker_options_common ${cxx_linker_options_common} -static-libstdc++ -static-libgcc) # no way to statically link against libc? maybe we can on windows? endif() -#[[ + #[[ else () if (${RUNTIME_STATIC_LINK}) set (cxx_linker_options_common ${cxx_linker_options_common} /MTd ) else () set (cxx_linker_options_common ${cxx_linker_options_common} /MDd ) endif () -]] + ]] endif() + #-- Apply linker flags, only the ones specific per build type. + + if(TARGET spheresvr_release) + target_link_options(spheresvr_release PUBLIC ${custom_link_options_release}) + endif() + if(TARGET spheresvr_nightly) + target_link_options(spheresvr_nightly PUBLIC ${custom_link_options_nightly}) + endif() + if(TARGET spheresvr_debug) + target_link_options(spheresvr_debug PUBLIC ${custom_link_options_debug}) + endif() + #if (NOT ${CLANG_USE_GCC_LINKER}) + # IF (TARGET spheresvr_release) + # target_link_options ( spheresvr_release PUBLIC "LINKER:SHELL:" ) + # ENDIF () + # IF (TARGET spheresvr_nightly) + # target_link_options ( spheresvr_nightly PUBLIC "LINKER:SHELL:" ) + # ENDIF () + # IF (TARGET spheresvr_debug) + # target_link_options ( spheresvr_debug PUBLIC "LINKER:/DEBUG" ) + # ENDIF () + #endif () + + #-- Store common define macros. set(cxx_compiler_definitions_common @@ -136,20 +145,6 @@ function(toolchain_exe_stuff_common) endif() endif() - #-- Apply linker options, only the ones specific per build type. - - #if (NOT ${CLANG_USE_GCC_LINKER}) - # IF (TARGET spheresvr_release) - # target_link_options ( spheresvr_release PUBLIC "LINKER:SHELL:" ) - # ENDIF () - # IF (TARGET spheresvr_nightly) - # target_link_options ( spheresvr_nightly PUBLIC "LINKER:SHELL:" ) - # ENDIF () - # IF (TARGET spheresvr_debug) - # target_link_options ( spheresvr_debug PUBLIC "LINKER:/DEBUG" ) - # ENDIF () - #endif () - #-- Now add back the common compiler options, preprocessor macros, linker targets and options. if(NOT ${CLANG_USE_GCC_LINKER}) diff --git a/cmake/toolchains/include/Windows-GNU_common.inc.cmake b/cmake/toolchains/include/Windows-GNU_common.inc.cmake index c2a90d51f..018e0dbc3 100644 --- a/cmake/toolchains/include/Windows-GNU_common.inc.cmake +++ b/cmake/toolchains/include/Windows-GNU_common.inc.cmake @@ -24,23 +24,14 @@ function(toolchain_exe_stuff_common) #-- Apply compiler flags, only the ones specific per build type. - # -fno-omit-frame-pointer disables a good optimization which may corrupt the debugger stack trace. - set(local_compile_options_extra) - if(ENABLED_SANITIZER OR TARGET spheresvr_debug) - set(local_compile_options_extra -fno-omit-frame-pointer -fno-inline) - endif() if(TARGET spheresvr_release) - target_compile_options(spheresvr_release PUBLIC -s -O3 ${local_compile_options_extra}) + target_compile_options(spheresvr_release PUBLIC ${custom_compile_options_release}) endif() if(TARGET spheresvr_nightly) - if(ENABLED_SANITIZER) - target_compile_options(spheresvr_nightly PUBLIC -ggdb3 -Og ${local_compile_options_extra}) - else() - target_compile_options(spheresvr_nightly PUBLIC -O3 ${local_compile_options_extra}) - endif() + target_compile_options(spheresvr_nightly PUBLIC ${custom_compile_options_nightly}) endif() if(TARGET spheresvr_debug) - target_compile_options(spheresvr_debug PUBLIC -ggdb3 -O0 ${local_compile_options_extra}) + target_compile_options(spheresvr_debug PUBLIC ${custom_compile_options_debug}) endif() #-- Store common linker flags. @@ -53,6 +44,18 @@ function(toolchain_exe_stuff_common) -static-libgcc> # no way to statically link against libc? maybe we can on windows? ) + #-- Apply linker flags, only the ones specific per build type. + + if(TARGET spheresvr_release) + target_link_options(spheresvr_release PUBLIC ${custom_link_options_release}) + endif() + if(TARGET spheresvr_nightly) + target_link_options(spheresvr_nightly PUBLIC ${custom_link_options_nightly}) + endif() + if(TARGET spheresvr_debug) + target_link_options(spheresvr_debug PUBLIC ${custom_link_options_debug}) + endif() + #-- Store common define macros. set(cxx_compiler_definitions_common diff --git a/lib/lib_build_flags_common_c.cmake b/lib/lib_build_flags_common_c.cmake index 2d3cc4734..33bc76815 100644 --- a/lib/lib_build_flags_common_c.cmake +++ b/lib/lib_build_flags_common_c.cmake @@ -23,7 +23,11 @@ elseif(NOT MSVC) -pipe -fexceptions -fnon-call-exceptions - -O3 + -O3 # -Os saves ~200 kb, probably not worth losing some performance for that amount... + # Place each function and variable into its own section, allowing the linker to remove unused ones. + #-ffunction-sections + #-fdata-sections + #-flto # Also needed to benefit the most from the two flags above. $<$:-ggdb3> )