Fix vscript ref counted instances

sp/src/game/server/baseanimating.cpp

@@ -317,7 +317,7 @@ BEGIN_ENT_SCRIPTDESC( CBaseAnimating, CBaseEntity, "Animating models" )
 	DEFINE_SCRIPTFUNC_NAMED( ScriptGetSequenceActivity, "GetSequenceActivity", "Gets the activity ID of the specified sequence index" )
 	DEFINE_SCRIPTFUNC_NAMED( ScriptSelectWeightedSequence, "SelectWeightedSequence", "Selects a sequence for the specified activity ID" )
 	DEFINE_SCRIPTFUNC_NAMED( ScriptSelectHeaviestSequence, "SelectHeaviestSequence", "Selects the sequence with the heaviest weight for the specified activity ID" )
-	DEFINE_SCRIPTFUNC_NAMED( ScriptGetSequenceKeyValues, "GetSequenceKeyValues", "Get a KeyValue class instance on the specified sequence. WARNING: This uses the same KeyValue pointer as GetModelKeyValues!" )
+	DEFINE_SCRIPTFUNC_NAMED( ScriptGetSequenceKeyValues, "GetSequenceKeyValues", "Get a KeyValue class instance on the specified sequence" )
 	DEFINE_SCRIPTFUNC( ResetSequenceInfo, "" )
 	DEFINE_SCRIPTFUNC( StudioFrameAdvance, "" )
 	DEFINE_SCRIPTFUNC( GetPlaybackRate, "" )
@@ -2305,21 +2305,14 @@ void CBaseAnimating::ScriptGetBoneTransform( int iBone, HSCRIPT hTransform )
 
 //-----------------------------------------------------------------------------
 // VScript access to sequence's key values
-// for iteration and value access, use:
-//	ScriptFindKey, ScriptGetFirstSubKey, ScriptGetString, 
-//	ScriptGetInt, ScriptGetFloat, ScriptGetNextKey
-// NOTE: This is recycled from ScriptGetModelKeyValues() and uses its pointer!!!
 //-----------------------------------------------------------------------------
-HSCRIPT CBaseAnimating::ScriptGetSequenceKeyValues( int iSequence )
+HSCRIPT_RC CBaseAnimating::ScriptGetSequenceKeyValues( int iSequence )
 {
 	KeyValues *pSeqKeyValues = GetSequenceKeyValues( iSequence );
 	HSCRIPT hScript = NULL;
 	if ( pSeqKeyValues )
 	{
-		// UNDONE: how does destructor get called on this
+		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pSeqKeyValues );
-		m_pScriptModelKeyValues = hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pSeqKeyValues, true );
-
-		// UNDONE: who calls ReleaseInstance on this??? Does name need to be unique???
 	}
 
 	return hScript;

sp/src/game/server/baseanimating.h

@@ -208,7 +208,7 @@ public:
 	int		ScriptSelectHeaviestSequence( int activity ) { return SelectHeaviestSequence( (Activity)activity ); }
 	int		ScriptSelectWeightedSequence( int activity, int curSequence ) { return SelectWeightedSequence( (Activity)activity, curSequence ); }
 
-	HSCRIPT ScriptGetSequenceKeyValues( int iSequence );
+	HSCRIPT_RC ScriptGetSequenceKeyValues( int iSequence );
 
 	// For VScript
 	int		GetSkin() { return m_nSkin; }

sp/src/game/server/baseentity.cpp

@@ -10153,7 +10153,7 @@ void CBaseEntity::SetScriptOwnerEntity(HSCRIPT pOwner)
 //	ScriptFindKey, ScriptGetFirstSubKey, ScriptGetString, 
 //	ScriptGetInt, ScriptGetFloat, ScriptGetNextKey
 //-----------------------------------------------------------------------------
-HSCRIPT CBaseEntity::ScriptGetModelKeyValues( void )
+HSCRIPT_RC CBaseEntity::ScriptGetModelKeyValues( void )
 {
 	KeyValues *pModelKeyValues = new KeyValues("");
 	HSCRIPT hScript = NULL;
@@ -10162,16 +10162,12 @@ HSCRIPT CBaseEntity::ScriptGetModelKeyValues( void )
 
 	if ( pModelKeyValues->LoadFromBuffer( pszModelName, pBuffer ) )
 	{
-		// UNDONE: how does destructor get called on this
 #ifdef MAPBASE_VSCRIPT
-		m_pScriptModelKeyValues = hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pModelKeyValues, true ); // Allow VScript to delete this when the instance is removed.
+		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pModelKeyValues );
 #else
+		// UNDONE: how does destructor get called on this
 		m_pScriptModelKeyValues = new CScriptKeyValues( pModelKeyValues );
-#endif
-
 		// UNDONE: who calls ReleaseInstance on this??? Does name need to be unique???
-
-#ifndef MAPBASE_VSCRIPT
 		hScript = g_pScriptVM->RegisterInstance( m_pScriptModelKeyValues );
 #endif
 		

sp/src/game/server/baseentity.h

@@ -2114,7 +2114,7 @@ public:
 #endif
 
 	const char* ScriptGetModelName(void) const;
-	HSCRIPT ScriptGetModelKeyValues(void);
+	HSCRIPT_RC ScriptGetModelKeyValues(void);
 
 	void ScriptStopSound(const char* soundname);
 	void ScriptEmitSound(const char* soundname);
@@ -2188,9 +2188,7 @@ public:
 	CScriptScope	m_ScriptScope;
 	HSCRIPT			m_hScriptInstance;
 	string_t		m_iszScriptId;
-#ifdef MAPBASE_VSCRIPT
+#ifndef MAPBASE_VSCRIPT
-	HSCRIPT			m_pScriptModelKeyValues;
-#else
 	CScriptKeyValues* m_pScriptModelKeyValues;
 #endif
 };

sp/src/game/server/mapbase/logic_externaldata.cpp

@@ -38,8 +38,8 @@ public:
 	void InputReload( inputdata_t &inputdata );
 
 #ifdef MAPBASE_VSCRIPT
-	HSCRIPT			ScriptGetKeyValues( void );
+	HSCRIPT_RC			ScriptGetKeyValues( void );
-	HSCRIPT			ScriptGetKeyValueBlock( void );
+	HSCRIPT_RC			ScriptGetKeyValueBlock( void );
 
 	void			ScriptSetKeyValues( HSCRIPT hKV );
 	void			ScriptSetKeyValueBlock( HSCRIPT hKV );
@@ -275,7 +275,7 @@ void CLogicExternalData::InputReload( inputdata_t &inputdata )
 #ifdef MAPBASE_VSCRIPT
 //-----------------------------------------------------------------------------
 //-----------------------------------------------------------------------------
-HSCRIPT CLogicExternalData::ScriptGetKeyValues( void )
+HSCRIPT_RC CLogicExternalData::ScriptGetKeyValues( void )
 {
 	if (m_bReloadBeforeEachAction)
 		LoadFile();
@@ -283,8 +283,9 @@ HSCRIPT CLogicExternalData::ScriptGetKeyValues( void )
 	HSCRIPT hScript = NULL;
 	if (m_pRoot)
 	{
-		// Does this need to be destructed or freed? m_pScriptModelKeyValues apparently doesn't.
+		KeyValues *pCopy = new KeyValues( NULL );
-		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, m_pRoot, false );
+		*pCopy = *m_pRoot;
+		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pCopy );
 	}
 
 	return hScript;
