Off by one error in multisource

[SamVanheer]

Multisource has an off by one error that can cause invalid triggers to be considered valid and toggle the trigger state of the last entity that was registered by the multisource as targeting it.

This can happen if an entity uses a means of targeting a multisource that does not involve the "target" keyvalue, or a multi_manager. For instance, a monster's trigger target could cause it.

The problem is here: https://github.com/ValveSoftware/halflife/blob/5d761709a31ce1e71488f2668321de05f791b405/dlls/buttons.cpp#L192

It should be >=, not >.

[SamVanheer]

The solution i provided is incorrect. If the entity that is triggering the multisource is the last one in the list, it will also result in the index equaling m_iTotal.

This solution works properly in that case: https://github.com/SamVanheer/HLEnhanced/blob/master/game/server/entities/CMultiSource.cpp#L68

Note that the usage of i has changed as well later in the method.

[SamVanheer]

Note that this issue will require extensive testing to make sure official games aren't relying on this behavior.

[SamVanheer]

Here's the fix for this issue:

Change this method: https://github.com/ValveSoftware/halflife/blob/5d761709a31ce1e71488f2668321de05f791b405/dlls/buttons.cpp#L182-L211

To this:

void CMultiSource::Use( CBaseEntity *pActivator, CBaseEntity *pCaller, USE_TYPE useType, float value )
{ 
    int i = 0;

    // Find the entity in our list
    for (; i < m_iTotal; ++i)
    {
        if (m_rgEntities[i] == pCaller)
        {
            break;
        }
    }

    // if we didn't find it, report error and leave
    if (i >= m_iTotal)
    {
        ALERT(at_console, "MultiSrc:Used by non member %s.\n", STRING(pCaller->pev->classname));
        return; 
    }

    // CONSIDER: a Use input to the multisource always toggles.  Could check useType for ON/OFF/TOGGLE

    m_rgTriggered[i] ^= 1;

    // 
    if ( IsTriggered( pActivator ) )
    {
        ALERT( at_aiconsole, "Multisource %s enabled (%d inputs)\n", STRING(pev->targetname), m_iTotal );
        USE_TYPE useType = USE_TOGGLE;
        if ( m_globalstate )
            useType = USE_ON;
        SUB_UseTargets( NULL, useType, 0 );
    }
}

As noted above it needs testing for official games, but it can be applied to the SDK just fine.

[SamVanheer]

I've noticed another edge case: multisource can't handle entities triggering it with a delay. Since delay triggers involve spawning another entity to do the actual triggering the check fails. It may not be possible to fix this issue properly without reworking how delayed triggers are handled.