halflife icon indicating copy to clipboard operation
halflife copied to clipboard

[TFC] Death Bug (Zero Health Bug)

Open se7entfc opened this issue 6 years ago • 8 comments

This is 1 of 2 remaining bugs in TFC that really need attention.

Name: Death bug (Zero Health Bug)

Explanation: The death bug is a long-standing and highly amusing problem that presents itself at the most inopportune of times. Upon reaching a low value of health close to zero, the client seems to decide that the player has died and pushes the view down to the death style camera (parallel to the ground rather than perpendicular as normal). The server, however, knows better. As a result, the player can wander around with a tipped view and even “resurrect” themselves using a health pack or friendly medic.

Example 1: https://youtu.be/4M8Ha-H27Ks Example 2: https://youtu.be/XZ8WESissOA

Servers are currently using a 3rd party Metamod Plugin called Sparkys Utilities to fix this bug, an official fix would make the game almost perfect.

se7entfc avatar Apr 28 '19 13:04 se7entfc

Can you provide a link to this plugin and its source code?

SamVanheer avatar Apr 28 '19 13:04 SamVanheer

Here is the plugin page: http://web.archive.org/web/20050502130920/http://sparky.konspiracy.org/server/

The source code was lost by the coder (Ruiner)

se7entfc avatar Apr 28 '19 13:04 se7entfc

I reverse engineered it and it looks like the fix is this (in PlayerPreThink):

g_engfuncs[57]("sparky_options");
if ( (char)(signed int)a1 < 0 && !*(_DWORD *)(a2 + 496) && *(float *)(a2 + 480) < 1.0 )
  *(float *)(a2 + 480) = 0x3F800000;

Roughly translates to:

if( ( CVAR_GET_FLOAT( "sparky_options" ) & 128 ) && pev->deadflag == DEAD_NO && pev->health < 1 )
{
  pev->health = 1;
}

In other words it clamps values smaller than 1 to 1 if the player is still alive.

This is probably a bug where some code casts the value to int and then checks if the player is alive using that. I recall fixing a bug like that once in another mod, but i don't remember what i did.

The best solution is to round up when casting, so:

int intHealth = ( int ) ceil( pev->health );

Since casting to int performs a floor rounding operation implicitly this should ensure that health is correctly handled. The only issue is finding the code responsible, but i think it exists in SDK code so i can take a look around.

SamVanheer avatar Apr 28 '19 13:04 SamVanheer

That was a quick response, Thanks Sam.

here are the options for Sparkys:

// SECTION 1: General Sparky options "1" Allow flag timers "2" Allow flag locations "4" Allow speedometer "8" Allow team info hud/timeleft "16" Enable spectator on join fix "32" Enable grenade in ceiling fix "64" Enable grenade exploding on spawn fix "128" Enable death bug fix "256" Allow players to use "unstuck" command "512" Unmute text fix + health/armor in team_say "1024" Enable pipe lagging server fix

128 is correct. Can you take a look at the Grenade Bug fix (option 64) and reverse that also?

se7entfc avatar Apr 28 '19 14:04 se7entfc

I've found the cause of this issue: https://github.com/ValveSoftware/halflife/blob/5d761709a31ce1e71488f2668321de05f791b405/cl_dll/view.cpp#L417-L423

This is where the health value is checked to see if the player is dead. A value of 0.5 is treated as 0 here. This is not the cause of the bug, but merely the symptom.

The bug occurs in the engine's CL_ParseClientdata:

ei.stats[0] = (int)ffloor(pFrame->clientdata.health);

ei.stats (probably called cl.stats or cls.stats, IDA renamed this) stores the player's health as an int. As you can see it's rounded down. It then gets copied to the ref params instance and compared to 0 later on as linked above.

Fixing it is simple: round up before casting. All games will be fixed when this is done.

Note that this can also be done in UpdateClientData, where i tested it. Rounding in the engine reduces the effect of the value being rounded to only change behavior in code that uses ei.stats.

See also Quake's version of this: https://github.com/id-Software/Quake/blob/bf4ac424ce754894ac8f1dae6a3981954bc9852d/WinQuake/client.h#L157 https://github.com/id-Software/Quake/blob/bf4ac424ce754894ac8f1dae6a3981954bc9852d/WinQuake/client.h#L275

SamVanheer avatar Apr 28 '19 14:04 SamVanheer

I managed to re-create this bug with the info you provided:

AMXX Code:

#include <amxmodx>
#include <fakemeta>

new g_pcvar_heath;

public plugin_init()
{
	register_plugin("Death Bug", "1.0", "se7en");
	g_pcvar_heath = register_cvar("db_health", "1");
	register_clcmd("say death", "cmd_Death");
}

public cmd_Death(id)
{
	new iHealth = pev(id, pev_health);
	client_print(0, print_chat, "Health before = %i", iHealth);
	
	new iChangeHealth = get_pcvar_num(g_pcvar_heath);
	set_pev(id, pev_health,	iChangeHealth);
	
	iHealth = pev(id, pev_health);
	client_print(0, print_chat, "Health after = %i", iHealth);
}

Changing the db_health cvar has the same effect at every value when set_pev health is used.

se7entfc avatar Apr 28 '19 20:04 se7entfc

any chance this could be assigned to an active goldsrc dev now? There's a number of these bugs where the tfc community identified in code the specific issue(s).

ph1sh55 avatar Nov 23 '23 00:11 ph1sh55

I have added a fix to the TFC Bug Fixes plugin for AMXX while this is fixed by Valve:

https://forums.alliedmods.net/showthread.php?t=333746

se7entfc avatar Sep 18 '24 06:09 se7entfc