@@ -292,7 +293,7 @@ HSCRIPT CLogicExternalData::ScriptGetKeyValues( void )
 
 //-----------------------------------------------------------------------------
 //-----------------------------------------------------------------------------
-HSCRIPT CLogicExternalData::ScriptGetKeyValueBlock( void )
+HSCRIPT_RC CLogicExternalData::ScriptGetKeyValueBlock( void )
 {
 	if (m_bReloadBeforeEachAction)
 		LoadFile();
@@ -300,8 +301,9 @@ HSCRIPT CLogicExternalData::ScriptGetKeyValueBlock( void )
 	HSCRIPT hScript = NULL;
 	if (m_pBlock)
 	{
-		// Does this need to be destructed or freed? m_pScriptModelKeyValues apparently doesn't.
+		KeyValues *pCopy = new KeyValues( NULL );
-		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, m_pBlock, false );
+		*pCopy = *m_pBlock;
+		hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pCopy );
 	}
 
 	return hScript;
@@ -321,7 +323,8 @@ void CLogicExternalData::ScriptSetKeyValues( HSCRIPT hKV )
 	KeyValues *pKV = scriptmanager->GetKeyValuesFromScriptKV( g_pScriptVM, hKV );
 	if (pKV)
 	{
-		m_pRoot = pKV;
+		m_pRoot = new KeyValues( NULL );
+		*m_pRoot = *pKV;
 	}
 }
 
@@ -336,7 +339,8 @@ void CLogicExternalData::ScriptSetKeyValueBlock( HSCRIPT hKV )
 	KeyValues *pKV = scriptmanager->GetKeyValuesFromScriptKV( g_pScriptVM, hKV );
 	if (pKV)
 	{
-		m_pBlock = pKV;
+		m_pBlock = new KeyValues( NULL );
+		*m_pBlock = *pKV;
 	}
 }
 

sp/src/game/shared/mapbase/vscript_funcs_shared.cpp

@@ -255,11 +255,10 @@ void ScriptDispatchSpawn( HSCRIPT hEntity )
 //-----------------------------------------------------------------------------
 //
 //-----------------------------------------------------------------------------
