GetHullBounds handles vector copying incorrectly

[SamVanheer]

The GetHullBounds function in client.cpp handles vector copying incorrectly.
Link to code:

halflife/dlls/client.cpp

Line 1842 in 5d76170

int GetHullBounds( int hullnumber, float *mins, float *maxs )

It assigns the bounds vectors to a float*. All this does is set the function argument to point to that vector (which is a local variable). It doesn't modify the actual vector stored in the engine.

The same thing happens on the client side:

halflife/cl_dll/cdll_int.cpp

Line 72 in 5d76170

int CL_DLLEXPORT HUD_GetHullBounds( int hullnumber, float *mins, float *maxs )

Both functions do the same thing, and neither actually set the bounds in the engine.

You can verify this by printing out the contents of the array that the arguments point to upon entry of the function. They are unmodified.

The engine uses the same values as those set in the SDK, so you won't notice any problems if you leave them unchanged.

Note that if you fix this, you must fix it for both functions. The same memory is used by the client and server, so if only one of them is fixed, you will end up with code going out of sync if the bounds are changed.

[JoelTroch]

So changing the function prototype to this :

int GetHullBounds( int hullnumber, float &mins, float &maxs )

On both sides should fix the issue or am I wrong ?

[SamVanheer]

No, if you want it to work properly and be simple, you should use Vector&:
int GetHullBounds( int hullnumber, Vector& mins, Vector& maxs )

You'll have to change the function prototypes, but it won't break anything.

Alternatively, if you don't want to change it, memcpy the vectors:
memcpy( mins, &VEC_HULL_MIN, sizeof( float ) * 3 );

I'm assuming it will allow you to take the address of VEC_HULL_MIN (it probably won't). I used the Vector& solution since it's the simplest, and since it makes the code easier to work with.

[JoelTroch]

Alright, thanks for sharing

[tschumann]

Nice - you should add a pull request to fix this.

[SamVanheer]

I'll see if i can put together a pull request for this then.

[SamVanheer]

Ignore the first commit, it was made in the wrong branch.

EDIT: removed the wrong commits.