1dot13 / source

Source code for the game executable of the Jagged Alliance 2 v1.13 project
101 stars 18 forks source link

Check RemoveSoldierNodeFromInitList() for correct behavior #277

Open Asdow opened 9 months ago

Asdow commented 9 months ago

The else branch with DebugBreakpoint() call feels very suspicious to me, and like it could be a leftover from original code when 20 hired mercs was the maximum amount.

I don't see any reason why it would not be possible nowadays for the player to have soldiers with ubID > 20.

void RemoveSoldierNodeFromInitList( SOLDIERINITNODE *pNode )
{
    if( !pNode )
        return;
    if( gfOriginalList )
        gMapInformation.ubNumIndividuals--;
    if( pNode->pBasicPlacement )
    {
        MemFree( pNode->pBasicPlacement );
        pNode->pBasicPlacement = NULL;
    }
    if( pNode->pDetailedPlacement )
    {
        delete( pNode->pDetailedPlacement );
        pNode->pDetailedPlacement = NULL;
    }
    if( pNode->pSoldier )
    {
        if( pNode->pSoldier->ubID >= 20 )
        {
            TacticalRemoveSoldier( pNode->pSoldier->ubID );
        }
        else
        {
            //bug if gets here and we aren't exiting the game, maybe a bug even if we are exiting (but who cares)
            //bug exists since beginning of SVN
            DebugBreakpoint();
            //INT8 bug = 0;
        }
    }
    if( pNode == gSoldierInitHead )
    {
        gSoldierInitHead = gSoldierInitHead->next;
        if( gSoldierInitHead )
            gSoldierInitHead->prev = NULL;
        if( gfOriginalList )
            gOriginalSoldierInitListHead = gSoldierInitHead;
        else
            gAlternateSoldierInitListHead = gSoldierInitHead;
    }
    else if( pNode == gSoldierInitTail )
    {
        gSoldierInitTail = gSoldierInitTail->prev;
        gSoldierInitTail->next = NULL;
    }
    else
    {
        pNode->prev->next = pNode->next;
        pNode->next->prev = pNode->prev;
    }
    MemFree( pNode );
}