-static HSCRIPT CreateDamageInfo( HSCRIPT hInflictor, HSCRIPT hAttacker, const Vector &vecForce, const Vector &vecDamagePos, float flDamage, int iDamageType )
+static HSCRIPT_RC CreateDamageInfo( HSCRIPT hInflictor, HSCRIPT hAttacker, const Vector &vecForce, const Vector &vecDamagePos, float flDamage, int iDamageType )
 {
-	// The script is responsible for deleting this via DestroyDamageInfo().
 	CTakeDamageInfo *damageInfo = new CTakeDamageInfo( ToEnt(hInflictor), ToEnt(hAttacker), flDamage, iDamageType );
-	HSCRIPT hScript = g_pScriptVM->RegisterInstance( damageInfo );
+	HSCRIPT hScript = g_pScriptVM->RegisterInstance( damageInfo, true );
 
 	damageInfo->SetDamagePosition( vecDamagePos );
 	damageInfo->SetDamageForce( vecForce );
@@ -267,14 +266,8 @@ static HSCRIPT CreateDamageInfo( HSCRIPT hInflictor, HSCRIPT hAttacker, const Ve
 	return hScript;
 }
 
-static void DestroyDamageInfo( HSCRIPT hDamageInfo )
+static void DestroyDamageInfo( HSCRIPT )
 {
-	CTakeDamageInfo *pInfo = HScriptToClass< CTakeDamageInfo >( hDamageInfo );
-	if ( pInfo )
-	{
-		g_pScriptVM->RemoveInstance( hDamageInfo );
-		delete pInfo;
-	}
 }
 
 void ScriptCalculateExplosiveDamageForce( HSCRIPT info, const Vector &vecDir, const Vector &vecForceOrigin, float flScale )
@@ -317,6 +310,8 @@ void ScriptGuessDamageForce( HSCRIPT info, const Vector &vecForceDir, const Vect
 //
 //-----------------------------------------------------------------------------
 BEGIN_SCRIPTDESC_ROOT_NAMED( CScriptGameTrace, "CGameTrace", "trace_t" )
+	DEFINE_SCRIPT_REFCOUNTED_INSTANCE()
+
 	DEFINE_SCRIPTFUNC( DidHitWorld, "Returns whether the trace hit the world entity or not." )
 	DEFINE_SCRIPTFUNC( DidHitNonWorldEntity, "Returns whether the trace hit something other than the world entity." )
 	DEFINE_SCRIPTFUNC( GetEntityIndex, "Returns the index of whatever entity this trace hit." )
@@ -347,7 +342,7 @@ BEGIN_SCRIPTDESC_ROOT_NAMED( CScriptGameTrace, "CGameTrace", "trace_t" )
 	DEFINE_SCRIPTFUNC( Surface, "" )
 	DEFINE_SCRIPTFUNC( Plane, "" )
 
-	DEFINE_SCRIPTFUNC( Destroy, "Deletes this instance. Important for preventing memory leaks." )
+	DEFINE_SCRIPTFUNC( Destroy, SCRIPT_HIDE )
 END_SCRIPTDESC();
 
 BEGIN_SCRIPTDESC_ROOT_NAMED( scriptsurfacedata_t, "surfacedata_t", "" )
@@ -379,9 +374,8 @@ CPlaneTInstanceHelper g_PlaneTInstanceHelper;
 BEGIN_SCRIPTDESC_ROOT_WITH_HELPER( cplane_t, "", &g_PlaneTInstanceHelper )
 END_SCRIPTDESC();
 
-static HSCRIPT ScriptTraceLineComplex( const Vector &vecStart, const Vector &vecEnd, HSCRIPT entIgnore, int iMask, int iCollisionGroup )
+static HSCRIPT_RC ScriptTraceLineComplex( const Vector &vecStart, const Vector &vecEnd, HSCRIPT entIgnore, int iMask, int iCollisionGroup )
 {
-	// The script is responsible for deleting this via Destroy().
 	CScriptGameTrace *tr = new CScriptGameTrace();
 
 	CBaseEntity *pIgnore = ToEnt( entIgnore );
@@ -390,13 +384,12 @@ static HSCRIPT ScriptTraceLineComplex( const Vector &vecStart, const Vector &vec
 	tr->RegisterSurface();
 	tr->RegisterPlane();
 
-	return tr->GetScriptInstance();
+	return g_pScriptVM->RegisterInstance( tr, true );
 }
 
-static HSCRIPT ScriptTraceHullComplex( const Vector &vecStart, const Vector &vecEnd, const Vector &hullMin, const Vector &hullMax,
+static HSCRIPT_RC ScriptTraceHullComplex( const Vector &vecStart, const Vector &vecEnd, const Vector &hullMin, const Vector &hullMax,
 	HSCRIPT entIgnore, int iMask, int iCollisionGroup )
 {
-	// The script is responsible for deleting this via Destroy().
 	CScriptGameTrace *tr = new CScriptGameTrace();
 
 	CBaseEntity *pIgnore = ToEnt( entIgnore );
@@ -405,7 +398,7 @@ static HSCRIPT ScriptTraceHullComplex( const Vector &vecStart, const Vector &vec
 	tr->RegisterSurface();
 	tr->RegisterPlane();
 
-	return tr->GetScriptInstance();
+	return g_pScriptVM->RegisterInstance( tr, true );
 }
 
 //-----------------------------------------------------------------------------
@@ -476,12 +469,11 @@ void FireBulletsInfo_t::ScriptSetAdditionalIgnoreEnt( HSCRIPT value )
 	m_pAdditionalIgnoreEnt = ToEnt( value );
 }
 
-static HSCRIPT CreateFireBulletsInfo( int cShots, const Vector &vecSrc, const Vector &vecDirShooting,
+static HSCRIPT_RC CreateFireBulletsInfo( int cShots, const Vector &vecSrc, const Vector &vecDirShooting,
 	const Vector &vecSpread, float iDamage, HSCRIPT pAttacker )
 {
-	// The script is responsible for deleting this via DestroyFireBulletsInfo().
 	FireBulletsInfo_t *info = new FireBulletsInfo_t();
-	HSCRIPT hScript = g_pScriptVM->RegisterInstance( info );
+	HSCRIPT hScript = g_pScriptVM->RegisterInstance( info, true );
 
 	info->SetShots( cShots );
 	info->SetSource( vecSrc );
@@ -493,14 +485,8 @@ static HSCRIPT CreateFireBulletsInfo( int cShots, const Vector &vecSrc, const Ve
 	return hScript;
 }
 
-static void DestroyFireBulletsInfo( HSCRIPT hBulletsInfo )
+static void DestroyFireBulletsInfo( HSCRIPT )
 {
-	FireBulletsInfo_t *pInfo = HScriptToClass< FireBulletsInfo_t >( hBulletsInfo );
-	if ( pInfo )
-	{
-		g_pScriptVM->RemoveInstance( hBulletsInfo );
-		delete pInfo;
-	}
 }
 
 //-----------------------------------------------------------------------------
@@ -1043,14 +1029,14 @@ void RegisterSharedScriptFunctions()
 #endif
 
 	ScriptRegisterFunction( g_pScriptVM, CreateDamageInfo, "" );
-	ScriptRegisterFunction( g_pScriptVM, DestroyDamageInfo, "" );
+	ScriptRegisterFunction( g_pScriptVM, DestroyDamageInfo, SCRIPT_HIDE );
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptCalculateExplosiveDamageForce, "CalculateExplosiveDamageForce", "Fill out a damage info handle with a damage force for an explosive." );
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptCalculateBulletDamageForce, "CalculateBulletDamageForce", "Fill out a damage info handle with a damage force for a bullet impact." );
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptCalculateMeleeDamageForce, "CalculateMeleeDamageForce", "Fill out a damage info handle with a damage force for a melee impact." );
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptGuessDamageForce, "GuessDamageForce", "Try and guess the physics force to use." );
 
 	ScriptRegisterFunction( g_pScriptVM, CreateFireBulletsInfo, "" );
-	ScriptRegisterFunction( g_pScriptVM, DestroyFireBulletsInfo, "" );
+	ScriptRegisterFunction( g_pScriptVM, DestroyFireBulletsInfo, SCRIPT_HIDE );
 
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptTraceLineComplex, "TraceLineComplex", "Complex version of TraceLine which takes 2 points, an ent to ignore, a trace mask, and a collision group. Returns a handle which can access all trace info." );
 	ScriptRegisterFunctionNamed( g_pScriptVM, ScriptTraceHullComplex, "TraceHullComplex", "Takes 2 points, min/max hull bounds, an ent to ignore, a trace mask, and a collision group to trace to a point using a hull. Returns a handle which can access all trace info." );

sp/src/game/shared/mapbase/vscript_funcs_shared.h

@@ -98,16 +98,10 @@ class CScriptGameTrace : public CGameTrace
 public:
 	CScriptGameTrace() : m_surfaceAccessor(NULL), m_planeAccessor(NULL)
 	{
-		m_hScriptInstance = g_pScriptVM->RegisterInstance( this );
 	}
 
 	~CScriptGameTrace()
 	{
-		if ( m_hScriptInstance )
-		{
-			g_pScriptVM->RemoveInstance( m_hScriptInstance );
-		}
-
 		if ( m_surfaceAccessor )
 		{
 			g_pScriptVM->RemoveInstance( m_surfaceAccessor );
@@ -130,11 +124,6 @@ public:
 		m_planeAccessor = g_pScriptVM->RegisterInstance( &plane );
 	}
 
-	HSCRIPT GetScriptInstance() const
-	{
-		return m_hScriptInstance;
-	}
-
 public:
 	float FractionLeftSolid() const		{ return fractionleftsolid; }
 	int HitGroup() const				{ return hitgroup; }
@@ -157,12 +146,11 @@ public:
 	HSCRIPT Surface() const				{ return m_surfaceAccessor; }
 	HSCRIPT Plane() const				{ return m_planeAccessor; }
 
-	void Destroy()						{ delete this; }
+	void Destroy()						{}
 
 private:
 	HSCRIPT m_surfaceAccessor;
 	HSCRIPT m_planeAccessor;
-	HSCRIPT m_hScriptInstance;
 
 	CSurfaceScriptHelper m_surfaceHelper;
 

sp/src/game/shared/mapbase/vscript_singletons.cpp

@@ -3247,7 +3247,7 @@ public:
 
 	// NOTE: These two functions are new with Mapbase and have no Valve equivalent
 	static bool KeyValuesWrite( const char *szFile, HSCRIPT hInput );
-	static HSCRIPT KeyValuesRead( const char *szFile );
+	static HSCRIPT_RC KeyValuesRead( const char *szFile );
 
 	void LevelShutdownPostEntity()
 	{
@@ -3433,7 +3433,7 @@ bool CScriptReadWriteFile::KeyValuesWrite( const char *szFile, HSCRIPT hInput )
 //-----------------------------------------------------------------------------
 //
 //-----------------------------------------------------------------------------
-HSCRIPT CScriptReadWriteFile::KeyValuesRead( const char *szFile )
+HSCRIPT_RC CScriptReadWriteFile::KeyValuesRead( const char *szFile )
 {
 	char pszFullName[MAX_PATH];
 	V_snprintf( pszFullName, sizeof(pszFullName), SCRIPT_RW_FULL_PATH_FMT, szFile );
@@ -3458,7 +3458,7 @@ HSCRIPT CScriptReadWriteFile::KeyValuesRead( const char *szFile )
 		return NULL;
 	}
 
-	HSCRIPT hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV, true ); // bAllowDestruct is supposed to automatically remove the involved KV
+	HSCRIPT hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV );
 
 	return hScript;
 }

sp/src/game/shared/takedamageinfo.cpp

@@ -33,6 +33,8 @@ END_DATADESC()
 
 #ifdef MAPBASE_VSCRIPT
 BEGIN_SCRIPTDESC_ROOT( CTakeDamageInfo, "Damage information handler." )
+	DEFINE_SCRIPT_REFCOUNTED_INSTANCE()
+
 	DEFINE_SCRIPTFUNC_NAMED( ScriptGetInflictor, "GetInflictor", "Gets the inflictor." )
 	DEFINE_SCRIPTFUNC_NAMED( ScriptSetInflictor, "SetInflictor", "Sets the inflictor." )
 	DEFINE_SCRIPTFUNC_NAMED( ScriptGetWeapon, "GetWeapon", "Gets the weapon." )
