ValveSoftware / Source-1-Games

Source 1 based games such as TF2 and Counter-Strike: Source
635 stars 74 forks source link

[TF2] [x64_test] Wrong airblast behavior #5522

Open old702 opened 7 months ago

old702 commented 7 months ago

Airblasting enemies up works way too weird vs 32bit version. It feels a bit off and inconsistent with airblasting enemies straight up. The problem is there is no intuitive sign between working and not-working angle anymore you probably need to guess if it's working or not. Some pyro combo styles are impossible with these changes. The size of the airblast detection box seems unchanged it just moves differently with your aim now. Aiming down works as same as before means you still able to airblast from your back nevertheless. Don't think that's intentional seeing the lack of nerfs or buffs in x64 tf2. Here's a video with comparison

https://github.com/ValveSoftware/Source-1-Games/assets/9538804/8e11cef3-af8a-4c3d-bd8d-93b0fb0d6a00

Listen Server, x64_test beta Windows machine

RetroGamingIsAwesome commented 7 months ago

There really isn't an issue here seems like the x64 test build fixed a lot a problems people complained about pyro's air blast.

TheBoctor commented 7 months ago

Does the same change apply, when airblasting a grounded enemy and looking all the way (89deg) down? (I wouldn't call any of this a "fix" or a deliberate change unless there's a primary source saying so.)

old702 commented 7 months ago

I don't think this is a solution given that other problems people complain about persist. Playing further on x64, I still think this is the wrong behavior, considering that airblast is not lag compensated, but player movement on the client side is, there will be a real issue with realgame network conditions. If the player is standing directly in front of the Pyro (less than melee range), intuitively aiming 89 degrees up should work in any circumstances, but right now it's less than 45 degrees, sometimes it does not register.

If we are considering that fact that we are at the point that x64 brings no balance changes at all (as stated at the present moment) it is not a deliberate change it is a bug.

RetroGamingIsAwesome commented 6 months ago

My guess for what is going on here that this is a x64 build so all floating point values get converted to double precision which causes the bounding box to become small since it has a more accurate value, which in this case it's not a bug the huntsman was effected as well you can't just aim for the side and hope it snaps to the closest players head, you have to aim.

TheBoctor commented 6 months ago

My guess for what is going on here that this is a x64 build so all floating point values get converted to double precision which causes the bounding box to become small

float in a C/C++ binary built for amd64 (x86_64) is still four bytes long, and double is still eight bytes long. These should never differ in representation between i686 and amd64, especially when both build targets are using the same SSE instructions. You can observe this yourself with sizeof().

