Multiplayer: Hacking A Fix Around OOS Errors

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?

My setup is that I compile a DEBUG dll and start a pitboss and 1 game client. I have set up additional MP RNG logging so that every call to the RNG is logged in a DEBUG dll in multiplayer. I start a pitboss game, and connect and examine the logs. When I found that something (but I had no idea what, because it had a NULL RNG pszLog message) was calling the RNG after game connect, I stuck a breakpoint there and reconnected. The CvPlot zobrist cache was initializing and that was the cause of the initial OOS. I don't want to trivalize the amount of work involved in that - creating the RNG logger and testing to find that issue took ~ 4 hours.

There is also the issue with the production I mentioned. I know what is causing the OOS, but not why. When the client chooses the production from the popup, it sends a CvNetMessageData with the city, production type (building/unit/process) and production item (building id) and when the pitboss client sees the data, it looks up the city to assign it the production item, but the city is NULL. The Pitboss game does have 1 city already assigned to the player (and I have only founded 1 city, this is the first turn of the game) and the ID being sent by the client is the 2nd city id, not the first. This leads me to suspect the client is founding the city twice, possibly it is receiving it's own CvNetMessageData about the unit found operation, and founding it once in the client, once from it's own MP notice. I am unsure though, and quit debugging due to time constraints before I could determine the root issue.
 
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. :(

I won't deny it, debugging MP is easily one of the hardest modding tasks because it involves a complete understanding of the entire Civ codebase. If you don't understand how everything works and works together, you will not succeed. Even with the right knowledge, it is extremely time consuming.
 
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.

In addition, I added 2 static functions in CvPlot to serialize the zobrist cache, and also added the m_zobristContribution to the CvPlot object serialization.

Code:
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);
}

These are called in CvMap, after the serialization of the array CvPlot objects in the game. Full code is in the repo.

I originally changed the zobrist hashes to use Async rand, which temporarily solved the OOS issue with them. In a pitboss game, the pitboss server initialized the game map at a different time than the client did. This caused the soren rand seed to go out of sync. Using an async rand does fix this. However, I was concerned that each client would have a unique and different zobrist hash and that this difference could eventually cause subtle OOS issues due to the breakdown in deterministic behavior due differing client values. The MP in Civ4 works because each client is supposed to calculate the game exactly the same way, in a deterministic fashion, and this violated that principle... I err'd on the side of caution and so all the zobrist cache values are now serialized - preventing any possible breakdown in the deterministic nature of the game, and hopefully preventing any OOS issues.

I also added a lot of logging for MP, when using a DEBUG dll. There will be a log file created for each active player that lists most of the steps for each net message, which is very helpful in determining where discrepancies may occur.
 
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.

The SVN log says "Improved id allocation algorithm" maybe it's faster?
But if it causes OOS you should completely remove that change.
Code:
#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.

Do you know the SVN revision that introduced the change to C2C? I didn't feel inclined to search through all 9000+ revisions.

The change to the id allocation looks like it might have made scanning the table for adding a new element faster - but seriously, the add operation on a dynamic list is the wrong place to be making such optimizations.
 
It seems Koshling added this Improved id allocation algorithm to fix bugs.

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.

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. :lol:
 
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. :lol:

I think that was when I was about to quit and just blindly pulling changes. :p
 
I think that was when I was about to quit and just blindly pulling changes. :p

Well, more or less it's what I've done for 2 years while you were away, so hopefully it's not that bad. :p
 
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.)

BTW, the hot-reload issue was the same issue as the pitboss issue, namely the zobrist hashes!

Hotloading in a game works fine now. Tested it last night with a friend.
 
If you can reduce the OOSs drastically I'm sure I'll be a lot more useful to testing the mod then I am now. Most of my experience is with a friend who plays with me every weekend, but with like 15 OOSs in a game that has been active for 3 hours just makes us quit and go play some Nintendo 64 nice games.

If I had 2 pcs I could help more too, but I don't so I'm stuck to my friend's patience.

IMO OOSs are the most important issue to deal with.

BTW, Simultaneous turn games can suffer from different/more kinds of OOSs sources, or are they equal to the normal game?
 
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.
 
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.
 
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.
 
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.

Doesn't that need to be persisted though (across the save and reload)?
 
Top Bottom