@@ -590,4 +592,4 @@ void CTakeDamageInfo::DebugGetDamageTypeString(unsigned int damageType, char *ou
 #define DMG_BLAST_SURFACE	(1<<27)		// A blast on the surface of water that cannot harm things underwater
 #define DMG_DIRECT			(1<<28)
 #define DMG_BUCKSHOT		(1<<29)		// not quite a bullet. Little, rounder, different.
-*/
+*/

sp/src/public/vscript/ivscript.h

@@ -141,6 +141,20 @@ class KeyValues;
 DECLARE_POINTER_HANDLE( HSCRIPT );
 #define INVALID_HSCRIPT ((HSCRIPT)-1)
 
+// Reference counted HSCRIPT return value
+//
+// This is an alias for HSCRIPT that is converted back to HSCRIPT on return
+// from vscript function bindings; it signals the vscript implementation to
+// release its hold and let the script control the lifetime
+// of the registered instance.
+struct HSCRIPT_RC
+{
+	HSCRIPT val;
+	HSCRIPT_RC( HSCRIPT v ) { val = v; }
+	HSCRIPT operator=( HSCRIPT v ) { val = v; return val; }
+	operator HSCRIPT() { return val; }
+};
+
 typedef unsigned int HScriptRaw;
 #endif
 
@@ -162,7 +176,7 @@ public:
 	virtual void DestroyVM( IScriptVM * ) = 0;
 
 #ifdef MAPBASE_VSCRIPT
-	virtual HSCRIPT CreateScriptKeyValues( IScriptVM *pVM, KeyValues *pKV, bool bAllowDestruct ) = 0;
+	virtual HSCRIPT CreateScriptKeyValues( IScriptVM *pVM, KeyValues *pKV, bool bBorrow = false ) = 0;
 	virtual KeyValues *GetKeyValuesFromScriptKV( IScriptVM *pVM, HSCRIPT hSKV ) = 0;
 #endif
 };
@@ -177,6 +191,9 @@ enum ExtendedFieldType
 	FIELD_CSTRING,
 	FIELD_HSCRIPT,
 	FIELD_VARIANT,
+#ifdef MAPBASE_VSCRIPT
+	FIELD_HSCRIPT_RC,
+#endif
 };
 
 typedef int ScriptDataType_t;
@@ -197,6 +214,7 @@ DECLARE_DEDUCE_FIELDTYPE( FIELD_CHARACTER,	char );
 DECLARE_DEDUCE_FIELDTYPE( FIELD_HSCRIPT,	HSCRIPT );
 DECLARE_DEDUCE_FIELDTYPE( FIELD_VARIANT,	ScriptVariant_t );
 #ifdef MAPBASE_VSCRIPT
+DECLARE_DEDUCE_FIELDTYPE( FIELD_HSCRIPT_RC,	HSCRIPT_RC );
 DECLARE_DEDUCE_FIELDTYPE( FIELD_VECTOR,		QAngle );
 DECLARE_DEDUCE_FIELDTYPE( FIELD_VECTOR,		const QAngle& );
 #endif
@@ -298,6 +316,9 @@ struct ScriptMemberDesc_t
 enum ScriptFuncBindingFlags_t
 {
 	SF_MEMBER_FUNC	= 0x01,
+#ifdef MAPBASE_VSCRIPT
+	SF_REFCOUNTED_RET	= 0x02,
+#endif
 };
 
 typedef bool (*ScriptBindingFunc_t)( void *pFunction, void *pContext, ScriptVariant_t *pArguments, int nArguments, ScriptVariant_t *pReturn );
@@ -619,6 +640,15 @@ struct ScriptEnumDesc_t
 #define ScriptInitMemberFunctionBindingNamed( pScriptFunction, class, func, scriptName )	ScriptInitMemberFunctionBinding_( pScriptFunction, class, func, scriptName )
 #define ScriptInitMemberFunctionBinding_( pScriptFunction, class, func, scriptName ) 		do { ScriptInitMemberFuncDescriptor_( (&(pScriptFunction)->m_desc), class, func, scriptName ); (pScriptFunction)->m_pfnBinding = ScriptCreateBinding( ((class *)0), &class::func ); 	(pScriptFunction)->m_pFunction = ScriptConvertFuncPtrToVoid( &class::func ); (pScriptFunction)->m_flags = SF_MEMBER_FUNC;  } while (0)
 
+#ifdef MAPBASE_VSCRIPT
+// Convert HSCRIPT_RC return type into HSCRIPT return with SF_REFCOUNTED_RET binding flag
+#undef ScriptInitFunctionBindingNamed
+#define ScriptInitFunctionBindingNamed( pScriptFunction, func, scriptName )					do { ScriptInitFuncDescriptorNamed( (&(pScriptFunction)->m_desc), func, scriptName ); (pScriptFunction)->m_pfnBinding = ScriptCreateBinding( &func ); (pScriptFunction)->m_pFunction = (void *)&func; if ( (pScriptFunction)->m_desc.m_ReturnType == FIELD_HSCRIPT_RC ) { (pScriptFunction)->m_desc.m_ReturnType = FIELD_HSCRIPT; (pScriptFunction)->m_flags |= SF_REFCOUNTED_RET; } } while (0)
+
+#undef ScriptInitMemberFunctionBinding_
+#define ScriptInitMemberFunctionBinding_( pScriptFunction, class, func, scriptName ) 		do { ScriptInitMemberFuncDescriptor_( (&(pScriptFunction)->m_desc), class, func, scriptName ); (pScriptFunction)->m_pfnBinding = ScriptCreateBinding( ((class *)0), &class::func ); 	(pScriptFunction)->m_pFunction = ScriptConvertFuncPtrToVoid( &class::func ); (pScriptFunction)->m_flags = SF_MEMBER_FUNC; if ( (pScriptFunction)->m_desc.m_ReturnType == FIELD_HSCRIPT_RC ) { (pScriptFunction)->m_desc.m_ReturnType = FIELD_HSCRIPT; (pScriptFunction)->m_flags |= SF_REFCOUNTED_RET; } } while (0)
+#endif
+
 #define ScriptInitClassDesc( pClassDesc, class, pBaseClassDesc )							ScriptInitClassDescNamed( pClassDesc, class, pBaseClassDesc, #class )
 #define ScriptInitClassDescNamed( pClassDesc, class, pBaseClassDesc, scriptName )			ScriptInitClassDescNamed_( pClassDesc, class, pBaseClassDesc, scriptName )
 #define ScriptInitClassDescNoBase( pClassDesc, class )										ScriptInitClassDescNoBaseNamed( pClassDesc, class, #class )
@@ -751,6 +781,10 @@ static inline int ToConstantVariant(int value)
 #define DEFINE_SCRIPT_INSTANCE_HELPER( p ) MUST_USE_BEGIN_SCRIPTDESC_WITH_HELPER_INSTEAD
 
 #ifdef MAPBASE_VSCRIPT
