Reconcile the concept of Edict & Networkable across the codebase (#1903)

* Reconcile the concept of Edict & Networkable across the codebase

* There's no need to check this, it's done elsewhere. Also could be null (segfault)

* This was never needed

* Pseudo review changes

Re-added removed null checks, and added new ones.

Changed the error messages in Get/SetProp natives to better reflect reality.

* Don't change the behaviour of GetEntityNetClass

* Overload IGameHelpers::FindServerClass

* Make error messages more accurate

* Fix a dev comment

* Rename FindServerClass

---------

Co-authored-by: Kenzzer <kenzzer@users.noreply.github.com>

core/HalfLife2.cpp

@@ -413,6 +413,17 @@ ServerClass *CHalfLife2::FindServerClass(const char *classname)
 	return pInfo->sc;
 }
 
+ServerClass *CHalfLife2::FindEntityServerClass(CBaseEntity *pEntity)
+{
+	IServerNetworkable* pNetwork = ((IServerUnknown *)pEntity)->GetNetworkable();
+	if (pNetwork == nullptr)
+	{
+		return nullptr;
+	}
+
+	return pNetwork->GetServerClass();
+}
+
 DataTableInfo *CHalfLife2::_FindServerClass(const char *classname)
 {
 	DataTableInfo *pInfo = NULL;

core/HalfLife2.h

@@ -221,6 +221,7 @@ public: //IGameHelpers
 	bool FindSendPropInfo(const char *classname, const char *offset, sm_sendprop_info_t *info);
 	datamap_t *GetDataMap(CBaseEntity *pEntity);
 	ServerClass *FindServerClass(const char *classname);
+	ServerClass *FindEntityServerClass(CBaseEntity *pEntity);
 	typedescription_t *FindInDataMap(datamap_t *pMap, const char *offset);
 	bool FindDataMapInfo(datamap_t *pMap, const char *offset, sm_datatable_info_t *pDataTable);
 	void SetEdictStateChanged(edict_t *pEdict, unsigned short offset);

core/smn_entities.cpp

@@ -134,12 +134,10 @@ inline edict_t *BaseEntityToEdict(CBaseEntity *pEntity)
 {
 	IServerUnknown *pUnk = (IServerUnknown *)pEntity;
 	IServerNetworkable *pNet = pUnk->GetNetworkable();
-
+	if (pNet == nullptr)
-	if (!pNet)
 	{
-		return NULL;
+		return nullptr;
 	}
-
 	return pNet->GetEdict();
 }
 
@@ -363,14 +361,20 @@ static cell_t IsValidEntity(IPluginContext *pContext, const cell_t *params)
 
 static cell_t IsEntNetworkable(IPluginContext *pContext, const cell_t *params)
 {
-	edict_t *pEdict = GetEdict(params[1]);
+	IServerUnknown *pUnknown = (IServerUnknown *)g_HL2.ReferenceToEntity(params[1]);
-
+	if (!pUnknown)
-	if (!pEdict)
 	{
 		return 0;
 	}
 
-	return (pEdict->GetNetworkable() != NULL) ? 1 : 0;
+	IServerNetworkable *pNet = pUnknown->GetNetworkable();
+	if (!pNet)
+	{
+		return 0;
+	}
+
+	edict_t* edict = pNet->GetEdict();
+	return (edict && !edict->IsFree()) ? 1 : 0;
 }
 
 static cell_t GetEntityCount(IPluginContext *pContext, const cell_t *params)
@@ -436,14 +440,12 @@ static cell_t GetEntityNetClass(IPluginContext *pContext, const cell_t *params)
 		return pContext->ThrowNativeError("Invalid entity (%d - %d)", g_HL2.ReferenceToIndex(params[1]), params[1]);
 	}
 
-	IServerNetworkable *pNet = pUnk->GetNetworkable();
+	ServerClass *pClass = g_HL2.FindEntityServerClass(pEntity);
-	if (!pNet)
+	if (!pClass)
 	{
 		return 0;
 	}
 
-	ServerClass *pClass = pNet->GetServerClass();
-
 	pContext->StringToLocal(params[2], params[3], pClass->GetName());
 
 	return 1;
