Skip to content

Update SetEntityHealth() to manage more entities#1770

Closed
einyux wants to merge 1 commit intoalliedmodders:masterfrom
einyux:master
Closed

Update SetEntityHealth() to manage more entities#1770
einyux wants to merge 1 commit intoalliedmodders:masterfrom
einyux:master

Conversation

@einyux
Copy link
Contributor

@einyux einyux commented May 23, 2022

In CS:GO, the health of a grenade projectile ("CBaseCSGrenadeProjectile") is present only in datamaps (NOT in sendprops).
This PR allows to set the health of this kind of entities.

@asherkin
Copy link
Member

HasEntProp is fairly slow for entities that don't have the prop, I think it would make sense in your case to use SetEntProp directly.

I don't really see a point to these utility stocks with all the general conveniences SM has around props these days.

@einyux
Copy link
Contributor Author

einyux commented May 23, 2022

I see, I will go with SetEntProp for my case.


At least the stock allows more with this, even if it's not the most efficient way.
-> Maybe these entity stocks should be deprecated in the future ?

@Kenzzer
Copy link
Member

Kenzzer commented Oct 4, 2022

Been wondering, since m_iHealth is a CBaseEntity property. Wouldn't it be okay to cache its offset (obtaining it from idk, world entity datamaps), and then blindly have the SetEntityHealth native set the health value to entityPtr + cachedHealthOffset, since it's a CBaseEntity property, we would always write into valid memory.

@KyleSanderson
Copy link
Member

Been wondering, since m_iHealth is a CBaseEntity property. Wouldn't it be okay to cache its offset (obtaining it from idk, world entity datamaps), and then blindly have the SetEntityHealth native set the health value to entityPtr + cachedHealthOffset, since it's a CBaseEntity property, we would always write into valid memory.

Incredibly highly not recommended. We had issues with nested datamaps for a while, to the point where we wouldn't even allow setting them because the offset was wrong and writing to the wrong location (which can still do pretty sigificant damage, like overwriting a handle). Please use Get/SetEntProp, and should there be a case where you need Data, file a bug.

@Kenzzer
Copy link
Member

Kenzzer commented Oct 7, 2022

Incredibly highly not recommended. We had issues with nested datamaps for a while, to the point where we wouldn't even allow setting them because the offset was wrong and writing to the wrong location (which can still do pretty sigificant damage, like overwriting a handle). Please use Get/SetEntProp, and should there be a case where you need Data, file a bug.

Would the same problem arise with netprop offset ? If duplicates are an issue, could go from the precept that the valid m_iHeath absolute offset is the one with lowest value (guessing from the term nested datamaps). worldspawn class CWorld is quite constant and well defined across different source games.

Though, I realise you're right in pointing out that Set/GetEntprop is better suited for this, as m_iHealth is present (at least on CSGO) on every entity as a datamap, so using Prop_Send instead of Prop_Data for the few entities that have it networked makes little to no sense. This is probably one of the few cases, where one'd want to use ChangeEdictState native, to counter act the fact they used Prop_Data instead of Prop_Send in order to remain universal, while avoiding the use of HasEntProp.

Going back to my original message, I was suggesting a native with cached offset because a PR I submitted in the past that landed does this. But regardless, taking a few steps back on the problem this PR tries to solve, I see that my answer makes no sense anyways.

Would the following revision to the SetEntityHealth stock be accepted ? (And if so I'll open another PR to close this one)

stock void SetEntityHealth(int entity, int amount)
{
	static bool gotconfig = false;
	static char prop[32];

	if (!gotconfig)
	{
		GameData gc = new GameData("core.games");
		bool exists = gc.GetKeyValue("m_iHealth", prop, sizeof(prop));
		delete gc;

		if (!exists)
		{
			strcopy(prop, sizeof(prop), "m_iHealth");
		}

		gotconfig = true;
	}

	PropFieldType type;
	int offset;

	if ((offset = FindDataMapInfo(entity, prop, type)) == -1)
	{
		ThrowError("SetEntityHealth not supported by this mod");
		return;
	}

	/* Dark Messiah uses a float for the health instead an integer */
	if (type == PropField_Float)
	{
		SetEntDataFloat(entity, offset, float(amount));
	}
	else
	{
		SetEntData(entity, offset, amount);
	}

	if (IsValidEdict(entity))
	{
		ChangeEdictState(entity);
	}
}

We essentially trade off FindSendPropInfo for FindDataMapInfo. And eliminate the call to GetEntityNetClass.
Finally, for valid edict we throw a ChangeEdictState at the end (to compensate the loss of Prop_Send). I'd be tempted to feed the offset, but following our earlier conversation, I'd rather not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants