Jump 2 - #2039
Conversation
|
(Edit) only if its not built? |
|
|
||
| virtual CBaseEntity *OnFailedPhysGunPickup( Vector vPhysgunPos ); | ||
| virtual void DriveVehicle( float flFrameTime, CUserCmd *ucmd, int iButtonsDown, int iButtonsReleased ); | ||
| virtual void DriveVehicle( float flFrameTime, CUserCmd *ucmd, int iButtonsDown, int iButtonsReleased ); // NEO TODO (Adam) change to int64 when building this |
There was a problem hiding this comment.
Nit: maybe instead of commenting this, add a NEO-ifdef guarded Assert(false) in the impl, so whoever touches this in the future cannot miss the comment without hitting an assert?
| virtual void UpdateOnRemove( void ); | ||
|
|
||
| void DriveCannon( int iDriverButtons, int iButtonsPressed ); | ||
| void DriveCannon( int iDriverButtons, int iButtonsPressed ); // NEO TODO (Adam) change iDriverButtons and iButtonsPressed to int64 when building this |
There was a problem hiding this comment.
Nit: would maybe prefer a hard Assert(false) in the implementation so we don't rely on the comment being seen but rather fail loudly
| void CPropJeep::DriveVehicle( float flFrameTime, CUserCmd *ucmd, int iButtonsDown, int iButtonsReleased ) | ||
| { | ||
| int iButtons = ucmd->buttons; | ||
| int iButtons = ucmd->buttons; // NEO TODO (Adam) change to int64 when building this |
There was a problem hiding this comment.
Maybe ifdef NEO Assert(false) here so this cannot work without the coder noticing & fixing this in the future
|
|
||
| // NOTE: Attack2 will be considered to be pressed until the first item is picked up. | ||
| int nAttack2Mask = pOwner->m_nButtons & (~m_nAttack2Debounce); | ||
| int nAttack2Mask = pOwner->m_nButtons & (~m_nAttack2Debounce); // NEO TODO (Adam) Change to int64 when building this? |
There was a problem hiding this comment.
Assert(false) so whoever hits this has to deal with it?
| void CPropJeep::DriveVehicle( float flFrameTime, CUserCmd *ucmd, int iButtonsDown, int iButtonsReleased ) | ||
| { | ||
| int iButtons = ucmd->buttons; | ||
| int iButtons = ucmd->buttons; // NEO TODO (Adam) change to int64 when building this |
There was a problem hiding this comment.
Ditto on maybe doing Assert(false) or similar here to force a fix for whoever tries to use it
| int64 bf_read::ReadSignedVarInt64() | ||
| { | ||
| uint32 value = ReadVarInt64(); | ||
| uint32 value = ReadVarInt64(); // NEO TODO (Adam) https://github.com/ValveSoftware/source-sdk-2013/issues/322 |
There was a problem hiding this comment.
Would it make sense to fix this retval type 32->64 here as part of this PR, while we're at it modifying this stuff anyway?
There was a problem hiding this comment.
I did a little test, writing the button state twice, once using writelonglong and once using writesignedvarint64
if ( to->buttons != from->buttons )
{
buf->WriteOneBit( 1 );
#ifdef NEO
buf->WriteLongLong( to->buttons );
buf->WriteSignedVarInt64( to->buttons );
#else
buf->WriteUBitLong( to->buttons, 32 );
#endif // NEO
} // read buttons
if ( buf->ReadOneBit() )
{
#ifdef NEO
move->buttons = buf->ReadLongLong();
int64 buttons2 = buf->ReadSignedVarInt64();
Assert(move->buttons == buttons2);
#else
move->buttons = buf->ReadUBitLong( 32 );
#endif // NEO
}using jump2 triggers the Assert before the fix and doesn't trigger the Assert after the fix
There was a problem hiding this comment.
of course this isn't an exhaustive test, ReadSignedVarInt64 exists separate to writeLongLong for a reason, probably handles a sign bit, but i'm also confident the fix is fine
There was a problem hiding this comment.
I'll mention the issue in a github comment for posterity ValveSoftware/source-sdk-2013#322
Description
+jump2 command that does a normal jump without doing a superjump. Personally I bind mwheelup to +jump2 and mwheeldown to +jump
Decided to extend m_nButtons to 64 bits so the PR is a bit involved
Toolchain
Linked Issues