+// Allow instance to be deleted but not constructed
+// Not needed if the class has a constructor
+#define DEFINE_SCRIPT_REFCOUNTED_INSTANCE()													do { pDesc->m_pfnDestruct = &CScriptConstructor<_className>::Destruct; } while (0);
+
 // Use this for hooks which have no parameters
 #define DEFINE_SIMPLE_SCRIPTHOOK( hook, hookName, returnType, description ) \
 	if (!hook.m_bDefined) \
@@ -939,14 +973,13 @@ public:
 	//--------------------------------------------------------
 
 #ifdef MAPBASE_VSCRIPT
-	// When a RegisterInstance instance is deleted, VScript normally treats it as a strong reference and only deregisters the instance itself, preserving the registered data
+	// if bRefCounted is true, pInstance memory will be deleted by the script,
-	// it points to so the game can continue to use it.
+	// returning the result will then behave as if the instance was constructed in script.
-	// bAllowDestruct is supposed to allow VScript to treat it as a weak reference created by the script, destructing the registered data automatically like any other type.
+	// Functions that return the result of this need to return HSCRIPT_RC
-	// This is useful for classes pretending to be primitive types.
+	virtual HSCRIPT RegisterInstance( ScriptClassDesc_t *pDesc, void *pInstance, bool bRefCounted = false ) = 0;
-	virtual HSCRIPT RegisterInstance( ScriptClassDesc_t *pDesc, void *pInstance, bool bAllowDestruct = false ) = 0;
 	virtual void SetInstanceUniqeId( HSCRIPT hInstance, const char *pszId ) = 0;
-	template <typename T> HSCRIPT RegisterInstance( T *pInstance, bool bAllowDestruct = false )																	{ return RegisterInstance( GetScriptDesc( pInstance ), pInstance, bAllowDestruct );	}
+	template <typename T> HSCRIPT RegisterInstance( T *pInstance, bool bRefCounted = false )																	{ return RegisterInstance( GetScriptDesc( pInstance ), pInstance, bRefCounted );	}
-	template <typename T> HSCRIPT RegisterInstance( T *pInstance, const char *pszInstance, HSCRIPT hScope = NULL, bool bAllowDestruct = false)					{ HSCRIPT hInstance = RegisterInstance( GetScriptDesc( pInstance ), pInstance, bAllowDestruct ); SetValue( hScope, pszInstance, hInstance ); return hInstance; }
+	template <typename T> HSCRIPT RegisterInstance( T *pInstance, const char *pszInstance, HSCRIPT hScope = NULL, bool bRefCounted = false)					{ HSCRIPT hInstance = RegisterInstance( GetScriptDesc( pInstance ), pInstance, bRefCounted ); SetValue( hScope, pszInstance, hInstance ); return hInstance; }
 #else
 	virtual HSCRIPT RegisterInstance( ScriptClassDesc_t *pDesc, void *pInstance ) = 0;
 	virtual void SetInstanceUniqeId( HSCRIPT hInstance, const char *pszId ) = 0;

sp/src/utils/vbsp/vscript_funcs_vmfs.cpp

@@ -18,7 +18,7 @@
 //-----------------------------------------------------------------------------
 //-----------------------------------------------------------------------------
 
-static HSCRIPT VMFKV_CreateBlank()
+static HSCRIPT_RC VMFKV_CreateBlank()
 {
 	KeyValues *pKV = new KeyValues("VMF");
 
@@ -28,7 +28,7 @@ static HSCRIPT VMFKV_CreateBlank()
 		pWorld->SetString( "classname", "worldspawn" );
 	}
 
-	return scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV, true );
+	return scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV );
 }
 
 static bool VMFKV_SaveToFile( const char *szFile, HSCRIPT hKV )
@@ -69,7 +69,7 @@ static bool VMFKV_SaveToFile( const char *szFile, HSCRIPT hKV )
 	return res;
 }
 
-static HSCRIPT VMFKV_LoadFromFile( const char *szFile )
+static HSCRIPT_RC VMFKV_LoadFromFile( const char *szFile )
 {
 	char pszFullName[MAX_PATH];
 	V_snprintf( pszFullName, sizeof(pszFullName), NULL, szFile );
@@ -87,7 +87,7 @@ static HSCRIPT VMFKV_LoadFromFile( const char *szFile )
 		return NULL;
 	}
 
-	HSCRIPT hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV, true ); // bAllowDestruct is supposed to automatically remove the involved KV
+	HSCRIPT hScript = scriptmanager->CreateScriptKeyValues( g_pScriptVM, pKV );
 
 	return hScript;
 }
@@ -142,7 +142,7 @@ static HSCRIPT VMFKV_AddEntityFromTables( HSCRIPT hVMF, HSCRIPT hKV, HSCRIPT hIO
 		}
 	}
 
-	return scriptmanager->CreateScriptKeyValues( g_pScriptVM, pEnt, false );
+	return scriptmanager->CreateScriptKeyValues( g_pScriptVM, pEnt, true );
 }
 
 //-----------------------------------------------------------------------------

sp/src/vscript/vscript.cpp

@@ -60,10 +60,14 @@ public:
 	}
 
 	// Mapbase moves CScriptKeyValues into the library so it could be used elsewhere
-	virtual HSCRIPT CreateScriptKeyValues( IScriptVM *pVM, KeyValues *pKV, bool bAllowDestruct ) override
+
+	// if bBorrow is false, CScriptKeyValues owns pKV memory
+	// Functions returning the result need to return HSCRIPT_RC
+	// see comment on IScriptVM::RegisterInstance()
+	virtual HSCRIPT CreateScriptKeyValues( IScriptVM *pVM, KeyValues *pKV, bool bBorrow ) override
 	{
-		CScriptKeyValues *pSKV = new CScriptKeyValues( pKV );
+		CScriptKeyValues *pSKV = new CScriptKeyValues( pKV, bBorrow );
-		HSCRIPT hSKV = pVM->RegisterInstance( pSKV, bAllowDestruct );
+		HSCRIPT hSKV = pVM->RegisterInstance( pSKV, true );
 		return hSKV;
 	}
 
@@ -72,11 +76,11 @@ public:
 		CScriptKeyValues *pSKV = (hSKV ? (CScriptKeyValues*)pVM->GetInstanceValue( hSKV, GetScriptDesc( (CScriptKeyValues*)NULL ) ) : nullptr);
 		if (pSKV)
 		{
-			return pSKV->m_pKeyValues;
+			return pSKV->GetKeyValues();
 		}
 
 		return nullptr;
 	}
 };
 
-EXPOSE_SINGLE_INTERFACE(CScriptManager, IScriptManager, VSCRIPT_INTERFACE_VERSION);
+EXPOSE_SINGLE_INTERFACE(CScriptManager, IScriptManager, VSCRIPT_INTERFACE_VERSION);

sp/src/vscript/vscript_bindings_base.cpp