@@ -1270,13 +1272,11 @@ static cell_t SetEntDataString(IPluginContext *pContext, const cell_t *params)
 #define FIND_PROP_SEND(type, type_name) \
 	sm_sendprop_info_t info;\
 	SendProp *pProp; \
-	IServerUnknown *pUnk = (IServerUnknown *)pEntity; \
+	ServerClass *pServerClass = g_HL2.FindEntityServerClass(pEntity); \
-	IServerNetworkable *pNet = pUnk->GetNetworkable(); \
+	if (pServerClass == nullptr) { \
-	if (!pNet) \
+		pContext->ThrowNativeError("Failed to retrieve entity %d (%d) server class!", g_HL2.ReferenceToIndex(params[1]), params[1]); \
-	{ \
-		return pContext->ThrowNativeError("Edict %d (%d) is not networkable", g_HL2.ReferenceToIndex(params[1]), params[1]); \
 	} \
-	if (!g_HL2.FindSendPropInfo(pNet->GetServerClass()->GetName(), prop, &info)) \
+	if (!g_HL2.FindSendPropInfo(pServerClass->GetName(), prop, &info)) \
 	{ \
 		const char *class_name = g_HL2.GetEntityClassname(pEntity); \
 		return pContext->ThrowNativeError("Property \"%s\" not found (entity %d/%s)", \
@@ -1434,13 +1434,13 @@ static cell_t GetEntPropArraySize(IPluginContext *pContext, const cell_t *params
 		{
 			sm_sendprop_info_t info;
 			
-			IServerUnknown *pUnk = (IServerUnknown *)pEntity;
+			ServerClass *pServerClass = g_HL2.FindEntityServerClass(pEntity);
-			IServerNetworkable *pNet = pUnk->GetNetworkable();
+			if (pServerClass == nullptr)
-			if (!pNet)
 			{
-				return pContext->ThrowNativeError("Edict %d (%d) is not networkable", g_HL2.ReferenceToIndex(params[1]), params[1]);
+				return pContext->ThrowNativeError("Failed to retrieve entity %d (%d) server class!", g_HL2.ReferenceToIndex(params[1]), params[1]);
 			}
-			if (!g_HL2.FindSendPropInfo(pNet->GetServerClass()->GetName(), prop, &info))
+
+			if (!g_HL2.FindSendPropInfo(pServerClass->GetName(), prop, &info))
 			{
 				const char *class_name = g_HL2.GetEntityClassname(pEntity);
 				return pContext->ThrowNativeError("Property \"%s\" not found (entity %d/%s)",
@@ -2079,13 +2079,7 @@ static cell_t SetEntPropEnt(IPluginContext *pContext, const cell_t *params)
 			edict_t *pOtherEdict = NULL;
 			if (pOther)
 			{
-				IServerNetworkable *pNetworkable = ((IServerUnknown *) pOther)->GetNetworkable();
+				pOtherEdict = BaseEntityToEdict(pOther);
-				if (!pNetworkable)
-				{
-					return pContext->ThrowNativeError("Entity %d (%d) does not have a valid edict", g_HL2.ReferenceToIndex(params[4]), params[4]);
-				}
-
-				pOtherEdict = pNetworkable->GetEdict();
 				if (!pOtherEdict || pOtherEdict->IsFree())
 				{
 					return pContext->ThrowNativeError("Entity %d (%d) does not have a valid edict", g_HL2.ReferenceToIndex(params[4]), params[4]);

extensions/cstrike/natives.cpp

@@ -254,10 +254,13 @@ static cell_t CS_DropWeapon(IPluginContext *pContext, const cell_t *params)
 
 	//Psychonic is awesome for this
 	sm_sendprop_info_t spi;
-	IServerUnknown *pUnk = (IServerUnknown *)pWeapon;
+	ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pWeapon);
-	IServerNetworkable *pNet = pUnk->GetNetworkable();
+	if (pServerClass == nullptr)
+	{
+		return pContext->ThrowNativeError("Failed to retrieve entity %d server class!", params[2]);
+	}
 
-	if (!UTIL_FindDataTable(pNet->GetServerClass()->m_pTable, "DT_WeaponCSBase", &spi, 0))
+	if (!UTIL_FindDataTable(pServerClass->m_pTable, "DT_WeaponCSBase", &spi, 0))
 		return pContext->ThrowNativeError("Entity index %d is not a weapon", params[2]);
 
 	if (!gamehelpers->FindSendPropInfo("CBaseCombatWeapon", "m_hOwnerEntity", &spi))

extensions/sdkhooks/extension.cpp

@@ -614,11 +614,16 @@ HookReturn SDKHooks::Hook(int entity, SDKHookType type, IPluginFunction *callbac
 
 	if (!!strcmp(g_HookTypes[type].dtReq, ""))
 	{
-		IServerUnknown *pUnk = (IServerUnknown *)pEnt;
+		ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pEnt);
-
+		if (pServerClass == nullptr)
-		IServerNetworkable *pNet = pUnk->GetNetworkable();
+		{
-		if (pNet && !UTIL_ContainsDataTable(pNet->GetServerClass()->m_pTable, g_HookTypes[type].dtReq))
 			return HookRet_BadEntForHookType;
+		}
+
+		if (!UTIL_ContainsDataTable(pServerClass->m_pTable, g_HookTypes[type].dtReq))
+		{
+			return HookRet_BadEntForHookType;
+		}
 	}
 
 	size_t entry;

extensions/sdkhooks/natives.cpp

@@ -60,15 +60,15 @@ cell_t Native_Hook(IPluginContext *pContext, const cell_t *params)
 			break;
 		case HookRet_BadEntForHookType:
 		{
-			CBaseEntity *pEnt = gamehelpers->ReferenceToEntity(params[1]);
+			CBaseEntity *pEnt = gamehelpers->ReferenceToEntity(params[1]);
-			const char *pClassname = pEnt ? gamehelpers->GetEntityClassname(pEnt) : NULL;
+			const char *pClassname = pEnt ? gamehelpers->GetEntityClassname(pEnt) : NULL;
-			if (!pClassname)
+			if (!pClassname)
-			{
+			{
-				pContext->ThrowNativeError("Hook type not valid for this type of entity (%i).", entity);
+				pContext->ThrowNativeError("Hook type not valid for this type of entity (%i).", entity);
-			}
+			}
-			else
+			else
-			{
+			{
-				pContext->ThrowNativeError("Hook type not valid for this type of entity (%i/%s)", entity, pClassname);
+				pContext->ThrowNativeError("Hook type not valid for this type of entity (%i/%s)", entity, pClassname);
 			}
 			
 			break;
@@ -185,9 +185,9 @@ cell_t Native_TakeDamage(IPluginContext *pContext, const cell_t *params)
 		if (!pCall)
 		{
 			int offset;
-			if (!g_pGameConf->GetOffset("OnTakeDamage", &offset))
+			if (!g_pGameConf->GetOffset("OnTakeDamage", &offset))
-			{
+			{
-				return pContext->ThrowNativeError("Could not find OnTakeDamage offset");
+				return pContext->ThrowNativeError("Could not find OnTakeDamage offset");
 			}			
 
 			PassInfo pass[2];
@@ -230,10 +230,13 @@ cell_t Native_DropWeapon(IPluginContext *pContext, const cell_t *params)
 	if (!pWeapon)
 		return pContext->ThrowNativeError("Invalid entity index %d for weapon", params[2]);
 
-	IServerUnknown *pUnk = (IServerUnknown *)pWeapon;
+	ServerClass *pClass = gamehelpers->FindEntityServerClass(pWeapon);
-	IServerNetworkable *pNet = pUnk->GetNetworkable();
+	if (pClass == nullptr)
-
+	{
-	if (!UTIL_ContainsDataTable(pNet->GetServerClass()->m_pTable, "DT_BaseCombatWeapon"))
+		return pContext->ThrowNativeError("Failed to retrieve entity %d server class!", params[2]);
+	}
+
+	if (!UTIL_ContainsDataTable(pClass->m_pTable, "DT_BaseCombatWeapon"))
 		return pContext->ThrowNativeError("Entity index %d is not a weapon", params[2]);
 
 	sm_sendprop_info_t spi;
@@ -291,9 +294,9 @@ cell_t Native_DropWeapon(IPluginContext *pContext, const cell_t *params)
 		if (!pCall)
 		{
 			int offset;
-			if (!g_pGameConf->GetOffset("Weapon_Drop", &offset))
+			if (!g_pGameConf->GetOffset("Weapon_Drop", &offset))
-			{
+			{
-				return pContext->ThrowNativeError("Could not find Weapon_Drop offset");
+				return pContext->ThrowNativeError("Could not find Weapon_Drop offset");
 			}
 
 			PassInfo pass[3];

extensions/sdktools/gamerulesnatives.cpp

@@ -45,26 +45,27 @@ static CBaseEntity *FindEntityByNetClass(int start, const char *classname)
 	int maxEntities = gpGlobals->maxEntities;
 	for (int i = start; i < maxEntities; i++)
 	{
-		edict_t *current = gamehelpers->EdictOfIndex(i);
+		CBaseEntity *pEntity = gamehelpers->ReferenceToEntity(i);
-		if (current == NULL || current->IsFree())
+		if (pEntity == nullptr)
+		{
 			continue;
+		}
 
-		IServerNetworkable *network = current->GetNetworkable();
+		ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pEntity);
-		if (network == NULL)
+		if (pServerClass == nullptr)
+		{
 			continue;
+		}
+		
 
-		IHandleEntity *pHandleEnt = network->GetEntityHandle();
+		const char *name = pServerClass->GetName();
-		if (pHandleEnt == NULL)
-			continue;
-
-		ServerClass *sClass = network->GetServerClass();
-		const char *name = sClass->GetName();
-
 		if (!strcmp(name, classname))
-			return gamehelpers->ReferenceToEntity(gamehelpers->IndexOfEdict(current));		
+		{
+			return pEntity;
+		}
 	}
 
-	return NULL;
+	return nullptr;
 }
 
 static CBaseEntity* GetGameRulesProxyEnt()

extensions/sdktools/teamnatives.cpp

@@ -50,19 +50,20 @@ void InitTeamNatives()
 
 	int edictCount = gpGlobals->maxEntities;
 
-	for (int i=0; i<edictCount; i++)
+	for (int i = 0; i < edictCount; i++)
 	{
-		edict_t *pEdict = PEntityOfEntIndex(i);
+		CBaseEntity *pEntity = gamehelpers->ReferenceToEntity(i);
-		if (!pEdict || pEdict->IsFree())
+		if (pEntity == nullptr)
-		{
-			continue;
-		}
-		if (!pEdict->GetNetworkable())
 		{
 			continue;
 		}
 
-		ServerClass *pClass = pEdict->GetNetworkable()->GetServerClass();
+		ServerClass *pClass = gamehelpers->FindEntityServerClass(pEntity);
+		if (pClass == nullptr)
+		{
+			continue;
+		}
+		
 		if (FindNestedDataTable(pClass->m_pTable, "DT_Team"))
 		{
 			SendProp *pTeamNumProp = g_pGameHelpers->FindInSendTable(pClass->GetName(), "m_iTeamNum");
@@ -70,15 +71,14 @@ void InitTeamNatives()
 			if (pTeamNumProp != NULL)
 			{
 				int offset = pTeamNumProp->GetOffset();
-				CBaseEntity *pEnt = pEdict->GetUnknown()->GetBaseEntity();
+				int TeamIndex = *(int *)((unsigned char *)pEntity + offset);
-				int TeamIndex = *(int *)((unsigned char *)pEnt + offset);
 
 				if (TeamIndex >= (int)g_Teams.size())
 				{
 					g_Teams.resize(TeamIndex+1);
 				}
 				g_Teams[TeamIndex].ClassName = pClass->GetName();
-				g_Teams[TeamIndex].pEnt = pEnt;
+				g_Teams[TeamIndex].pEnt = pEntity;
 			}
 		}
 	}

extensions/sdktools/vglobals.cpp

@@ -321,28 +321,23 @@ void GetResourceEntity()
 	{
 		int edictCount = gpGlobals->maxEntities;
 
-		for (int i=0; i<edictCount; i++)
+		for (int i = 0; i < edictCount; i++)
 		{
-			edict_t *pEdict = PEntityOfEntIndex(i);
+			CBaseEntity *pEntity = gamehelpers->ReferenceToEntity(i);
-			if (!pEdict || pEdict->IsFree())
+			if (pEntity == nullptr)
-			{
-				continue;
-			}
-			if (!pEdict->GetNetworkable())
 			{
 				continue;
 			}
 
-			IHandleEntity *pHandleEnt = pEdict->GetNetworkable()->GetEntityHandle();
+			ServerClass *pClass = gamehelpers->FindEntityServerClass(pEntity);
-			if (!pHandleEnt)
+			if (pClass == nullptr)
 			{
 				continue;
 			}
-
+			
-			ServerClass *pClass = pEdict->GetNetworkable()->GetServerClass();
 			if (FindNestedDataTable(pClass->m_pTable, "DT_PlayerResource"))
 			{
-				g_ResourceEntity = pHandleEnt->GetRefEHandle();
+				g_ResourceEntity = ((IHandleEntity *)pEntity)->GetRefEHandle();
 				break;
 			}
 		}

extensions/sdktools/vnatives.cpp

@@ -1689,7 +1689,12 @@ static cell_t LookupEntityAttachment(IPluginContext* pContext, const cell_t* par
 	CBaseEntity* pEntity;
 	ENTINDEX_TO_CBASEENTITY(params[1], pEntity);
 
-	ServerClass* pClass = ((IServerUnknown*)pEntity)->GetNetworkable()->GetServerClass();
+	ServerClass* pClass = gamehelpers->FindEntityServerClass(pEntity);
+	if (pClass == nullptr)
+	{
+		return pContext->ThrowNativeError("Failed to retrieve entity %d (%d) server class!", gamehelpers->ReferenceToIndex(params[1]), params[1]);
+	}
+
 	if (!FindNestedDataTable(pClass->m_pTable, "DT_BaseAnimating"))
 	{
 		return pContext->ThrowNativeError("Entity %d (%d) is not a CBaseAnimating", gamehelpers->ReferenceToIndex(params[1]), params[1]);
@@ -1735,7 +1740,12 @@ static cell_t GetEntityAttachment(IPluginContext* pContext, const cell_t* params
 	CBaseEntity* pEntity;
 	ENTINDEX_TO_CBASEENTITY(params[1], pEntity);
 
-	ServerClass* pClass = ((IServerUnknown*)pEntity)->GetNetworkable()->GetServerClass();
+	ServerClass* pClass = gamehelpers->FindEntityServerClass(pEntity);
+	if (pClass == nullptr)
+	{
+		return pContext->ThrowNativeError("Failed to retrieve entity %d (%d) server class!", gamehelpers->ReferenceToIndex(params[1]), params[1]);
+	}
+	
 	if (!FindNestedDataTable(pClass->m_pTable, "DT_BaseAnimating"))
 	{
 		return pContext->ThrowNativeError("Entity %d (%d) is not a CBaseAnimating", gamehelpers->ReferenceToIndex(params[1]), params[1]);

extensions/tf2/criticals.cpp

@@ -76,16 +76,21 @@ bool CritManager::TryEnable()
 	for (size_t i = playerhelpers->GetMaxClients() + 1; i < MAX_EDICTS; ++i)
 	{
 		CBaseEntity *pEntity = gamehelpers->ReferenceToEntity(i);
-		if (pEntity == NULL)
+		if (pEntity == nullptr)
+		{
 			continue;
+		}
 
-		IServerUnknown *pUnknown = (IServerUnknown *)pEntity;
+		ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pEntity);
-		IServerNetworkable *pNetworkable = pUnknown->GetNetworkable();
+		if (pServerClass == nullptr)
-		if (!pNetworkable)
+		{
 			continue;
+		}
 
-		if (!UTIL_ContainsDataTable(pNetworkable->GetServerClass()->m_pTable, TF_WEAPON_DATATABLE))
+		if (!UTIL_ContainsDataTable(pServerClass->m_pTable, TF_WEAPON_DATATABLE))
+		{
 			continue;
+		}
 
 		SH_ADD_MANUALHOOK(CalcIsAttackCriticalHelper, pEntity, SH_MEMBER(&g_CritManager, &CritManager::Hook_CalcIsAttackCriticalHelper), false);
 		SH_ADD_MANUALHOOK(CalcIsAttackCriticalHelperNoCrits, pEntity, SH_MEMBER(&g_CritManager, &CritManager::Hook_CalcIsAttackCriticalHelperNoCrits), false);
@@ -116,15 +121,20 @@ void CritManager::Disable()
 void CritManager::OnEntityCreated(CBaseEntity *pEntity, const char *classname)
 {
 	if (!m_enabled)
+	{
 		return;
+	}
 
-	IServerUnknown *pUnknown = (IServerUnknown *)pEntity;
+	ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pEntity);
-	IServerNetworkable *pNetworkable = pUnknown->GetNetworkable();
+	if (pServerClass == nullptr)
-	if (!pNetworkable)
+	{
 		return;
+	}
 
-	if (!UTIL_ContainsDataTable(pNetworkable->GetServerClass()->m_pTable, TF_WEAPON_DATATABLE))
+	if (!UTIL_ContainsDataTable(pServerClass->m_pTable, TF_WEAPON_DATATABLE))
+	{
 		return;
+	}
 
 	SH_ADD_MANUALHOOK(CalcIsAttackCriticalHelper, pEntity, SH_MEMBER(&g_CritManager, &CritManager::Hook_CalcIsAttackCriticalHelper), false);
 	SH_ADD_MANUALHOOK(CalcIsAttackCriticalHelperNoCrits, pEntity, SH_MEMBER(&g_CritManager, &CritManager::Hook_CalcIsAttackCriticalHelperNoCrits), false);
@@ -164,11 +174,9 @@ bool CritManager::Hook_CalcIsAttackCriticalHelpers(bool noCrits)
 {
 	CBaseEntity *pWeapon = META_IFACEPTR(CBaseEntity);
 	
-	// If there's an invalid ent or invalid networkable here, we've got issues elsewhere.
+	// If there's an invalid ent or invalid server class here, we've got issues elsewhere.
-
+	ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pWeapon);
-	IServerNetworkable *pNetWeapon = ((IServerUnknown *)pWeapon)->GetNetworkable();
+	if (pServerClass == nullptr)
-	ServerClass *pServerClass = pNetWeapon->GetServerClass();
-	if (!pServerClass)
 	{
 		g_pSM->LogError(myself, "Invalid server class on weapon.");
 		RETURN_META_VALUE(MRES_IGNORED, false);

extensions/tf2/extension.cpp

@@ -435,23 +435,19 @@ int FindEntityByNetClass(int start, const char *classname)
 
 	for (int i = ((start != -1) ? start : 0); i < gpGlobals->maxEntities; i++)
 	{
-		current = engine->PEntityOfEntIndex(i);
+		CBaseEntity *pEntity = gamehelpers->ReferenceToEntity(i);
-		if (current == NULL || current->IsFree())
+		if (pEntity == nullptr)
 		{
 			continue;
 		}
 
-		IServerNetworkable *network = current->GetNetworkable();
+		ServerClass *pServerClass = gamehelpers->FindEntityServerClass(pEntity);
-
+		if (pServerClass == nullptr)
-		if (network == NULL)
 		{
 			continue;
 		}
-
-		ServerClass *sClass = network->GetServerClass();
-		const char *name = sClass->GetName();
 		
-
+		const char *name = pServerClass->GetName();
 		if (strcmp(name, classname) == 0)
 		{
 			return i;

public/IGameHelpers.h

@@ -40,7 +40,7 @@
  */
 
 #define SMINTERFACE_GAMEHELPERS_NAME		"IGameHelpers"
-#define SMINTERFACE_GAMEHELPERS_VERSION		11
+#define SMINTERFACE_GAMEHELPERS_VERSION		12
 
 class CBaseEntity;
 class CBaseHandle;
@@ -351,6 +351,13 @@ namespace SourceMod
 		 * @return				64-bit server Steam id.
 		 */
 		virtual uint64_t GetServerSteamId64() const =0;
+
+		/**
+		 * @brief Finds a given entity's server class.
+		 *
+		 * @return				ServerClass pointer on success, nullptr on failure.
+		 */
+		virtual ServerClass *FindEntityServerClass(CBaseEntity *pEntity) = 0;
 	};
 }