(I am only a user, not an authority on Source 1, but purely for the sake of politeness to the developers, I'd gently suggest avoiding speculation or tangents.)

Regarding client prediction and lag compensation on the compression blast: Was it ever compensated? Last I had checked, Jungle Inferno doubled the size of the axis-aligned bounding box used for the initial check, in an attempt to make reflecting projectiles feel more responsive.

old702 commented 6 months ago

Regarding client prediction and lag compensation on the compression blast: Was it ever compensated? Last I had checked, Jungle Inferno doubled the size of the axis-aligned bounding box used for the initial check, in an attempt to make reflecting projectiles feel more responsive.

I believe it has never been lag compensated in the first place because that would be very weirdly lookin for both client-server-client sides (because airblast working with projectiles). What i meant was the box for players were that big for better aiming and better registration results.

old702 commented 6 months ago

Does the same change apply, when airblasting a grounded enemy and looking all the way (89deg) down? (I wouldn't call any of this a "fix" or a deliberate change unless there's a primary source saying so.)

For looking down there are no significant changes noticed so far. I can make an another video with some different angles and Pyro positioning

RetroGamingIsAwesome commented 6 months ago

Replying to https://github.com/ValveSoftware/Source-1-Games/issues/5522#issuecomment-1960077429

My Bad.

wgetJane commented 6 months ago

My guess for what is going on here that this is a x64 build so all floating point values get converted to double precision which causes the bounding box to become small since it has a more accurate value, which in this case it's not a bug the huntsman was effected as well you can't just aim for the side and hope it snaps to the closest players head, you have to aim.

Regarding client prediction and lag compensation on the compression blast: Was it ever compensated?

enemy players get lag compensated when you airblast

you can confirm this using sv_showlagcompensation 1 on a listen server

slender100 commented 5 months ago

These new airblast changes, intentional or not, fix issue #4228

Overall, I would say these changes are an improvement over the 32 bit airblast, as this revision requires more aiming instead of being in the general direction of a swath of area to airblast something away.

old702 commented 5 months ago

The new changes are an improvement, but it feels like the cone angle needs to be increased a bit, this will fix issues like #4228 while still being efficient and mechanically feasible at point-blank range.

wgetJane commented 5 months ago

this change is seemingly intentional, just undocumented (i have changed my mind, i think this is likely unintentional, however the original intention years ago back in JI seems to be for the cone check to be working, yes it's confusing)

https://github.com/ValveSoftware/Source-1-Games/assets/52103358/e9c60b89-3926-4fa6-a144-4604285ff166

(video stolen from @ReplayCoding)

previously, these airblasts would've reached the medic:

image

UndefinedBHVR commented 5 months ago

Can confirm this bug is still here. Most likely a regression or some conditional compile flag that wasn't set when it should have been

old702 commented 5 months ago

After playing with different player camera angles it looks like the cone check (for airblasting other players only) is back. The cone angle is about of 90 degrees wide. (if we are considering center of players collision box)

@wgetJane @slender100 I would like to make a proposal to increase the cone angle from 90° up to ~115-120°, this will make it more similar to the airblast that everyone has become accustomed to since 2017, will make it more feasible at close ranges and at the same time will solve the problems with reflecting players behind the pyro and issues like those mentioned by slender100 and others https://github.com/ValveSoftware/Source-1-Games/issues/4228

cone90-1 cone115-1 cone90-2 cone115-2

danielpinoy commented 4 months ago

Replying to https://github.com/ValveSoftware/Source-1-Games/issues/5522#issuecomment-2067793417

I support this as well.

wgetJane commented 4 months ago

Replying to https://github.com/ValveSoftware/Source-1-Games/issues/5522#issuecomment-2067793417

i think you should make a separate issue for this as a feature request

wgetJane commented 4 months ago

here's a sort of recap on the context around this issue, along with a bunch of new information thanks to @ReplayCoding and @UndefinedBHVR:

pre-Jungle Inferno airblast hitbox characteristics:

box: https://youtube.com/watch?v=W1g2x4b_Byg cone: https://youtube.com/watch?v=kXM32e4cNgA

note: centre point = the infinitely small middle point of a player's "collision hull" touch = collision hull intersects with the shape

post-Jungle Inferno (32-bit) airblast hitbox characteristics:

video comparison between pre-JI vs post-JI (32-bit) airblast hitbox: https://youtube.com/watch?v=sZ0axNXvNik afaik the earliest mention of the JI airblast hitbox is from this video: https://youtube.com/watch?v=bNnYJwW_3DI

post-Jungle Inferno (64-bit) airblast hitbox characteristics:

here's a little example drawing from a top-down view, depicting which player positions will be airblasted on different versions of the game: image

yes, this means that this is what the current airblast hitbox looks like when the pyro is aiming 45 degrees horizontally, which looks quite funny: image


the interesting thing is that the 64-bit update apparently makes zero changes to anything related to the airblast, in fact, the cone check exists in the current 32-bit version of the game and it just doesn't work

video from @plazma127 of the cone check not working on the CURRENT Windows 32-bit version (can be opened manually through tf.exe):

https://github.com/ValveSoftware/Source-1-Games/assets/52103358/f6f96e9e-dbf7-490c-b429-013b915f6df3

but firstly...

the pre-JI cone check is performed using a very simple dot product check:

from CTFFlameThrower::DeflectPlayer

// Require our target be in a cone in front of us. Default threshold is the dot-product needs to be at least 0.8 = 1 - 0.2. 
float flDot = DotProduct( vecForward, vecToTarget );
float flAirblastConeScale = 0.2f;
CALL_ATTRIB_HOOK_FLOAT( flAirblastConeScale, mult_airblast_cone_scale );
float flAirblastConeThreshold = Clamp(1.0f - flAirblastConeScale, 0.0f, 1.0f);
if (flDot < flAirblastConeThreshold)
{
    return false;
}

at 1:17 of this video, you can see that the player does not get airblasted even if their collision hull is touching the cone, and they only get airblasted when their centre point is inside the cone: image

the post-JI cone check uses an aabb-cone intersection test from vphysics, using the CPhysicsTrace::IsBoxIntersectingCone function in particular (thanks to @ReplayCoding for figuring this out through decompilation) image

it seems that the goal of replacing the dot product check with an aabb-cone intersection check was to make it more forgiving (because the cone gets much narrower the closer you are) this is what the Jungle Inferno patch notes looks to be implying: image

here's some clips by @UndefinedBHVR in the current 64-bit version with visualisation (using vscript) to show that just touching the cone with the edge of your collision hull will pass the cone check:

https://github.com/ValveSoftware/Source-1-Games/assets/52103358/08a25322-d908-4f54-b8df-f81f9cf19735

https://github.com/ValveSoftware/Source-1-Games/assets/52103358/7ea90f47-4efc-4e2a-8248-203c3162e3ec

so why does this only work on 64-bit?

decompiling the CTFFlameThrower::DeflectPlayer function from the 32-bit version of the game reveals that there's supposed to be a cone check all along, it just doesn't work properly so the check always passes

according to @ReplayCoding's tests, the CPhysicsTrace::IsBoxIntersectingCone function in the 32-bit version will produce weird non-boolean values, which results in the cone check always passing even when it shouldn't:

bool CPhysicsTrace::IsBoxIntersectingCone( const Vector &boxAbsMins, const Vector &boxAbsMaxs, const truncatedcone_t &cone )

https://github.com/ValveSoftware/Source-1-Games/assets/52103358/e54ac2c7-3527-41bf-9656-d42f34814998

this doesn't happen in the 64-bit version, so the cone check works correctly

the reason why this happens is still unconfirmed as of now (my personal guess is compiler shenanigans)

anyway, here's what i think: 1) the cone check was unintentionally broken by the Jungle Inferno update 2) the cone check was unintentionally fixed by the 64-bit update (7 years later)

Wolfcl0ck commented 4 months ago

So basically? We call this "The Lore Document."

ethanholt1 commented 4 months ago

After sort of thoroughly skimming through what you all have said, I believe this to be an extension of Jungle Inferno's patch notes, as explained by @wgetJane . Whether or not this is a positive change or if the change should be removed is still up in the air, but I'm just putting in my two cents to say this seems intended.

I'm not nearly as in tune with the Source engine as some of you are, so I'll leave the more techie discussions to you all.

wgetJane commented 4 months ago

if you see people talking about spheres in regards to the airblast hitbox, it's because i mentioned in various places a few times over the years (starting back in probably 2020) that the airblast hitbox is probably intended to be a sphere rather than a cube

this is because when i decompiled the game back in 2020, i saw that the CTFWeaponBase::DeflectProjectiles function uses the UTIL_EntitiesInSphere function but it just acts like a cube test instead of a sphere test for some reason

@ReplayCoding confirms that this is still the case in the latest version of the game: image

additionally, the CTFWeaponBase::GetDeflectionSize() function is replaced with CTFWeaponBase::GetDeflectionRadius() image

the GetDeflectionSize function returns Vector( 128, 128, 64 ) which corresponds to the pre-JI 256x256x128 airblast box

the GetDeflectionRadius function returns 128, which implies a 256-radius sphere, which can be enclosed by the post-JI 256x256x256 airblast cube

to me, this really sounds a lot like the cube was actually intended to be a sphere, which would make the airblast hitbox consistent at any angle and prevent projectiles from being airblasted behind the pyro

top: airblasting at 0 degree yaw, bottom: airblasting at 45 degree yaw left: current airblast hitbox with cube, right: hypothetical airblast hitbox with sphere image

this actually results in a consistently-shaped airblast cone regardless of the pyro's aim direction: image

so we know that, despite the name, the UTIL_EntitiesInSphere function checks for entities within a CUBE instead of a sphere

wait, what else is supposed to be a sphere but is actually a cube?

out of curiosity, i looked through tf2's usages of UTIL_EntitiesInSphere, and one of the countless things that use the function is jarate/mad milk

well that can't be right, because i remember sigsegv's video showing a visualisation of the sphere used by jarate and mad milk

however, watch the video again: https://youtube.com/watch?v=1hYZbyUK170

image

image

it's obviously a cube, right? that's the only way it this makes sense

so a LOT of things in tf2 that are presumably supposed to be using spheres have actually been using cubes all along

image

not sure what to really make of this whole thing, it's difficult to confirm what the original designer intentions are without just directly asking them

Wolfcl0ck commented 4 months ago

If I may ask, given that it seems that the current airblast behavior on 64 bit appears to be the intended airblast behavior and that post-JI 32 bit airblast was apparently broken, what is the exact current purpose of this thread now? Is the hoped-for solution here to make the 64 bit version work the same way as the 32 bit version, fix the 32 bit version so that it functions the way that it was intended to, or something else entirely? I understand that competitive players may not like that the mechanic has changed on the 64 bit branch, but if it's functioning as intended now, would the proper response here be to leave the system up to server operators with a convar that changes whether or not "IsBoxIntersectingCone" always reports back as true?

wgetJane commented 4 months ago

@Wolfcl0ck i think it is a very confusing issue, some opinionated/controversial decisions will have to be made

the original designer intent in Jungle Inferno seems to be for the airblast to have a working cone check + sphere check

however, we've had the coneless cube airblast for 7 years now, which is almost half the time that the airblast has existed as a mechanic in tf2

the cone check was fixed just entirely accidentally because of undefined behaviour in ivp (according to ficool), and the airblast detection would only be half-fixed right now if we were to say that it's definitely supposed to be using a sphere instead of a cube

personally, i'm just providing what i think is relevant information, so i haven't said which outcome i would prefer because it would be biased towards how i like to play the game (since i'd prefer not to turn this issue into about discussing game balance requests)

Wolfcl0ck commented 4 months ago

@wgetJane I see. I would suspect, in all likelihood, that if there is enough demand for it, it should not be very difficult to commit the difference between the broken post-JI airblast and the fixed 64-bit airblast to a convar that can be chosen host-side. If my understanding of the above code is correct - and forgive me, my programming skills are rudimentary in this area so I may be misunderstanding things a bit - it seems like the main issue that caused it to break in the first place was just due to a boolean function always returning true, correct? With that being the case, it should theoretically be simple enough to find whatever causes the function to always return true and fix it for 32 bit, then just have a boolean convar that, if true, sets said function to also always return true. Should be best of both worlds, in theory.

wgetJane commented 4 months ago

it seems like the main issue that caused it to break in the first place was just due to a boolean function always returning true, correct?

no, not exactly

the function is CPhysicsTrace::IsBoxIntersectingCone (from ivp) with this signature:

bool CPhysicsTrace::IsBoxIntersectingCone( const Vector &boxAbsMins, const Vector &boxAbsMaxs, const truncatedcone_t &cone )

obviously, it's supposed to return only either 0 or 1

when the aabb-cone test succeeds in 32-bit tf2, it returns a value of 1, which is expected

when the aabb-cone test fails in 32-bit tf2, it doesn't return the expected value value of 0, instead it returns seemingly random arbitrary values like 49 or 65 or whatever (apparently caused by undefined behaviour)

this basically means that the airblast's cone check will always pass because the function never returns zero

(this is all according to @ReplayCoding so blame them if this is wrong :trollface:)

old702 commented 4 months ago

I would like to make a proposal to increase the cone angle from 90° up to ~115-120°

image

This topic and video https://www.youtube.com/watch?v=L1MF49sMHg4&t=1s from @wgetJane puts a lot of things into place. And while I agree that the new mechanic improves and complements the previous one, we've had the old mechanics for many years so many have been taught by the mechanics that actually skip checking for the presence of a player's collision box within the radius of the cone.

however, we've had the coneless cube airblast for 7 years now, which is almost half the time that the airblast has existed as a mechanic in tf2

Here is my final thoughts:

So basically? We call this "The Lore Document." For sure :)