@@ -106,7 +106,7 @@ BEGIN_SCRIPTDESC_ROOT( CScriptKeyValues, "Wrapper class over KeyValues instance"
 	DEFINE_SCRIPTFUNC_NAMED( ScriptGetKeyValueBool, "GetKeyBool", "Given a KeyValues object and a key name, return associated bool value" );
 	DEFINE_SCRIPTFUNC_NAMED( ScriptGetKeyValueString, "GetKeyString", "Given a KeyValues object and a key name, return associated string value" );
 	DEFINE_SCRIPTFUNC_NAMED( ScriptIsKeyValueEmpty, "IsKeyEmpty", "Given a KeyValues object and a key name, return true if key name has no value" );
-	DEFINE_SCRIPTFUNC_NAMED( ScriptReleaseKeyValues, "ReleaseKeyValues", "Given a root KeyValues object, release its contents" );
+	DEFINE_SCRIPTFUNC_NAMED( ScriptReleaseKeyValues, "ReleaseKeyValues", SCRIPT_HIDE );
 
 	DEFINE_SCRIPTFUNC( TableToSubKeys, "Converts a script table to KeyValues." );
 	DEFINE_SCRIPTFUNC( SubKeysToTable, "Converts to script table." );
@@ -131,79 +131,84 @@ BEGIN_SCRIPTDESC_ROOT( CScriptKeyValues, "Wrapper class over KeyValues instance"
 	DEFINE_SCRIPTFUNC_NAMED( ScriptSetString, "SetString", "Given a KeyValues object, set its own associated string value" );
 END_SCRIPTDESC();
 
-HSCRIPT CScriptKeyValues::ScriptFindKey( const char *pszName )
+HSCRIPT_RC CScriptKeyValues::ScriptFindKey( const char *pszName )
 {
-	KeyValues *pKeyValues = m_pKeyValues->FindKey(pszName);
+	KeyValues *pKeyValues = GetKeyValues()->FindKey(pszName);
 	if ( pKeyValues == NULL )
 		return NULL;
 
-	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues );
+	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues, true );
+	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey, true );
+
+	pScriptKey->m_pBase = m_pSelf;
+	pScriptKey->m_pBase->AddRef();
 
-	// UNDONE: who calls ReleaseInstance on this??
-	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey );
 	return hScriptInstance;
 }
 
-HSCRIPT CScriptKeyValues::ScriptGetFirstSubKey( void )
+HSCRIPT_RC CScriptKeyValues::ScriptGetFirstSubKey( void )
 {
-	KeyValues *pKeyValues = m_pKeyValues->GetFirstSubKey();
+	KeyValues *pKeyValues = GetKeyValues()->GetFirstSubKey();
 	if ( pKeyValues == NULL )
 		return NULL;
 
-	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues );
+	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues, true );
+	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey, true );
+
+	pScriptKey->m_pBase = m_pSelf;
+	pScriptKey->m_pBase->AddRef();
 
-	// UNDONE: who calls ReleaseInstance on this??
-	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey );
 	return hScriptInstance;
 }
 
-HSCRIPT CScriptKeyValues::ScriptGetNextKey( void )
+HSCRIPT_RC CScriptKeyValues::ScriptGetNextKey( void )
 {
-	KeyValues *pKeyValues = m_pKeyValues->GetNextKey();
+	KeyValues *pKeyValues = GetKeyValues()->GetNextKey();
 	if ( pKeyValues == NULL )
 		return NULL;
 
-	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues );
+	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues, true );
+	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey, true );
+
+	// if I don't have a parent, then I own my siblings
+	pScriptKey->m_pBase = m_pBase ? m_pBase : m_pSelf;
+	pScriptKey->m_pBase->AddRef();
 
-	// UNDONE: who calls ReleaseInstance on this??
-	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey );
 	return hScriptInstance;
 }
 
 int CScriptKeyValues::ScriptGetKeyValueInt( const char *pszName )
 {
-	int i = m_pKeyValues->GetInt( pszName );
+	int i = GetKeyValues()->GetInt( pszName );
 	return i;
 }
 
 float CScriptKeyValues::ScriptGetKeyValueFloat( const char *pszName )
 {
-	float f = m_pKeyValues->GetFloat( pszName );
+	float f = GetKeyValues()->GetFloat( pszName );
 	return f;
 }
 
 const char *CScriptKeyValues::ScriptGetKeyValueString( const char *pszName )
 {
-	const char *psz = m_pKeyValues->GetString( pszName );
+	const char *psz = GetKeyValues()->GetString( pszName );
 	return psz;
 }
 
 bool CScriptKeyValues::ScriptIsKeyValueEmpty( const char *pszName )
 {
-	bool b = m_pKeyValues->IsEmpty( pszName );
+	bool b = GetKeyValues()->IsEmpty( pszName );
 	return b;
 }
 
 bool CScriptKeyValues::ScriptGetKeyValueBool( const char *pszName )
 {
-	bool b = m_pKeyValues->GetBool( pszName );
+	bool b = GetKeyValues()->GetBool( pszName );
 	return b;
 }
 
 void CScriptKeyValues::ScriptReleaseKeyValues( )
 {
-	m_pKeyValues->deleteThis();
-	m_pKeyValues = NULL;
 }
 
 void KeyValues_TableToSubKeys( HSCRIPT hTable, KeyValues *pKV )
@@ -259,125 +264,134 @@ void KeyValues_SubKeysToTable( KeyValues *pKV, HSCRIPT hTable )
 
 void CScriptKeyValues::TableToSubKeys( HSCRIPT hTable )
 {
-	KeyValues_TableToSubKeys( hTable, m_pKeyValues );
+	KeyValues_TableToSubKeys( hTable, GetKeyValues() );
 }
 
 void CScriptKeyValues::SubKeysToTable( HSCRIPT hTable )
 {
-	KeyValues_SubKeysToTable( m_pKeyValues, hTable );
+	KeyValues_SubKeysToTable( GetKeyValues(), hTable );
 }
 
-HSCRIPT CScriptKeyValues::ScriptFindOrCreateKey( const char *pszName )
+HSCRIPT_RC CScriptKeyValues::ScriptFindOrCreateKey( const char *pszName )
 {
-	KeyValues *pKeyValues = m_pKeyValues->FindKey(pszName, true);
+	KeyValues *pKeyValues = GetKeyValues()->FindKey(pszName, true);
 	if ( pKeyValues == NULL )
 		return NULL;
 
-	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues );
+	CScriptKeyValues *pScriptKey = new CScriptKeyValues( pKeyValues, true );
+	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey, true );
+
+	pScriptKey->m_pBase = m_pSelf;
+	pScriptKey->m_pBase->AddRef();
 
