closes #54

The argument is now always treated as a plain string and passed directly
to the cheat. This is the behavior expected by all the functions in
xCheat.py, which parse the string argument manually where needed.

The previous implementation evaluated the argument value as a Python
expression. This theoretically allowed passing non-string arguments, but
none of the cheats use this (and it's also a potential code execution
issue). If no argument was passed, the function was called with no
arguments, which is also not supported by any of the cheats.

In practice, this required unintuitive syntax for calling the cheats
correctly, for example:

    Python.Cheat GetPlayerID None
    Python.Cheat GetSDL "'FirstWeekClothing'"

With the new argument handling, these have been simplified to:

    Python.Cheat GetPlayerID
    Python.Cheat GetSDL FirstWeekClothing

Sources/Plasma/FeatureLib/pfConsole/pfConsoleCommands.cpp

@@ -7061,17 +7061,12 @@ PF_CONSOLE_CMD( Python,
 				"string functions, ...",	// Params
 				"Run a cheat command" )
 {
-	std::string extraParms;
+	const char* extraParms = "";
 	if (numParams > 1)
 	{
-		extraParms = "(";
-		extraParms.append(params[1]);
-		extraParms.append(",)");
+		extraParms = params[1];
 	}
-	else
-		extraParms = "()";
-
-	PythonInterface::RunFunctionSafe("xCheat", params[0], extraParms.c_str());
+	PythonInterface::RunFunctionStringArg("xCheat", params[0], extraParms);
 
 	std::string output;
 	// get the messages

Sources/Plasma/FeatureLib/pfMessage/pfKIMsg.h

@@ -165,6 +165,8 @@ class pfKIMsg : public plMessage
 			kUNUSED1		= 0x00000008,
 			kStatusMsg		= 0x00000010,
 			kNeighborMsg	= 0x00000020,	// sending to all the neighbors
+			kSubtitleMsg	= 0x00000040,
+			kLocKeyMsg		= 0x00000080,
 			kChannelMask	= 0x0000ff00
 		};
 

Sources/Plasma/FeatureLib/pfPython/cyMiscGlue3.cpp

@@ -777,4 +777,4 @@ void cyMisc::AddPlasmaMethods3(std::vector<PyMethodDef> &methods)
 
 	PYTHON_GLOBAL_METHOD_NOARGS(methods, PtGetUserPath);
 	PYTHON_GLOBAL_METHOD_NOARGS(methods, PtGetInitPath);
-}
+}

Sources/Plasma/FeatureLib/pfPython/cyMiscGlue4.cpp

@@ -853,4 +853,4 @@ void cyMisc::AddPlasmaMethods4(std::vector<PyMethodDef> &methods)
 	PYTHON_GLOBAL_METHOD(methods, PtGetAIAvatarsByModelName);
 	PYTHON_GLOBAL_METHOD(methods, PtForceVaultNodeUpdate);
 	PYTHON_GLOBAL_METHOD(methods, PtVaultDownload);
-}
+}

Sources/Plasma/FeatureLib/pfPython/cyPythonInterface.cpp

@@ -2102,38 +2102,32 @@ PyObject* PythonInterface::RunFunction(PyObject* module, const char* name, PyObj
 	return result;
 }
 
