delta.lst setting for entity skins doesn't have enough bits

[SamVanheer]

The delta.lst setting for entity skin variables doesn't have enough bits configured for it, which causes sprites that follow another entity and attach to an attachment point on that entity's model to sometimes appear in the wrong location.

For example this screenshot shows 4 Alien Controller charging balls, normally attached to the Alien Controllers' hands (1 ball per hand, 2 Controllers in this area) all showing up in the same location:

What happens here is as follows:
The controller creates the sprites and sets them to attach to itself, on attachment points 3 and 4 (this API is 1-based, not 0-based like CBaseAnimating::GetAttachment):

halflife/dlls/controller.cpp

Lines 851 to 854 in c7240b9

	m_pBall[i] = CSprite::SpriteCreate( "sprites/xspark4.spr", pev->origin, TRUE );
	m_pBall[i]->SetTransparency( kRenderGlow, 255, 255, 255, 255, kRenderFxNoDissipation );
	m_pBall[i]->SetAttachment( edict(), (i + 3) );
	m_pBall[i]->SetScale( 1.0 );

This sets the skin member of the sprite to the index of the controller entity:

halflife/dlls/effects.h

Lines 53 to 62 in c7240b9

	inline void SetAttachment( edict_t *pEntity, int attachment )
	{
	if ( pEntity )
	{
	pev->skin = ENTINDEX(pEntity);
	pev->body = attachment;
	pev->aiment = pEntity;
	pev->movetype = MOVETYPE_FOLLOW;
	}
	}

The skin member is copied to an entity_state_t for networking:

halflife/dlls/client.cpp

Line 1189 in c7240b9

state->skin = ent->v.skin;

This is then written to a buffer to be sent over the network to the client. It uses the configuration from delta.lst to determine how to do this:

halflife/network/delta.lst

Line 105 in c7240b9

DEFINE_DELTA( skin, DT_SHORT | DT_SIGNED, 9, 1.0 ),

This is where the problem occurs. The variable is configured to use 9 bits of precision. Normally this would be enough for indices up to 512, but since this is a signed value one bit is used for the negative bit, which leaves 8 bits. This limits the skin value to a maximum of 255. Any values that are larger than this will be clamped (in MSG_WriteSBits), which results in the sprite essentially attaching to a random entity, whichever entity has index 255.

If this entity no longer exists the client's outdated entity data is used, which results in the sprite showing up in a seemingly random location as seen above.

In the case shown in the screenshot all 4 sprites attach to the entity with index 255 since all of them attach to en entity with an index greater than 255.

To fix this the number of bits should be sufficient to store the largest possible entity index + 1 for the sign bit (since the variable is used for other things that can be negative, such as brush entity content types). The largest index in a default configured game is around 900 (not a fixed value because of #1777) which requires 10 bits of precision. aiment, which stores the index for MOVETYPE_FOLLOW entities uses 11 bits by default which is enough for at most 2048 entities, which is also the largest number of entities that can be networked by the engine (due to hardcoded limits, as noted in #1777). So 12 bits would be enough to ensure it always works, even if the game is configured to use the maximum number of entities supported by the engine.

However, there is something important to note about when skin is used in this manner: only sprites that set body to a non-zero value will use skin (see uses of R_GetAttachmentPoint in the engine). Otherwise the sprite's origin is used, which is set to the origin of the entity it's following if it uses MOVETYPE_FOLLOW (see CL_MoveAiments in the engine).

Any game that uses the SDK's sprite code to attach sprites to another entity will set both MOVETYPE_FOLLOW along with aiment, as well as skin and body.

What this means is that in all cases on the client aiment == skin will be true for sprites. The client already has the right entity index (verified by forcing skin to aiment for sprites, at least in the controller ball sprites case) so using skin here is pretty much useless, all it does is use more bandwidth to send the same data.

It might be a good idea to change the engine to use aiment as the entity index parameter to R_GetAttachmentPoint instead of skin. This fixes the bug in question without increasing bandwidth usage, and fixes the problem for all games and mods in one go rather than requiring each game and mod's delta.lst to be changed.

There is of course the risk that there exists some game or mod that only uses skin and not aiment, where this change would break those entity setups. I think that's unlikely given the lack of documentation on how sprites use skin, but it's always possible.

[tschumann]

Interesting - I've noticed when playing Gunman Chronicles multiplayer that sometimes sprites from one of the weapons appear at random locations - this could be why.