-	// UNDONE: who calls ReleaseInstance on this??
-	HSCRIPT hScriptInstance = g_pScriptVM->RegisterInstance( pScriptKey );
 	return hScriptInstance;
 }
 
 const char *CScriptKeyValues::ScriptGetName()
 {
-	const char *psz = m_pKeyValues->GetName();
+	const char *psz = GetKeyValues()->GetName();
 	return psz;
 }
 
 int CScriptKeyValues::ScriptGetInt()
 {
-	int i = m_pKeyValues->GetInt();
+	int i = GetKeyValues()->GetInt();
 	return i;
 }
 
 float CScriptKeyValues::ScriptGetFloat()
 {
-	float f = m_pKeyValues->GetFloat();
+	float f = GetKeyValues()->GetFloat();
 	return f;
 }
 
 const char *CScriptKeyValues::ScriptGetString()
 {
-	const char *psz = m_pKeyValues->GetString();
+	const char *psz = GetKeyValues()->GetString();
 	return psz;
 }
 
 bool CScriptKeyValues::ScriptGetBool()
 {
-	bool b = m_pKeyValues->GetBool();
+	bool b = GetKeyValues()->GetBool();
 	return b;
 }
 
 
 void CScriptKeyValues::ScriptSetKeyValueInt( const char *pszName, int iValue )
 {
-	m_pKeyValues->SetInt( pszName, iValue );
+	GetKeyValues()->SetInt( pszName, iValue );
 }
 
 void CScriptKeyValues::ScriptSetKeyValueFloat( const char *pszName, float flValue )
 {
-	m_pKeyValues->SetFloat( pszName, flValue );
+	GetKeyValues()->SetFloat( pszName, flValue );
 }
 
 void CScriptKeyValues::ScriptSetKeyValueString( const char *pszName, const char *pszValue )
 {
-	m_pKeyValues->SetString( pszName, pszValue );
+	GetKeyValues()->SetString( pszName, pszValue );
 }
 
 void CScriptKeyValues::ScriptSetKeyValueBool( const char *pszName, bool bValue )
 {
-	m_pKeyValues->SetBool( pszName, bValue );
+	GetKeyValues()->SetBool( pszName, bValue );
 }
 
 void CScriptKeyValues::ScriptSetName( const char *pszValue )
 {
-	m_pKeyValues->SetName( pszValue );
+	GetKeyValues()->SetName( pszValue );
 }
 
 void CScriptKeyValues::ScriptSetInt( int iValue )
 {
-	m_pKeyValues->SetInt( NULL, iValue );
+	GetKeyValues()->SetInt( NULL, iValue );
 }
 
 void CScriptKeyValues::ScriptSetFloat( float flValue )
 {
-	m_pKeyValues->SetFloat( NULL, flValue );
+	GetKeyValues()->SetFloat( NULL, flValue );
 }
 
 void CScriptKeyValues::ScriptSetString( const char *pszValue )
 {
-	m_pKeyValues->SetString( NULL, pszValue );
+	GetKeyValues()->SetString( NULL, pszValue );
 }
 
 void CScriptKeyValues::ScriptSetBool( bool bValue )
 {
-	m_pKeyValues->SetBool( NULL, bValue );
+	GetKeyValues()->SetBool( NULL, bValue );
 }
 
 
 // constructors
-CScriptKeyValues::CScriptKeyValues( KeyValues *pKeyValues = NULL )
+CScriptKeyValues::CScriptKeyValues( KeyValues *pKeyValues = NULL, bool bBorrow = false ) :
+	m_pBase( NULL )
 {
 	if (pKeyValues == NULL)
 	{
-		m_pKeyValues = new KeyValues("CScriptKeyValues");
+		pKeyValues = new KeyValues("CScriptKeyValues");
-	}
+		// Borrowed new memory doesn't make sense, are you trying to leak?
-	else
+		Assert( !bBorrow );
-	{
-		m_pKeyValues = pKeyValues;
 	}
+
+	m_pSelf = new KeyValues_RC( pKeyValues, bBorrow );
 }
 
 // destructor
 CScriptKeyValues::~CScriptKeyValues( )
 {
-	if (m_pKeyValues)
+	Assert( m_pSelf != m_pBase );
+
+	// Children are always borrowed
+	Assert( !m_pBase || m_pSelf->borrow );
+
+	m_pSelf->Release();
+
+	if ( m_pBase )
 	{
-		m_pKeyValues->deleteThis();
+		m_pBase->Release();
 	}
-	m_pKeyValues = NULL;
 }
 
 //=============================================================================

sp/src/vscript/vscript_bindings_base.h

@@ -20,12 +20,12 @@
 class CScriptKeyValues
 {
 public:
-	CScriptKeyValues( KeyValues *pKeyValues );
+	CScriptKeyValues( KeyValues *pKeyValues, bool bBorrow );
 	~CScriptKeyValues( );
 
-	HSCRIPT ScriptFindKey( const char *pszName );
+	HSCRIPT_RC ScriptFindKey( const char *pszName );
-	HSCRIPT ScriptGetFirstSubKey( void );
+	HSCRIPT_RC ScriptGetFirstSubKey( void );
-	HSCRIPT ScriptGetNextKey( void );
+	HSCRIPT_RC ScriptGetNextKey( void );
 	int ScriptGetKeyValueInt( const char *pszName );
 	float ScriptGetKeyValueFloat( const char *pszName );
 	const char *ScriptGetKeyValueString( const char *pszName );
@@ -37,7 +37,7 @@ public:
 	void TableToSubKeys( HSCRIPT hTable );
 	void SubKeysToTable( HSCRIPT hTable );
 
-	HSCRIPT ScriptFindOrCreateKey( const char *pszName );
+	HSCRIPT_RC ScriptFindOrCreateKey( const char *pszName );
 
 	const char *ScriptGetName();
 	int ScriptGetInt();
@@ -55,9 +55,54 @@ public:
 	void ScriptSetString( const char *pszValue );
 	void ScriptSetBool( bool bValue );
 
-	KeyValues *GetKeyValues() { return m_pKeyValues; }
+	KeyValues *GetKeyValues() { return m_pSelf->ptr; }
 
-	KeyValues *m_pKeyValues;	// actual KeyValue entity
+	// The lifetime of the KeyValues pointer needs to be decoupled from refcounted script objects
+	// because base kv script objects can be released while their children live: kv = kv.GetFirstSubKey()
+	// Refcounting externally allows children to extend the lifetime of KeyValues while
+	// being able to automatically dispose of CScriptKeyValues and HSCRIPT objects with
+	// script refcounted HSCRIPT_RC
+
+	struct KeyValues_RC
+	{
+		KeyValues *ptr;
+		unsigned int refs;
+		// Wheter KeyValues memory is borrowed or owned by CScriptKeyValues
+		// if not borrowed, it is deleted on release
+		bool borrow;
+
+		KeyValues_RC( KeyValues *pKeyValues, bool bBorrow ) :
+			ptr( pKeyValues ),
+			refs( 1 ),
+			borrow( bBorrow )
+		{
+		}
+
+		void AddRef()
+		{
+			Assert( refs < (unsigned int)-1 );
+			refs++;
+		}
+
+		void Release()
+		{
+			Assert( refs > 0 );
+			refs--;
+
+			if ( refs == 0 )
+			{
+				if ( !borrow )
+				{
+					ptr->deleteThis();
+				}
+
+				delete this;
+			}
+		}
+	};
+
+	KeyValues_RC *m_pSelf;
+	KeyValues_RC *m_pBase;
 };
 
 //-----------------------------------------------------------------------------

sp/src/vscript/vscript_squirrel.cpp

@@ -194,7 +194,7 @@ public:
 	// External instances. Note class will be auto-registered.
 	//--------------------------------------------------------
 
-	virtual HSCRIPT RegisterInstance(ScriptClassDesc_t* pDesc, void* pInstance, bool bAllowDestruct = false) override;
+	virtual HSCRIPT RegisterInstance(ScriptClassDesc_t* pDesc, void* pInstance, bool bRefCounted = false) override;
 	virtual void SetInstanceUniqeId(HSCRIPT hInstance, const char* pszId) override;
 	virtual void RemoveInstance(HSCRIPT hInstance) override;
 
