ValveSoftware / halflife

Half-Life 1 engine based games
Other
3.65k stars 617 forks source link

[HL1/BS/OF] Crowbar corpse hitting bug #2590

Open agrastiOs opened 5 years ago

agrastiOs commented 5 years ago

Instead of the Crowbar just hitting a corpse normally, the Crowbar starts to hit the corpse incredibly fast and gib it almost instantly, the animation of hitting doesn't play and the hitting sound gets spammed.

Though mikela-valve mentioned that it seems to be "intentional logic in the crowbar code", it only started existing since 1.1.0.8, and crowbar changes weren't mentioned in 1.1.0.8 patch notes.

Perhaps the same behavior could be retained, but the sound spam and no animation playing fixed to reduce the impression of it looking like a bug.

SamVanheer commented 5 years ago

This happens because when the prediction code was added, the code that sets the next primary attack time was moved. Compare SDK 1.0 with the current SDK: https://github.com/twhl-community/HalfLifeSDKHistory/blob/b3db8f14e3c77b6f219bf3ea7edc1f0a58a0605c/dlls/crowbar.cpp#L264 https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/crowbar.cpp#L307

So why was it moved? Because when implementing prediction they had to disable server-only code with a #ifndef CLIENT_DLL check, and that required that line to be moved so both client and server set it.

What they didn't realize is that there is an unconditional return TRUE; inside the server-only code that skips the code: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/crowbar.cpp#L267

Fixing it is easy: move the code back, and split the #ifndef CLIENT_DLL into 2 pieces to wrap the code before and after the line.

Make sure to keep the code as it looks now:

m_flNextPrimaryAttack = GetNextAttackDelay( 0.25 );

Otherwise the time value will use the wrong time base.

Note that fixing this will increase the time it takes to gib corpses, restoring it to the original amount of time it took at release.

It is possible to add a cvar to choose the behavior here, if you feel the chainsaw crowbar is iconic enough. You'll need to network the cvar value for the crowbar so client prediction accounts for it though.

mikela-valve commented 5 years ago

Thanks @SamVanheer. It was obvious that the return was there to intentionally skip the rest of that function for dead entities but the oldest revision that I have that isn't in backups has weapon prediction in it so I wasn't sure if not resetting the attack timer was intentional. I'll fix this in a future release and determine then if it needs a cvar to re-enable this old behaviour.

SamVanheer commented 5 years ago

You're welcome. I collected the old official releases and put them here if you need to find the older SDK versions and stuff: https://github.com/twhl-community/OldValveReleases/releases/tag/releases

CS-PRO1 commented 5 years ago

@mikela-valve Please keep that as a cvar. When I reported that the "Machine-Gun Crowbar" bug may be fixed in the future the community didn't like that much and asked if it'd be kept in. Actually it's in some cases a bit useful. So a cvar is the best option..

JulianHeuser commented 5 years ago

It might also be worth adding a "beta" branch on steam that keeps old bugs that people tend to be fond of. Or just keep an old version on there. A cvar works fine but I feel like there are lots of small quirks to the game that people tend to like for one reason or another, and it could be annoying keeping track of all those cvars. It's just a suggestion though, and it depends on what's easier to implement.

agrastiOs commented 5 years ago

@Sockman1 An old branch sounds weird tbh. What quirks would deserve to be kept in and what quirks wouldn't in a branch? Why not just put a WON version at a branch then? Cvars sound better IMO. Plus another branch might be locked out from most servers. More cvars don't really make using the console more annoying. If a person knows what cvar he wants to use, then he'll use that cvar. Or use the "find" command.

CS-PRO1 commented 5 years ago

Btw can someone test if this happens with DMC and TFC's crowbars?

agrastiOs commented 5 years ago

@CS-PRO1 Tested both games, you can't destroy corpses at all in these.

CS-PRO1 commented 5 years ago

Then it's a problem in SP games and doesn't happen in MP ones.. Well even in HL1 MP dead bodies don't react to bullets/crowbar hits (is this considered a bug as well or just intended like that?)

SamVanheer commented 5 years ago

NPC corpses and player corpses are different types of entities. NPC corpses are treated as solid, player corpses are non-solid: https://github.com/ValveSoftware/halflife/blob/c7240b965743a53a29491dd49320c88eecf6257b/dlls/world.cpp#L200-L206