45°38'N-13°47'E
Deity
This is really great news! May I ask how have you discovered what was causing that initial out of sync error?
45°38'N-13°47'E;13419903 said:This is really great news! May I ask how have you discovered what was causing that initial out of sync error?
45°38'N-13°47'E;13420599 said:I've tried for a long time to debug that OOS but writing down a logger was beyond what I can do.
void setNextFreeIndex(int iIndex, int iNewValue)
{
if ((iIndex >= getNumSlots()) || (m_pArray == NULL))
{
assert(false);
return;
}
m_pArray[iIndex].iNextFreeIndex = iNewValue;
//Afforess: not sure why this was added, it is not in the base BTS code, and causes OOS in pitboss due to ids mismatching!
/*
m_pArray[iIndex].iLastUsed = m_iCurrentID;
*/
}
void CvPlot::readZobristCache(CvTaggedSaveFormatWrapper *pStream)
{
static int _idHint, _saveSeq = -1;
pStream->Read("CvPlot::g_plotTypeZobristHashes", _idHint, _saveSeq, NUM_PLOT_TYPES, g_plotTypeZobristHashes);
}
void CvPlot::writeZobristCache(CvTaggedSaveFormatWrapper *pStream)
{
static int _idHint, _saveSeq = -1;
if (!g_plotTypeZobristHashesSet)
{
for (int i = 0; i < NUM_PLOT_TYPES; i++)
{
//Afforess: use async rand so as to not pollute mp games
g_plotTypeZobristHashes[i] = GC.getASyncRand().getInt();
}
g_plotTypeZobristHashesSet = true;
}
pStream->Write("CvPlot::g_plotTypeZobristHashes", _idHint, _saveSeq, NUM_PLOT_TYPES, g_plotTypeZobristHashes);
}
Update!
I have fixed another OOS issue, due to changes (by Koshling I presume) to FFreeListTrashArray.h causing an OOS. The relevant change is here:
Code:void setNextFreeIndex(int iIndex, int iNewValue) { if ((iIndex >= getNumSlots()) || (m_pArray == NULL)) { assert(false); return; } m_pArray[iIndex].iNextFreeIndex = iNewValue; //Afforess: not sure why this was added, it is not in the base BTS code, and causes OOS in pitboss due to ids mismatching! /* m_pArray[iIndex].iLastUsed = m_iCurrentID; */ }
Setting the iLastUsed to m_iCurrentID is not part of the base BTS code. The setNextFreeIndex function is called during the reading process, when a game is read from a MP host. This caused the iLastUsed id to be changed from -1 (empty array) to the first value in the sequence (8192). So any player that connected to the host (pitboss server) would have an m_iCurrentID of 8192 for iLastUsed, while the pitboss would be still at the correct value of -1, having never read the game stream itself. If you added an element to the list (by founding a city, say) the ids would go out of sync. If you save and exit and reload, because there was an element in the list, the element would be serialized correctly problem would be solved, temporarily (until a new element was added to the list...).
I don't know when or why that line was added but it's clearly wrong. Removing it and restoring that function back to the BTS base code fixes the OOS I was seeing city ids.
#pragma once
// $Revision: #2 $ $Author: mbreitkreutz $ $DateTime: 2005/06/13 13:35:55 $
//------------------------------------------------------------------------------------------------
//
// ***************** FIRAXIS GAME ENGINE ********************
//
// FILE: FFreeListTrashArray.h
//
// AUTHOR: Soren Johnson
//
// PURPOSE: A dynamic array with a free list that keeps track of its own memory...
//
//------------------------------------------------------------------------------------------------
// Copyright (c) 2004 Firaxis Games, Inc. All rights reserved.
//------------------------------------------------------------------------------------------------
#ifndef FFREELISTTRASHARRAY_H
#define FFREELISTTRASHARRAY_H
#pragma once
#include "FFreeListArrayBase.h"
#include "FDataStreamBase.h"
#include "CvTaggedSaveFormatWrapper.h"
#define FLTA_ID_SHIFT (13)
#define FLTA_MAX_BUCKETS (1 << FLTA_ID_SHIFT)
#define FLTA_INDEX_MASK (FLTA_MAX_BUCKETS - 1)
#define FLTA_ID_MASK (~(FLTA_INDEX_MASK))
#define FLTA_GROWTH_FACTOR (2)
template <class T>
class FFreeListTrashArray : public FFreeListArrayBase<T>
{
public:
FFreeListTrashArray();
virtual ~FFreeListTrashArray();
virtual void init(int iNumSlots = 8);
virtual void uninit();
virtual T* getAt(int iID) const;
T* add();
bool remove(T* pData);
bool removeAt(int iID);
virtual void removeAll();
void load(T* pData);
int getNumSlots() const
{
return m_iNumSlots;
}
int getLastIndex() const
{
return m_iLastIndex;
}
void setLastIndex(int iNewValue)
{
m_iLastIndex = iNewValue;
}
int getFreeListHead() const
{
return m_iFreeListHead;
}
void setFreeListHead(int iNewValue)
{
m_iFreeListHead = iNewValue;
}
int getFreeListCount() const
{
return m_iFreeListCount;
}
void setFreeListCount(int iNewValue)
{
m_iFreeListCount = iNewValue;
}
int getCurrentID()
{
return m_iCurrentID;
}
void setCurrentID(int iNewValue)
{
assert((iNewValue & FLTA_INDEX_MASK) == 0);
assert((iNewValue & FLTA_ID_MASK) != 0);
m_iCurrentID = iNewValue;
}
int getNextFreeIndex(int iIndex)
{
if ((iIndex >= getNumSlots()) || (m_pArray == NULL))
{
assert(false);
return FFreeList::INVALID_INDEX;
}
return m_pArray[iIndex].iNextFreeIndex;
}
void setNextFreeIndex(int iIndex, int iNewValue)
{
if ((iIndex >= getNumSlots()) || (m_pArray == NULL))
{
assert(false);
return;
}
m_pArray[iIndex].iNextFreeIndex = iNewValue;
}
void Read( FDataStreamBase* pStream );
void Write( FDataStreamBase* pStream );
protected:
struct FFreeListTrashArrayNode
{
int iNextFreeIndex;
T* pData;
};
int m_iCurrentID;
FFreeListTrashArrayNode* m_pArray;
mutable CRITICAL_SECTION m_cModifySection;
virtual void growArray();
};
// Public functions...
template <class T>
FFreeListTrashArray<T>::FFreeListTrashArray()
{
m_iCurrentID = FLTA_MAX_BUCKETS;
m_pArray = NULL;
InitializeCriticalSectionAndSpinCount(&m_cModifySection, 4000);
}
template <class T>
FFreeListTrashArray<T>::~FFreeListTrashArray()
{
uninit();
DeleteCriticalSection(&m_cModifySection);
}
template <class T>
void FFreeListTrashArray<T>::init(int iNumSlots)
{
int iCount;
int iI;
assert(iNumSlots >= 0);
// make sure it's binary...
if ((iNumSlots > 0) && ((iNumSlots - 1) & iNumSlots) != 0)
{
// find high bit
iCount = 0;
while (iNumSlots != 1)
{
iNumSlots >>= 1;
iCount++;
}
iNumSlots = (1 << (iCount + 1));
}
assert(((iNumSlots - 1) & iNumSlots) == 0);
assert((m_iNumSlots <= FLTA_MAX_BUCKETS) && "FFreeListTrashArray<T>::init() size too large");
uninit();
m_iNumSlots = iNumSlots;
m_iLastIndex = FFreeList::INVALID_INDEX;
m_iFreeListHead = FFreeList::INVALID_INDEX;
m_iFreeListCount = 0;
m_iCurrentID = FLTA_MAX_BUCKETS;
if (m_iNumSlots > 0)
{
m_pArray = new FFreeListTrashArrayNode[m_iNumSlots];
for (iI = 0; iI < m_iNumSlots; iI++)
{
m_pArray[iI].iNextFreeIndex = FFreeList::INVALID_INDEX;
m_pArray[iI].pData = NULL;
}
}
}
template <class T>
void FFreeListTrashArray<T>::uninit()
{
if (m_pArray != NULL)
{
removeAll();
SAFE_DELETE_ARRAY(m_pArray);
}
}
template <class T>
T* FFreeListTrashArray<T>::add()
{
int iIndex;
T* result = NULL;
EnterCriticalSection(&m_cModifySection);
if (m_pArray == NULL)
{
init();
}
if ((m_iLastIndex == m_iNumSlots - 1) &&
(m_iFreeListCount == 0))
{
if ((m_iNumSlots * FLTA_GROWTH_FACTOR) > FLTA_MAX_BUCKETS)
{
LeaveCriticalSection(&m_cModifySection);
return NULL;
}
growArray();
}
if (m_iFreeListCount > 0)
{
iIndex = m_iFreeListHead;
m_iFreeListHead = m_pArray[m_iFreeListHead].iNextFreeIndex;
m_iFreeListCount--;
}
else
{
m_iLastIndex++;
iIndex = m_iLastIndex;
}
MEMORY_TRACK_EXEMPT();
m_pArray[iIndex].pData = new T;
m_pArray[iIndex].iNextFreeIndex = FFreeList::INVALID_INDEX;
m_pArray[iIndex].pData->setID(m_iCurrentID + iIndex);
m_iCurrentID += FLTA_MAX_BUCKETS;
result = m_pArray[iIndex].pData;
LeaveCriticalSection(&m_cModifySection);
return result;
}
template <class T>
T* FFreeListTrashArray<T>::getAt(int iID) const
{
int iIndex;
T* result = NULL;
if ((iID == FFreeList::INVALID_INDEX) || (m_pArray == NULL))
{
return NULL;
}
EnterCriticalSection(&m_cModifySection);
iIndex = (iID & FLTA_INDEX_MASK);
assert(iIndex >= 0);
if ((iIndex <= m_iLastIndex) &&
(m_pArray[iIndex].pData != NULL))
{
if (((iID & FLTA_ID_MASK) == 0) || (m_pArray[iIndex].pData->getID() == iID))
{
result = m_pArray[iIndex].pData;
}
}
LeaveCriticalSection(&m_cModifySection);
return result;
}
template <class T>
bool FFreeListTrashArray<T>::remove(T* pData)
{
int iI;
bool result = false;
assert(m_pArray != NULL);
EnterCriticalSection(&m_cModifySection);
if (pData != NULL)
{
for (iI = 0; iI <= m_iLastIndex; iI++)
{
if (m_pArray[iI].pData == pData)
{
result = removeAt(iI);
}
}
}
LeaveCriticalSection(&m_cModifySection);
return result;
}
template <class T>
bool FFreeListTrashArray<T>::removeAt(int iID)
{
int iIndex;
bool result = false;
if ((iID == FFreeList::INVALID_INDEX) || (m_pArray == NULL))
{
return false;
}
EnterCriticalSection(&m_cModifySection);
iIndex = (iID & FLTA_INDEX_MASK);
assert(iIndex >= 0);
if ((iIndex <= m_iLastIndex) &&
(m_pArray[iIndex].pData != NULL))
{
if (((iID & FLTA_ID_MASK) == 0) || (m_pArray[iIndex].pData->getID() == iID))
{
delete m_pArray[iIndex].pData;
m_pArray[iIndex].pData = NULL;
m_pArray[iIndex].iNextFreeIndex = m_iFreeListHead;
m_iFreeListHead = iIndex;
m_iFreeListCount++;
result = true;
}
else
{
assert(false);
}
}
LeaveCriticalSection(&m_cModifySection);
return result;
}
template <class T>
void FFreeListTrashArray<T>::removeAll()
{
int iI;
if (m_pArray == NULL)
{
return;
}
EnterCriticalSection(&m_cModifySection);
m_iLastIndex = FFreeList::INVALID_INDEX;
m_iFreeListHead = FFreeList::INVALID_INDEX;
m_iFreeListCount = 0;
for (iI = 0; iI < m_iNumSlots; iI++)
{
m_pArray[iI].iNextFreeIndex = FFreeList::INVALID_INDEX;
if (m_pArray[iI].pData != NULL)
{
delete m_pArray[iI].pData;
}
m_pArray[iI].pData = NULL;
}
LeaveCriticalSection(&m_cModifySection);
}
template <class T>
void FFreeListTrashArray<T>::load(T* pData)
{
int iIndex;
assert(pData != NULL);
//assert((pData->getID() & FLTA_ID_MASK) < m_iCurrentID);
assert(m_pArray != NULL);
iIndex = (pData->getID() & FLTA_INDEX_MASK);
assert(iIndex < FLTA_MAX_BUCKETS);
assert(iIndex <= m_iLastIndex);
assert(m_pArray[iIndex].pData == NULL);
assert(m_pArray[iIndex].iNextFreeIndex == FFreeList::INVALID_INDEX);
m_pArray[iIndex].pData = pData;
}
// Protected functions...
template <class T>
void FFreeListTrashArray<T>::growArray()
{
FFreeListTrashArrayNode* pOldArray;
int iOldNumSlots;
int iI;
MEMORY_TRACK_EXEMPT();
assert(m_pArray != NULL);
pOldArray = m_pArray;
iOldNumSlots = m_iNumSlots;
m_iNumSlots *= FLTA_GROWTH_FACTOR;
assert((m_iNumSlots <= FLTA_MAX_BUCKETS) && "FFreeListTrashArray<T>::growArray() size too large");
m_pArray = new FFreeListTrashArrayNode[m_iNumSlots];
for (iI = 0; iI < m_iNumSlots; iI++)
{
if (iI < iOldNumSlots)
{
m_pArray[iI] = pOldArray[iI];
}
else
{
m_pArray[iI].iNextFreeIndex = FFreeList::INVALID_INDEX;
m_pArray[iI].pData = NULL;
}
}
delete [] pOldArray;
}
//
// use when list contains non-streamable types
//
template < class T >
inline void FFreeListTrashArray< T >::Read( FDataStreamBase* pStream )
{
CvTaggedSaveFormatWrapper& wrapper = CvTaggedSaveFormatWrapper::getSaveFormatWrapper();
wrapper.AttachToStream(pStream);
WRAPPER_READ_OBJECT_START(wrapper);
int iTemp;
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "numSlots" );
init( iTemp );
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "lastIndex" );
setLastIndex( iTemp );
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "freeListHead" );
setFreeListHead( iTemp );
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "freeListCount" );
setFreeListCount( iTemp );
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "currentID" );
setCurrentID( iTemp );
int i;
for ( i = 0; i < getNumSlots(); i++ )
{
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iTemp, "nextFreeIndex" );
setNextFreeIndex( i, iTemp );
}
int iCount;
WRAPPER_READ_DECORATED(wrapper, "FFreeListTrashArray", &iCount, "count" );
for ( i = 0; i < iCount; i++ )
{
T* pData = new T;
WRAPPER_READ_ARRAY_DECORATED(wrapper, "FFreeListTrashArray", sizeof ( T ), ( byte* )pData, "pData" );
load( pData );
}
WRAPPER_READ_OBJECT_END(wrapper);
}
template < class T >
inline void FFreeListTrashArray< T >::Write( FDataStreamBase* pStream )
{
CvTaggedSaveFormatWrapper& wrapper = CvTaggedSaveFormatWrapper::getSaveFormatWrapper();
wrapper.AttachToStream(pStream);
WRAPPER_WRITE_OBJECT_START(wrapper);
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getNumSlots(), "numSlots" );
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getLastIndex(), "lastIndex" );
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getFreeListHead(), "freeListHead" );
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getFreeListCount(), "freeListCount" );
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getCurrentID(), "currentID" );
int i;
for ( i = 0; i < getNumSlots(); i++ )
{
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getNextFreeIndex( i ), "nextFreeIndex" );
}
WRAPPER_WRITE_DECORATED(wrapper, "FFreeListTrashArray", getCount(), "count" );
for ( i = 0; i < getIndexAfterLast(); i++ )
{
if ( getAt( i ) )
{
WRAPPER_WRITE_ARRAY_DECORATED(wrapper, "FFreeListTrashArray", sizeof ( T ), ( byte* )getAt( i ), "pData" );
}
}
WRAPPER_WRITE_OBJECT_END(wrapper);
}
//-------------------------------
// Serialization helper templates:
//-------------------------------
//
// use when list contains streamable types
//
template < class T >
inline void ReadStreamableFFreeListTrashArray( FFreeListTrashArray< T >& flist, FDataStreamBase* pStream )
{
CvTaggedSaveFormatWrapper& wrapper = CvTaggedSaveFormatWrapper::getSaveFormatWrapper();
wrapper.AttachToStream(pStream);
WRAPPER_READ_OBJECT_START(wrapper);
int iTemp;
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistNumSlots" );
flist.init( iTemp );
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistLastIndex" );
flist.setLastIndex( iTemp );
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistFreeListHead" );
flist.setFreeListHead( iTemp );
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistFreeListCount" );
flist.setFreeListCount( iTemp );
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistCurrentId" );
flist.setCurrentID( iTemp );
int i;
for ( i = 0; i < flist.getNumSlots(); i++ )
{
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iTemp, "flistNextFree" );
flist.setNextFreeIndex( i, iTemp );
}
int iCount;
WRAPPER_READ_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", &iCount, "flistCount" );
for ( i = 0; i < iCount; i++ )
{
T* pData = new T;
pData->read( pStream );
flist.load( pData );
}
WRAPPER_READ_OBJECT_END(wrapper);
}
template < class T >
inline void WriteStreamableFFreeListTrashArray( FFreeListTrashArray< T >& flist, FDataStreamBase* pStream )
{
CvTaggedSaveFormatWrapper& wrapper = CvTaggedSaveFormatWrapper::getSaveFormatWrapper();
wrapper.AttachToStream(pStream);
WRAPPER_WRITE_OBJECT_START(wrapper);
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getNumSlots(), "flistNumSlots" );
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getLastIndex(), "flistLastIndex" );
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getFreeListHead(), "flistFreeListHead" );
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getFreeListCount(), "flistFreeListCount" );
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getCurrentID(), "flistCurrentId" );
int i;
for ( i = 0; i < flist.getNumSlots(); i++ )
{
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getNextFreeIndex( i ), "flistNextFree" );
}
WRAPPER_WRITE_DECORATED(wrapper, "WriteStreamableFFreeListTrashArray", flist.getCount(), "flistCount" );
for ( i = 0; i < flist.getIndexAfterLast(); i++ )
{
if ( flist[ i ] )
{
flist[ i ]->write( pStream );
}
}
WRAPPER_WRITE_OBJECT_END(wrapper);
}
#endif //FFREELISTTRASHARRAY_H
The SVN log says "Improved id allocation algorithm" maybe it's faster?
But if it causes OOS you should completely remove that change.
Just pushed to SVN:
- Small tweak to id allocation to make id-wrapping less likely (caused some very rare and obscure bugs)
- Widespread memory usage reduction - saves about another 100M so far on the runtime footprint
It seems Koshling added this Improved id allocation algorithm to fix bugs.
45°38'N-13°47'E;13423502 said:Well, I've checked and it seems that this change has been imported from C2C into AND code back in rev557, january 2012 by... Afforess.
I think that was when I was about to quit and just blindly pulling changes.
Otherwise, as I theorized back then, you're absolutely right that it should be quite useful and SHOULD be easily done. 45* is right though that one player exiting and re-entering (hot-reload) while the other keeps the game going just doesn't tend to work at all. So that may be some issue in the Firaxis core (I don't think it's EVER worked well.)
Two updates. I fixed an OOS related to the Realistic Culture Spread. Realistic Culture Spread used a hashmap cache of culture distances, which was updated whenever the culture level changed (or city was captured.), at the end of the turn. This was fine for SP, but in MP, changes to tiles/features/improvements came whenever the players did an action, and could not wait to be processed at the end of the turn. The cache has to be cleared before each use. A minor performance hit, but the culture distance is used very infrequently (I am not sure it even deserved a cache, but that is not relevant).
I also hardcoded Barbarian Civs off in the game options if you are in multiplayer. I had Barbarian civs spawn three times in MP, but only on one players machine (it was a different player each time, our setups were fine). Barbarian Civs needs to be looked at (or possibly, simply rewritten in the SDK) so that it no longer causes OOS.
So progress on my original idea. I created a class, FDataStreamBuffer which writes/reads from a ByteBuffer (a library I pulled in, a mutable length byte buffer array...If you have used ByteBuffer in Java, this is a carbon copy). It works great, I can write the game state to the buffer fine, and I saved the state to my computer and it looks ok in a hexadecimal editor.
When trying to re-serialize the game state from the previously written game state, just to prove it works locally, I have 2 problems. The InitCore reads from the buffer fine, but once the CvGameAI object tries to read the buffer, it blows up in two places:
1.) Deinitializing Unit Graphics. When you call read(...) from CvGame it deinitializes the entire game state for the map, all players, etc. Deinitializing players causes units to deinitialize as well. The deinitialization process blows up when trying to remove unit graphics, specifically CvDLLEntity::destroyEntity() in the deconstructor. Manually setting a breakpoint and skipping that step avoids that - but that might leak memory!
2.) Reading in the m_sorenRand object in CvGame blows up. The seed right before (above) it reads in fine, but for some reason the m_sorenRand seed blows up. Skipping it causes the save stream to desync.
I included the code I was using to test this in CvGame, 2742. It is turned off by a preprocessor statement, but you can turn it back on and reproduce this. Make sure to update the szFile paths unless you want your game to blow up earlier. I am hoping AIAndy or Koshling, or someone familiar with the save format can tell me what I am doing wrong here.
I think as a possible plan B workaround, if we can't figure this out, is that I could write a MERGE function for CvGame, CvUnit, CvPlayer, etc... That tries to merge the data instead of deinitializing. It would be pretty tedious, so I'd like to avoid it if possible.
I'm afraid I have no insights on these issues to offer.
Reading back in thread a little though, the change to the id allocator was (at least as I recall) because in large games with high unit counts the id space under the old allocator wrapped way too fast, and games just died completely, typically around the industrial era. The new allocator chews up id space far less quickly.
As far as I am aware, I haven't disabled your new allocator, I just removed the last id used in the loadup of the game.