@@ -1091,19 +1091,19 @@ namespace SQVector
 
 struct ClassInstanceData
 {
-	ClassInstanceData(void* instance, ScriptClassDesc_t* desc, const char* instanceId = nullptr, bool allowDestruct = false) :
+	ClassInstanceData(void* instance, ScriptClassDesc_t* desc, const char* instanceId = nullptr, bool refCounted = false) :
 		instance(instance),
 		desc(desc),
 		instanceId(instanceId),
-		allowDestruct(allowDestruct)
+		refCounted(refCounted)
 	{}
 
 	void* instance;
 	ScriptClassDesc_t* desc;
 	CUtlConstString instanceId;
 
-	// Indicates this game-created instance is a weak reference and can be destructed (Blixibon)
+	// keep for setting instance release hook in save/restore
-	bool allowDestruct;
+	bool refCounted;
 };
 
 bool CreateParamCheck(const ScriptFunctionBinding_t& func, char* output)
@@ -1442,6 +1442,17 @@ SQInteger function_stub(HSQUIRRELVM vm)
 	if (retval.m_type == FIELD_VECTOR)
 		delete retval.m_pVector;
 
+	Assert( ( pFunc->m_desc.m_ReturnType != FIELD_VOID ) || !( pFunc->m_flags & SF_REFCOUNTED_RET ) );
+
+	if ( ( pFunc->m_flags & SF_REFCOUNTED_RET ) && retval.m_hScript )
+	{
+		Assert( retval.m_type == FIELD_HSCRIPT );
+
+		// Release the intermediary ref held from RegisterInstance
+		sq_release(vm, (HSQOBJECT*)retval.m_hScript);
+		delete (HSQOBJECT*)retval.m_hScript;
+	}
+
 	return pFunc->m_desc.m_ReturnType != FIELD_VOID;
 }
 
@@ -1450,6 +1461,10 @@ SQInteger destructor_stub(SQUserPointer p, SQInteger size)
 {
 	auto classInstanceData = (ClassInstanceData*)p;
 
+	// if instance is not deleted, then it's leaking
+	// this should never happen
+	Assert( classInstanceData->desc->m_pfnDestruct );
+
 	if (classInstanceData->desc->m_pfnDestruct)
 		classInstanceData->desc->m_pfnDestruct(classInstanceData->instance);
 
@@ -1460,7 +1475,7 @@ SQInteger destructor_stub(SQUserPointer p, SQInteger size)
 SQInteger destructor_stub_instance(SQUserPointer p, SQInteger size)
 {
 	auto classInstanceData = (ClassInstanceData*)p;
-	// We don't call destructor here because this is owned by the game
+	// This instance is owned by the game, don't delete it
 	classInstanceData->~ClassInstanceData();
 	return 0;
 }
@@ -2676,7 +2691,7 @@ void SquirrelVM::RegisterHook(ScriptHook_t* pHookDesc)
 	RegisterHookDocumentation(vm_, pHookDesc, pHookDesc->m_desc, nullptr);
 }
 
-HSCRIPT SquirrelVM::RegisterInstance(ScriptClassDesc_t* pDesc, void* pInstance, bool bAllowDestruct)
+HSCRIPT SquirrelVM::RegisterInstance(ScriptClassDesc_t* pDesc, void* pInstance, bool bRefCounted)
 {
 	SquirrelSafeCheck safeCheck(vm_);
 
@@ -2698,15 +2713,19 @@ HSCRIPT SquirrelVM::RegisterInstance(ScriptClassDesc_t* pDesc, void* pInstance,
 	}
 
 	{
-		SQUserPointer p;
+		ClassInstanceData *self;
-		sq_getinstanceup(vm_, -1, &p, 0);
+		sq_getinstanceup(vm_, -1, (SQUserPointer*)&self, 0);
-		new(p) ClassInstanceData(pInstance, pDesc, nullptr, bAllowDestruct);
+		new(self) ClassInstanceData(pInstance, pDesc, nullptr, bRefCounted);
+
+		// can't delete the instance if it doesn't have a destructor
+		// if the instance doesn't have a constructor,
+		// the class needs to register the destructor with DEFINE_SCRIPT_REFCOUNTED_INSTANCE()
+		Assert( !bRefCounted || self->desc->m_pfnDestruct );
 	}
 
-	sq_setreleasehook(vm_, -1, bAllowDestruct ? &destructor_stub : &destructor_stub_instance);
+	sq_setreleasehook(vm_, -1, bRefCounted ? &destructor_stub : &destructor_stub_instance);
 
 	HSQOBJECT* obj = new HSQOBJECT;
-	sq_resetobject(obj);
 	sq_getstackobj(vm_, -1, obj);
 	sq_addref(vm_, obj);
 	sq_pop(vm_, 3);
@@ -2734,22 +2753,22 @@ void SquirrelVM::SetInstanceUniqeId(HSCRIPT hInstance, const char* pszId)
 
 void SquirrelVM::RemoveInstance(HSCRIPT hInstance)
 {
+	if (!hInstance)
+		return;
+
 	SquirrelSafeCheck safeCheck(vm_);
 
-	if (!hInstance) return;
 	HSQOBJECT* obj = (HSQOBJECT*)hInstance;
+	ClassInstanceData *self;
+
 	sq_pushobject(vm_, *obj);
-
+	sq_getinstanceup(vm_, -1, (SQUserPointer*)&self, nullptr);
-	SQUserPointer self;
-	sq_getinstanceup(vm_, -1, &self, nullptr);
-
-	((ClassInstanceData*)self)->~ClassInstanceData();
-
 	sq_setinstanceup(vm_, -1, nullptr);
 	sq_setreleasehook(vm_, -1, nullptr);
 	sq_pop(vm_, 1);
-
 	sq_release(vm_, obj);
+
+	self->~ClassInstanceData();
 	delete obj;
 }
 
@@ -3611,7 +3630,7 @@ void SquirrelVM::WriteObject( const SQObjectPtr &obj, CUtlBuffer* pBuffer, Write
 					{
 						Assert( strlen(pData->instanceId.Get()) < NATIVE_NAME_READBUF_SIZE );
 						pBuffer->PutString( pData->instanceId );
-						pBuffer->PutChar( pData->allowDestruct ? 1 : 0 );
+						pBuffer->PutChar( pData->refCounted ? 1 : 0 );
 					}
 					else
 					{
@@ -4222,7 +4241,7 @@ void SquirrelVM::ReadObject( SQObjectPtr &pObj, CUtlBuffer* pBuffer, ReadStateMa
 
 				if ( pszInstanceName[0] )
 				{
-					bool allowDestruct = ( pBuffer->GetChar() != 0 );
+					bool refCounted = ( pBuffer->GetChar() != 0 );
 
 					HSQOBJECT *hInstance = new HSQOBJECT;
 					hInstance->_type = OT_INSTANCE;
@@ -4234,8 +4253,8 @@ void SquirrelVM::ReadObject( SQObjectPtr &pObj, CUtlBuffer* pBuffer, ReadStateMa
 					if ( pInstance )
 					{
 						sq_addref( vm_, hInstance );
-						new( pThis->_userpointer ) ClassInstanceData( pInstance, pDesc, pszInstanceName, allowDestruct );
+						new( pThis->_userpointer ) ClassInstanceData( pInstance, pDesc, pszInstanceName, refCounted );
-						pThis->_hook = allowDestruct ? &destructor_stub : &destructor_stub_instance;
+						pThis->_hook = refCounted ? &destructor_stub : &destructor_stub_instance;
 					}
 					else
 					{