-PyObject* PythonInterface::ParseArgs(const char* args)
-{
-	PyObject* result = NULL;
-	PyObject* scope = PyDict_New();
-	if (scope) 
-	{
-		//- Py_eval_input makes this function accept only single expresion (not statement)
-		//- When using empty scope, functions and classes like 'file' or '__import__' are not visible
-		result = PyRun_String(args, Py_eval_input, scope, NULL);
-		Py_DECREF(scope);
-	}
-   
-	return result;
-}
-
-bool PythonInterface::RunFunctionSafe(const char* module, const char* function, const char* args) 
+bool PythonInterface::RunFunctionStringArg(const char* module, const char* name, const char* arg)
 {
 	PyObject* moduleObj = ImportModule(module);
 	bool result = false;
 	if (moduleObj) 
 	{
-		PyObject* argsObj = ParseArgs(args);
-		if (argsObj) 
+		PyObject* argObj = PyString_FromString(arg);
+		if (argObj)
 		{
-			PyObject* callResult = RunFunction(moduleObj, function, argsObj);
-			if (callResult) 
+			PyObject* argsObj = PyTuple_New(1);
+			if (argsObj)
 			{
-				result = true;
-				Py_DECREF(callResult);
+				// PyTuple_SET_ITEM steals the reference to argObj.
+				PyTuple_SET_ITEM(argsObj, 0, argObj);
+				PyObject* callResult = RunFunction(moduleObj, name, argsObj);
+				if (callResult)
+				{
+					result = true;
+					Py_DECREF(callResult);
+				}
+				Py_DECREF(argsObj);
+			}
+			else
+			{
+				Py_DECREF(argObj);
 			}
-
-			Py_DECREF(argsObj);
 		}
 		Py_DECREF(moduleObj);
 	}

Sources/Plasma/FeatureLib/pfPython/cyPythonInterface.h

@@ -223,9 +223,7 @@ public:
 
 	static PyObject* RunFunction(PyObject* module, const char* name, PyObject* args);
 
-	static PyObject* ParseArgs(const char* args);
-
-	static bool RunFunctionSafe(const char* module, const char* function, const char* args);
+	static bool RunFunctionStringArg(const char* module, const char* name, const char* arg);
 
 	/////////////////////////////////////////////////////////////////////////////
 	//

Sources/Plasma/FeatureLib/pfPython/plPythonSDLModifier.cpp

@@ -474,6 +474,12 @@ void plPythonSDLModifier::IPythonVarToSDL(plStateDataRecord* state, const char*
 		int count = PyTuple_Size(pyVar);
 		plSimpleVarDescriptor::Type type = var->GetSimpleVarDescriptor()->GetType();
 
+		// Ensure that variable length arrays match.
+		if (var->GetSimpleVarDescriptor()->IsVariableLength()) {
+			if (var->GetCount() != count)
+				var->Alloc(count);
+		}
+
 		for (int i = 0; i < count; i++)
 		{
 			PyObject* pyVarItem = PyTuple_GetItem(pyVar, i);

Sources/Plasma/PubUtilLib/plAvatar/plAvBrainClimb.cpp

@@ -326,7 +326,7 @@ bool plAvBrainClimb::IProcessExitStage(double time, float elapsed)
 
 	float curBlend = ai->GetBlend();
 	
-	if(curBlend > .99)		// reached peak strength
+	if(fCurStage && curBlend > .99)		// reached peak strength
 	{
 		fCurStage->Detach(fAvMod);	// remove the (now completely masked) underlying anim
 		fCurStage = nil;

Sources/Plasma/PubUtilLib/plInputCore/plInputDevice.cpp

@@ -481,8 +481,8 @@ void plMouseDevice::SetCursorY(hsScalar y)
 
 void plMouseDevice::HideCursor(hsBool override)
 {
-	if( fInstance->fCursor != nil )
-		fInstance->fCursor->SetVisible( false );
+	if (fInstance && fInstance->fCursor)
+		fInstance->fCursor->SetVisible(false);
 
 	plMouseDevice::bCursorOverride = (override != 0);
 	plMouseDevice::bCursorHidden = true;
@@ -499,9 +499,11 @@ void plMouseDevice::ShowCursor(hsBool override)
 	plMouseDevice::bCursorHidden = false;
 	plMouseDevice::bCursorOverride = false;
 	
-	if( fInstance->fCursor == nil )
-		fInstance->CreateCursor( fInstance->fCursorID );
-	fInstance->fCursor->SetVisible( true );
+	if (fInstance) {
+		if (!fInstance->fCursor)
+			fInstance->CreateCursor(fInstance->fCursorID);
+		fInstance->fCursor->SetVisible(true);
+	}
 }
 
 void plMouseDevice::NewCursor(char* cursor)

Sources/Plasma/PubUtilLib/plModifier/plResponderModifier.cpp

@@ -217,6 +217,8 @@ bool plResponderModifier::IIsLocalOnlyCmd(plMessage* cmd)
 		return true;
 	if (plCameraMsg::ConvertNoRef(cmd))		// don't want to change our camera
 		return true;
+	if (plSubWorldMsg::ConvertNoRef(cmd))   // don't want to enter a subworld (changes the avatar SDL)
+		return true;
 
 	plSoundMsg *snd = plSoundMsg::ConvertNoRef( cmd );
 	if( snd != nil && snd->Cmd( plSoundMsg::kIsLocalOnly ) )

Sources/Plasma/PubUtilLib/plNetClient/plNetCliAgeJoiner.cpp

@@ -211,7 +211,7 @@ void plNCAgeJoiner::Start () {
 
 	plNetClientMgr * nc = plNetClientMgr::GetInstance();
 	nc->SetFlagsBit(plNetClientMgr::kPlayingGame, false);
-	nc->fServerTimeOffset				= 0;	// reset since we're connecting to a new server
+	nc->ResetServerTimeOffset();
 	nc->fRequiredNumInitialSDLStates	= 0;
 	nc->fNumInitialSDLStates			= 0;
 	nc->SetFlagsBit(plNetClientApp::kNeedInitialAgeStateCount);

Sources/Plasma/PubUtilLib/plNetClient/plNetClientMgr.cpp

@@ -133,9 +133,7 @@ plNetClientMgr::plNetClientMgr() :
 		// fProgressBar( nil ),
 		fTaskProgBar( nil ),
 		fMsgRecorder(nil),
-		fServerTimeOffset(0),
-		fTimeSamples(0),
-		fLastTimeUpdate(0),
+		fLastLocalTime(), 
 		fListenListMode(kListenList_Distance),
 		fAgeSDLObjectKey(nil),
 		fExperimentalLevel(0),
@@ -480,34 +478,23 @@ void plNetClientMgr::StartLinkInFX()
 //
 void plNetClientMgr::UpdateServerTimeOffset(plNetMessage* msg)
 {
-	if ((hsTimer::GetSysSeconds() - fLastTimeUpdate) > 5)
-	{
-		fLastTimeUpdate = hsTimer::GetSysSeconds();
+	if (!msg->GetHasTimeSent())
+		return;
+	if (msg->GetTimeSent().AtEpoch())
+		return;
 
-		const plUnifiedTime& msgSentUT = msg->GetTimeSent();
-		if (!msgSentUT.AtEpoch())
-		{
-			double diff = plUnifiedTime::GetTimeDifference(msgSentUT, plClientUnifiedTime::GetCurrentTime());
+	double localTime = hsTimer::GetSeconds();
+	if (localTime - fLastLocalTime < 1.0)
+		return;
 
-			if (fServerTimeOffset == 0)
-			{
-				fServerTimeOffset = diff;
-			}
-			else
-			{
-				fServerTimeOffset = fServerTimeOffset + ((diff - fServerTimeOffset) / ++fTimeSamples);
-			}
-
-			DebugMsg("Setting server time offset to %f", fServerTimeOffset);
-		}
-	}
+	fLastServerTime = msg->GetTimeSent();
+	fLastLocalTime = localTime;
 }
 
 void plNetClientMgr::ResetServerTimeOffset()
 {
-	fServerTimeOffset = 0;
-	fTimeSamples = 0;
-	fLastTimeUpdate = 0;
+	fLastServerTime.ToEpoch();
+	fLastLocalTime = 0.0;
 }
 
 //
@@ -515,14 +502,12 @@ void plNetClientMgr::ResetServerTimeOffset()
 //
 plUnifiedTime plNetClientMgr::GetServerTime() const 
 { 
-	if ( fServerTimeOffset==0 )		// offline mode or before connecting/calibrating to a server
+	if (fLastServerTime.AtEpoch()) {
+		WarningMsg("Someone asked for the server time, but we don't know it yet!");
 		return plUnifiedTime::GetCurrentTime();
-	
-	plUnifiedTime serverUT;
-	if (fServerTimeOffset<0)
-		return plUnifiedTime::GetCurrentTime() - plUnifiedTime(fabs(fServerTimeOffset)); 
-	else
-		return  plUnifiedTime::GetCurrentTime() + plUnifiedTime(fServerTimeOffset); 
+	}
+
+	return fLastServerTime + plUnifiedTime(hsTimer::GetSeconds() - fLastLocalTime);
 }
 
 //

Sources/Plasma/PubUtilLib/plNetClient/plNetClientMgr.h

@@ -166,9 +166,8 @@ private:
 	std::string			fSPDesiredPlayerName;	// SP: the player we want to load from vault.
 	
 	// server info
-	double				fServerTimeOffset;		// diff between our unified time and server's unified time
-	UInt32				fTimeSamples;
-	double				fLastTimeUpdate;
+	plUnifiedTime		fLastServerTime;		// Last received time update from the server
+	double				fLastLocalTime;			// Last monotonic time (in seconds) when the above update was received
 
 	UInt8				fJoinOrder;			// returned by the server
 	

Sources/Plasma/PubUtilLib/plPhysX/plPXPhysical.cpp

@@ -1328,9 +1328,9 @@ void plPXPhysical::GetSyncState(hsPoint3& pos, hsQuat& rot, hsVector3& linV, hsV
 
 void plPXPhysical::SetSyncState(hsPoint3* pos, hsQuat* rot, hsVector3* linV, hsVector3* angV)
 {
-	bool initialSync =	plNetClientApp::GetInstance()->IsLoadingInitialAgeState() &&
-						plNetClientApp::GetInstance()->GetJoinOrder() == 0;
-
+	bool isLoading = plNetClientApp::GetInstance()->IsLoadingInitialAgeState();
+	bool isFirstIn = plNetClientApp::GetInstance()->GetJoinOrder() == 0;
+	bool initialSync = isLoading && isFirstIn;
 	// If the physical has fallen out of the sim, and this is initial age state, and we're
 	// the first person in, reset it to the original position.  (ie, prop the default state
 	// we've got right now)
@@ -1350,6 +1350,13 @@ void plPXPhysical::SetSyncState(hsPoint3* pos, hsQuat* rot, hsVector3* linV, hsV
 		SetLinearVelocitySim(*linV);
 	if (angV)
 		SetAngularVelocitySim(*angV);
+	// If we're loading the age, then we should ensure the objects stay asleep if they're supposed to be asleep.
+	// NOTE: We should only do this if the objects are not at their initial locations. Otherwise, they might
+	//       sleep inside each other and explode or float randomly in midair
+	if (isLoading && GetProperty(plSimulationInterface::kStartInactive) && !fActor->readBodyFlag(NX_BF_KINEMATIC)) {
+		if (!pos && !rot)
+			fActor->putToSleep();
+	}
 
 	SendNewLocation(false, true);
 }

Sources/Plasma/PubUtilLib/plResMgr/plRegistryKeyList.cpp

@@ -59,8 +59,10 @@ plRegistryKeyList::~plRegistryKeyList()
 	for (int i = 0; i < fStaticKeys.size(); i++)
 	{
 		plKeyImp* keyImp = fStaticKeys[i];
-		if (!keyImp->ObjectIsLoaded())
+		if (keyImp && !keyImp->ObjectIsLoaded()) {
+			fStaticKeys[i] = nullptr;
 			delete keyImp;
+		}
 	}
 }
 

Sources/Plasma/PubUtilLib/plResMgr/plRegistryNode.cpp

@@ -203,6 +203,7 @@ void plRegistryPageNode::UnloadKeys()
 	for (; it != fKeyLists.end(); it++)
 	{
 		plRegistryKeyList* keyList = it->second;
+		it->second = nullptr;
 		delete keyList;
 	}
 	fKeyLists.clear();

Sources/Plasma/PubUtilLib/plResMgr/plResManager.cpp

@@ -219,14 +219,20 @@ void plResManager::IShutdown()
 	// Shut down the registry (finally!)
 	ILockPages();
 
-	PageSet::const_iterator it;
-	for (it = fAllPages.begin(); it != fAllPages.end(); it++)
-		delete *it;
-	fAllPages.clear();
+	// Unload all keys before actually deleting the pages.
+	// When a key's refcount drops to zero, IKeyUnreffed looks up the key's page.
+	// If the page is already deleted at that point, this causes a use after free and potential crash.
+	for (PageSet::const_iterator it = fAllPages.begin(); it != fAllPages.end(); it++) {
+		(*it)->UnloadKeys();
+	}
 	fLoadedPages.clear();
+	fLastFoundPage = nil;
+	for (PageSet::const_iterator it = fAllPages.begin(); it != fAllPages.end(); it++) {
+		delete *it;
+	}
+	fAllPages.clear();
 
 	IUnlockPages();
-	fLastFoundPage = nil;
 
 	// Now, kill off the Dispatcher
 	hsRefCnt_SafeUnRef(fDispatch);
@@ -1299,7 +1305,7 @@ void plResManager::PageInAge(const char *age)
 
 hsBool plResManager::VerifyPages()
 {
-	hsTArray<plRegistryPageNode*> invalidPages, newerPages;
+	PageSet invalidPages, newerPages;
 
 	// Step 1: verify major/minor version changes
 	if (plResMgrSettings::Get().GetFilterNewerPageVersions() ||
@@ -1313,7 +1319,7 @@ hsBool plResManager::VerifyPages()
 
 			if (page->GetPageCondition() == kPageTooNew && plResMgrSettings::Get().GetFilterNewerPageVersions())
 			{
-				newerPages.Append(page);
+				newerPages.insert(page);
 				fAllPages.erase(page);
 			}
 			else if (
@@ -1321,21 +1327,21 @@ hsBool plResManager::VerifyPages()
 				page->GetPageCondition() == kPageOutOfDate)
 				&& plResMgrSettings::Get().GetFilterOlderPageVersions())
 			{
-				invalidPages.Append(page);
+				invalidPages.insert(page);
 				fAllPages.erase(page);
 			}
 		}
 	}
 
 	// Handle all our invalid pages now
-	if (invalidPages.GetCount() > 0)
+	if (!invalidPages.empty())
 	{
 		if (!IDeleteBadPages(invalidPages, false))
 			return false;
 	}
 
 	// Warn about newer pages
-	if (newerPages.GetCount() > 0)
+	if (!newerPages.empty())
 	{
 		if (!IWarnNewerPages(newerPages))
 			return false;
@@ -1343,22 +1349,35 @@ hsBool plResManager::VerifyPages()
 
 	// Step 2 of verification: make sure no sequence numbers conflict
 	PageSet::iterator it = fAllPages.begin();
-	for (; it != fAllPages.end(); it++)
+	for (; it != fAllPages.end();)
 	{
 		plRegistryPageNode* page = *it;
 
 		PageSet::iterator itUp = it;
 		itUp++;
-		for (; itUp != fAllPages.end(); itUp++)
+		bool amValid = true;
+		for (; itUp != fAllPages.end();)
 		{
 			plRegistryPageNode* upPage = *itUp;
-			if (page->GetPageInfo().GetLocation() == upPage->GetPageInfo().GetLocation())
-			{
-				invalidPages.Append(upPage);
-				fAllPages.erase(itUp);
-				break;
+			if (page->GetPageInfo().GetLocation() == upPage->GetPageInfo().GetLocation()) {
+				invalidPages.insert(upPage);
+				itUp = fAllPages.erase(itUp);
+				amValid = false;
+			} else {
+				itUp++;
 			}
 		}
+
+		// Delete *all* conflicting pages, not just conflicting - 1.
+		// If we leave in a single conflicting page, a new conflict can trivially
+		// be introduced if the page we leave behind is defunct and a new version
+		// is later downloaded from the FileSrv.
+		if (!amValid) {
+			invalidPages.insert(page);
+			it = fAllPages.erase(it);
+		} else {
+			it++;
+		}
 	}
 
 	// Redo our loaded pages list, since Verify() might force the page's keys to load or unload
@@ -1374,7 +1393,7 @@ hsBool plResManager::VerifyPages()
 	}
 
 	// Handle all our conflicting pages now
-	if (invalidPages.GetCount() > 0)
+	if (!invalidPages.empty())
 		return IDeleteBadPages(invalidPages, true);
 
 	return true;
@@ -1384,9 +1403,10 @@ hsBool plResManager::VerifyPages()
 //	Given an array of pages that are invalid (major version out-of-date or
 //	whatnot), asks the user what we should do about them.
 
-static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int bufSize)
+static void ICatPageNames(const std::set<plRegistryPageNode*>& pages, char* buf, int bufSize)
 {
-	for (int i = 0; i < pages.GetCount(); i++)
+	int i = 0;
+	for (auto it = pages.cbegin(); it != pages.cend(); ++it, ++i)
 	{
 		if (i >= 25)
 		{
@@ -1394,7 +1414,7 @@ static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int b
 			break;
 		}
 
-		const char* pagePath = pages[i]->GetPagePath();
+		const char* pagePath = (*it)->GetPagePath();
 		const char* pageFile = plFileUtils::GetFileName(pagePath);
 
 		if (strlen(buf) + strlen(pageFile) > bufSize - 5)
@@ -1408,7 +1428,7 @@ static void ICatPageNames(hsTArray<plRegistryPageNode*>& pages, char* buf, int b
 	}
 }
 
-hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages, hsBool conflictingSeqNums)
+hsBool plResManager::IDeleteBadPages(PageSet& invalidPages, hsBool conflictingSeqNums)
 {
 #ifndef PLASMA_EXTERNAL_RELEASE
 	if (!hsMessageBox_SuppressPrompts)
@@ -1430,12 +1450,12 @@ hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages
 #endif // PLASMA_EXTERNAL_RELEASE
 
 	// Delete 'em
-	for (int i = 0; i < invalidPages.GetCount(); i++)
+	for (PageSet::iterator it = invalidPages.begin(); it != invalidPages.end(); ++it)
 	{
-		invalidPages[i]->DeleteSource();
-		delete invalidPages[i];
+		(*it)->DeleteSource();
+		delete *it;
 	}
-	invalidPages.Reset();
+	invalidPages.clear();
 
 	fLastFoundPage = nil;
 
@@ -1447,7 +1467,7 @@ hsBool plResManager::IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages
 //	than the "current" one), warns the user about them but does nothing to
 //	them.
 
-hsBool plResManager::IWarnNewerPages(hsTArray<plRegistryPageNode*> &newerPages)
+hsBool plResManager::IWarnNewerPages(PageSet &newerPages)
 {
 #ifndef PLASMA_EXTERNAL_RELEASE
 	if (!hsMessageBox_SuppressPrompts)
@@ -1465,9 +1485,9 @@ hsBool plResManager::IWarnNewerPages(hsTArray<plRegistryPageNode*> &newerPages)
 
 
 	// Not deleting the files, just delete them from memory
-	for (int i = 0; i < newerPages.GetCount(); i++)
-		delete newerPages[i];
-	newerPages.Reset();
+	for (PageSet::iterator it = newerPages.begin(); it != newerPages.end(); ++it)
+		delete *it;
+	newerPages.clear();
 
 	fLastFoundPage = nil;
 

Sources/Plasma/PubUtilLib/plResMgr/plResManager.h

@@ -61,6 +61,9 @@ typedef	void(*plProgressProc)(plKey key);
 
 class plResManager : public hsResMgr
 {
+protected:
+	typedef std::set<plRegistryPageNode*> PageSet;
+
 public:
 	plResManager();
 	virtual ~plResManager();
@@ -194,8 +197,8 @@ protected:
 
 	void IUnloadPageKeys(plRegistryPageNode* pageNode, hsBool dontClear = false);
 
-	hsBool IDeleteBadPages(hsTArray<plRegistryPageNode*>& invalidPages, hsBool conflictingSeqNums);
-	hsBool IWarnNewerPages(hsTArray<plRegistryPageNode*>& newerPages);
+	hsBool IDeleteBadPages(PageSet& invalidPages, hsBool conflictingSeqNums);
+	hsBool IWarnNewerPages(PageSet& newerPages);
 
 	void ILockPages();
 	void IUnlockPages();
@@ -233,7 +236,6 @@ protected:
 	UInt8 fPageListLock;	// Number of locks on the page lists.  If it's greater than zero, they can't be modified
 	hsBool fPagesNeedCleanup;	// True if something modified the page lists while they were locked.
 
-	typedef std::set<plRegistryPageNode*> PageSet;
 	PageSet fAllPages;		// All the pages, loaded or not
 	PageSet fLoadedPages;	// Just the loaded pages