Memory Corruption Bug Options

[JRob]

I've been looking at the entity handle code, and I think it is bugged and causing crashes.

http://sourceforge.net/p/rcbot2/code/HEAD/...t_ehandle.h#l59

First of all, it should null m_pEnt if it is free or the serial number changed. It shoudn't just leave it until another entity reuses it. even though the chances might be low.

Next, there is a bunch of places where these MyEHandle aren't checked for null (the wrapped entity, not the MyEHandle itself). The first thing that crashed for me was here

http://sourceforge.net/p/rcbot2/code/HEAD/...rtress.cpp#l671

and there are probably a lot more. I fixed CBotFortress :: setVisible though.

CODE

bool CBotFortress :: setVisible ( edict_t *pEntity, bool bVisible )
{
    bool bValid = CBot::setVisible(pEntity,bVisible);

    // check for people to heal
    if ( m_iClass == TF_CLASS_MEDIC )
    {
        if ( bValid && bVisible )
        {
            if (CBotGlobals::isPlayer(pEntity) ) // player
            {
                CBotWeapon *pMedigun = m_pWeapons->getWeapon(CWeapons::getWeapon(TF2_WEAPON_MEDIGUN));
                bool bIsSpy = CClassInterface::getTF2Class(pEntity)==TF_CLASS_SPY;
                int iDisguise = 0;

                if ( bIsSpy )
                {
                    CClassInterface::getTF2SpyDisguised(pEntity,&iDisguise,NULL,NULL,NULL);
                }

                if ( pMedigun && pMedigun->hasWeapon() &&
                    (  // Heal my team member or a spy if I think he is on my team
                       (CBotGlobals::getTeam(pEntity) == getTeam()) ||
                       ((bIsSpy&&!thinkSpyIsEnemy(pEntity,(TF_Class)iDisguise)))
                    ) )
                {
                    Vector vPlayer = CBotGlobals::entityOrigin(pEntity);

                    if ( distanceFrom(vPlayer) <= CWaypointLocations::REACHABLE_RANGE )
                    {
                        float fFactor;

                        if ( (fFactor = getHealFactor(pEntity)) > 0 )
                        {
                            if ( m_pHeal.get() != NULL )
                            {
                                if ( m_pHeal != pEntity )
                                {
                                    if ( fFactor > m_fHealFactor )
                                    {            
                                        m_pHeal = pEntity;
                                        m_fHealFactor = fFactor;
                                        updateCondition(CONDITION_SEE_HEAL);
                                    }
                                }
                                else
                                {
                                    // not healing -- what am I doing?
                                    if ( !m_pSchedules->hasSchedule(SCHED_HEAL) )
                                    {
                                        // not healing -- what am I doing?
                                        m_pSchedules->freeMemory();
                                        m_pSchedules->addFront(new CBotTF2HealSched(m_pHeal));
                                    }
                                }
                            }
                            else
                            {                                
                                m_fHealFactor = fFactor;
                                m_pHeal = pEntity;
                                updateCondition(CONDITION_SEE_HEAL);

                                if ( !m_pSchedules->hasSchedule(SCHED_HEAL) )
                                {
                                    // not healing -- what am I doing?
                                    m_pSchedules->freeMemory();
                                    m_pSchedules->addFront(new CBotTF2HealSched(m_pHeal));
                                }
                            }
                        }
                    }
                }
            }
        }
        else if ( m_pHeal == pEntity )
        {
            m_pHeal = NULL;
            removeCondition(CONDITION_SEE_HEAL);
        }
    }
    //else if ( m_iClass == TF_CLASS_SPY ) // Fix
    //{
        // Look for nearest sentry to sap!!!
    if ( bValid && bVisible )
    {
        if ( CTeamFortress2Mod::isSentry(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( (m_iClass!=TF_CLASS_ENGINEER)||!CClassInterface::isObjectCarried(pEntity) )
            {
                if ( !m_pNearestEnemySentry || m_pNearestEnemySentry.notValid() || ((pEntity != m_pNearestEnemySentry) && (distanceFrom(pEntity) < distanceFrom(m_pNearestEnemySentry)) ))
                {
                    m_pNearestEnemySentry = pEntity;
                }
            }
        }
        else if ( CTeamFortress2Mod::isTeleporter(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( !m_pNearestEnemyTeleporter || m_pNearestEnemyTeleporter.notValid() || ((pEntity != m_pNearestEnemyTeleporter)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestEnemyTeleporter))))
            {
                m_pNearestEnemyTeleporter = pEntity;
            }
        }
        else if ( CTeamFortress2Mod::isDispenser(pEntity,CTeamFortress2Mod::getEnemyTeam(getTeam())) )
        {
            if ( !m_pNearestEnemyDisp || m_pNearestEnemyDisp.notValid() || ((pEntity != m_pNearestEnemyDisp)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestEnemyDisp))))
            {
                m_pNearestEnemyDisp = pEntity;
            }
        }
        else if ( CTeamFortress2Mod::isHurtfulPipeGrenade(pEntity,m_pEdict) )
        {
            if ( !m_pNearestPipeGren || m_pNearestPipeGren.notValid() || ((pEntity != m_pNearestPipeGren)&&(distanceFrom(pEntity)<distanceFrom(m_pNearestPipeGren))))
            {
                m_pNearestPipeGren = pEntity;
            }
        }
    }
    else if ( pEntity == m_pNearestEnemySentry )
    {
        m_pNearestEnemySentry = NULL;
    }
    else if ( pEntity == m_pNearestEnemyTeleporter )
    {
        m_pNearestEnemyTeleporter = NULL;
    }
    else if ( pEntity == m_pNearestEnemyDisp )
    {
        m_pNearestEnemyDisp = NULL;
    }
    else if ( pEntity == m_pNearestPipeGren )
    {
        m_pNearestPipeGren = NULL;
    }

    //}

    // Check for nearest Dispenser for health/ammo & flag
    if ( bValid && bVisible && !(CClassInterface::getEffects(pEntity)&EF_NODRAW) ) // EF_NODRAW == invisible
    {
        if ( m_pFlag != pEntity && CTeamFortress2Mod::isFlag(pEntity,getTeam()) )
            m_pFlag = pEntity;
        else if ( (m_pNearestAllySentry != pEntity) && CTeamFortress2Mod::isSentry(pEntity,getTeam()) )
        {
            if ( !m_pNearestAllySentry || m_pNearestAllySentry.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestAllySentry)))
                m_pNearestAllySentry = pEntity;
        }
        else if ( (m_pNearestDisp != pEntity) && CTeamFortress2Mod::isDispenser(pEntity,getTeam()) )
        {
            if ( !m_pNearestDisp || m_pNearestDisp.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestDisp)) )
                m_pNearestDisp = pEntity;
        }
        else if ( (pEntity != m_pNearestTeleEntrance) && CTeamFortress2Mod::isTeleporterEntrance(pEntity,getTeam()) )
        {
            if ( !m_pNearestTeleEntrance || m_pNearestTeleEntrance.notValid() || (distanceFrom(pEntity) < distanceFrom(m_pNearestTeleEntrance)))
                m_pNearestTeleEntrance = pEntity;
        }
        else if ( (pEntity != m_pAmmo) && CTeamFortress2Mod::isAmmo(pEntity) )
        {
            static float fDistance;

            fDistance = distanceFrom(pEntity);

            if ( fDistance <= 200 )
            {

                if ( !m_pAmmo || m_pAmmo.notValid() || (fDistance < distanceFrom(m_pAmmo)))
                    m_pAmmo = pEntity;
            }
            
        }
        else if ( (pEntity != m_pHealthkit) && CTeamFortress2Mod::isHealthKit(pEntity) )
        {
            static float fDistance;

            fDistance = distanceFrom(pEntity);

            if ( fDistance <= 200 )
            {
                if ( !m_pHealthkit || m_pHealthkit.notValid() || (fDistance < distanceFrom(m_pHealthkit)))
                    m_pHealthkit = pEntity;
            }
        }
    }
    else
    {
        if ( pEntity == m_pFlag.get_old() )
            m_pFlag = NULL;
        else if ( pEntity == m_pNearestDisp.get_old() )
            m_pNearestDisp = NULL;
        else if ( pEntity == m_pAmmo.get_old() )
            m_pAmmo = NULL;
        else if ( pEntity == m_pHealthkit.get_old() )
            m_pHealthkit = NULL;
        else if ( pEntity == m_pHeal.get_old() )
            m_pHeal = NULL;
        else if ( pEntity == m_pNearestPipeGren.get_old() )
            m_pNearestPipeGren = NULL;
    }

    return bValid;
}

Here are the MyEHandles in bot_fortress.cpp. It looks like you are correctly checking some of them.

MyEHandle m_pHeal;
MyEHandle m_pLastHeal;
MyEHandle m_pSentryGun;
MyEHandle m_pDispenser;
MyEHandle m_pTeleEntrance;
MyEHandle m_pTeleExit;

MyEHandle m_pAmmo;
MyEHandle m_pHealthkit;

MyEHandle m_pNearestDisp;
MyEHandle m_pNearestEnemySentry;
MyEHandle m_pNearestAllySentry;
MyEHandle m_pNearestEnemyTeleporter;
MyEHandle m_pNearestEnemyDisp;
MyEHandle m_pNearestTeleEntrance;
MyEHandle m_pNearestPipeGren;

MyEHandle m_pFlag;
MyEHandle m_pPrevSpy;

MyEHandle m_pHealer;

MyEHandle m_pLastEnemySentry;
MyEHandle m_NearestEnemyRocket;
MyEHandle m_NearestEnemyGrenade;


MyEHandle m_pDefendPayloadBomb;
MyEHandle m_pPushPayloadBomb;
MyEHandle m_pRedPayloadBomb;
MyEHandle m_pBluePayloadBomb;

[JRob]

Found another crash, possibly related.

This code is accessing out the bounds of the m_SentryGuns array.

http://sourceforge.net/p/rcbot2/code/HEAD/...ot_mods.h#l1082

It is going from int i = 1; i <= gpGlobals->maxClients when every other function does i = 0; i < MAX_PLAYERS

You'll probably also need to change INDEXENT(i-1) to INDEXENT(i+1)

And another one.

http://sourceforge.net/p/rcbot2/code/HEAD/...2_mod.cpp#l1412

And another

http://sourceforge.net/p/rcbot2/code/HEAD/...2_mod.cpp#l1461

Another one

http://sourceforge.net/p/rcbot2/code/HEAD/...a/bot.cpp#l1884

It looks like there are a lot of off by 1 bugs involving the use of gpGlobals->maxClients

A missing check for edict->IsFree()

http://sourceforge.net/p/rcbot2/code/HEAD/...a/bot.cpp#l3239

m_Resource needs to be checked for validity.

http://sourceforge.net/p/rcbot2/code/HEAD/...2_points.h#l307

[Cheeseh]

I've fixed the sentry gun stuff, those were silly mistakes

Though the logic here is fine:

CODE

if ( !m_pAmmo || (fDistance < distanceFrom(m_pAmmo)))

It won't execute (fDistance < distanceFrom(m_pAmmo) unless m_pAmmo is not NULL.

If it is crashing here then I've yet to experience it, but it could be because of another unexpected problem within the operator. I'd rather have the code there than adding extra functions

CODE

    inline operator edict_t * const ()
    { // same as get function (inlined for speed)
        if ( m_iSerialNumber && m_pEnt )
        {
            if ( !m_pEnt->IsFree() && (m_iSerialNumber == m_pEnt->m_NetworkSerialNumber) )
                return m_pEnt;
        }
        else if ( m_pEnt )
            m_pEnt = NULL;

        return NULL;
    }

CODE

    if ( (m_iStatsIndex == 0) || ( m_iStatsIndex > gpGlobals->maxClients ) )
    {
        if ( m_iStatsIndex != 0 )
            m_bStatsCanUse = true;


        m_StatsCanUse.data = m_Stats.data;
        m_Stats.data = 0;
        m_iStatsIndex = 0; // reset to be sure in case of m_iStatsIndex > gpGlobals->maxClients


        if ( !m_uSquadDetail.b1.said_area_clear && (m_StatsCanUse.stats.m_iEnemiesInRange == 0) && (m_StatsCanUse.stats.m_iEnemiesVisible == 0) && (m_StatsCanUse.stats.m_iTeamMatesInRange > 0))
        {
            if ( !inSquad() || isSquadLeader() && (m_fLastSeeEnemy && ((m_fLastSeeEnemy + 10.0f)<engine->Time())) )

                areaClear();

            m_uSquadDetail.b1.said_area_clear = true;

        }
    }

    CClient *pClient = CClients::get(m_iStatsIndex++);

This is fine too. Which means if m_iStatsIndex is outside the bounds of players it will reset to zero. Every CClient is the same as an INDEXENT(m_iStatsIndex+1) However it doesn't need to use CClients I'm going to change it just to use INDEXENT.

CODE

    CTeamRoundTimer()::MyEHandle m_Resource;

this is not referenced in the code. it is only set once at map change.

[JRob]

I've fixed the sentry gun stuff, those were silly mistakes

Though the logic here is fine:

CODE

if ( !m_pAmmo || (fDistance < distanceFrom(m_pAmmo)))

It won't execute (fDistance < distanceFrom(m_pAmmo) unless m_pAmmo is not NULL.

If it is crashing here then I've yet to experience it, but it could be because of another unexpected problem within the operator. I'd rather have the code there than adding extra functions

CODE

    inline operator edict_t * const ()
    { // same as get function (inlined for speed)
        if ( m_iSerialNumber && m_pEnt )
        {
            if ( !m_pEnt->IsFree() && (m_iSerialNumber == m_pEnt->m_NetworkSerialNumber) )
                return m_pEnt;
        }
        else if ( m_pEnt )
            m_pEnt = NULL;

        return NULL;
    }

CODE

    if ( (m_iStatsIndex == 0) || ( m_iStatsIndex > gpGlobals->maxClients ) )
    {
        if ( m_iStatsIndex != 0 )
            m_bStatsCanUse = true;
        m_StatsCanUse.data = m_Stats.data;
        m_Stats.data = 0;
        m_iStatsIndex = 0; // reset to be sure in case of m_iStatsIndex > gpGlobals->maxClients
        if ( !m_uSquadDetail.b1.said_area_clear && (m_StatsCanUse.stats.m_iEnemiesInRange == 0) && (m_StatsCanUse.stats.m_iEnemiesVisible == 0) && (m_StatsCanUse.stats.m_iTeamMatesInRange > 0))
        {
            if ( !inSquad() || isSquadLeader() && (m_fLastSeeEnemy && ((m_fLastSeeEnemy + 10.0f)<engine->Time())) )

                areaClear();

            m_uSquadDetail.b1.said_area_clear = true;

        }
    }

    CClient *pClient = CClients::get(m_iStatsIndex++);

This is fine too. Which means if m_iStatsIndex is outside the bounds of players it will reset to zero. Every CClient is the same as an INDEXENT(m_iStatsIndex+1) However it doesn't need to use CClients I'm going to change it just to use INDEXENT.

CODE

    CTeamRoundTimer()::MyEHandle m_Resource;

this is not referenced in the code. it is only set once at map change.

No you still need to check m_pAmmo. Just because m_pAmmo isn't null doesn't mean it is valid anymore because it could have been deleted by code outside of rcbot. That's the whole point of of the EHandle, to tell when an entity was deleted outside your control. And also m_pAmmo being null only means that it doesn't have a pointer to MyEhandle, it doesn't check the underlying entity to see if it is valid.

I assure you that the m_iStatsIndex code isn't fine otherwise it wouldn't be crashing there, and doesn't crash anymore after I fixed it.

You have defined an array from 0 to 32 (33 entries). With that code you are stopping right after 33 since gpGlobals->maxClients is 33 when you have the replay bot or using the player limit increaser. You probably never see it crash because you only test it on your listen server.

Every CClient is the same as an INDEXENT(m_iStatsIndex+1)

This is not correct. All your arrays are from 0 to MAXPLAYERS-1. Also CClients :: slotOfEdict does ENTINDEX(pPlayer)-1, so CClients is 0 based, not 1 based.

static CClient m_Clients[MAX_PLAYERS];

This defines an array from 0 to 32. If you wanted it to go from 1 to 33, you need to do m_teleporters[MAX_PLAYERS+1]

Also you are doing CClients::get(m_iStatsIndex++);. This increments the index pointer after you access it. So you are not accessing m_iStatsIndex+1.

And with https://sourceforge.net/p/rcbot2/code/HEAD/...2_points.h#l307 that is the exact location of the crash as indicated by crash dumps. I am not guessing where crashes might occur, it has actually happened there. The problem is simple to figure out. The underlying entity became invalid and thus m_nSetupTimeLength turned into an invalid access

Just found another crash. https://sourceforge.net/p/rcbot2/code/HEAD/...rtress.cpp#l880

m_pAvoidEntity immediately detects pDetector is invalid and turns into null.

[Cheeseh]

No you still need to check m_pAmmo. Just because m_pAmmo isn't null doesn't mean it is valid anymore because it could have been deleted by code outside of rcbot. That's the whole point of of the EHandle, to tell when an entity was deleted outside your control. And also m_pAmmo being null only means that it doesn't have a pointer to MyEhandle, it doesn't check the underlying entity to see if it is valid.

I understand the point of Ehandle . If you call !m_pAmmo the following code is run : therefore if the serialnumber isn't matched it will return NULL. So it does check whether or not it is valid without extra code.

CODE

   inline operator edict_t * const ()
    { // same as get function (inlined for speed)
        if ( m_iSerialNumber && m_pEnt )
        {
            if ( !m_pEnt->IsFree() && (m_iSerialNumber == m_pEnt->m_NetworkSerialNumber) )
                return m_pEnt;
        }
        else if ( m_pEnt )
            m_pEnt = NULL;

        return NULL;
    }

This works fine in Windows in Visual Studio. Perhaps Linux the operator doesn't overload

[Cheeseh]

thanks for finding more bugs, lots of help

Every CClient is the same as an INDEXENT(m_iStatsIndex+1)

This is not correct. All your arrays are from 0 to MAXPLAYERS-1. Also CClients :: slotOfEdict does ENTINDEX(pPlayer)-1, so CClients is 0 based, not 1 based.

I'm just letting you know how its set up so forgive me if it's wrong I haven't used or looked at the code for literally years hence the modify date.

so this here:

CODE

static CClient m_Clients[MAX_PLAYERS];

INDEXENT(i) is the same as CClient(i-1).getPlayer()

because of this

CODE

int CClients :: slotOfEdict ( edict_t *pPlayer )
{
    return ENTINDEX(pPlayer)-1;
}

CClient *CClients :: clientConnected ( edict_t *pPlayer )
{
    CClient *pClient = &m_Clients[slotOfEdict(pPlayer)];

    pClient->clientConnected(pPlayer);

    return pClient;
}

so every player is shifted into the 0 to 31 bounds

I know it's confusing because iStatsIndex starts at 0, which is worldspawn, not a player, so iStatsIndex + 1 is player 1. which will be in CClients(0) thus CClients(0).getPlayer() == INDEXENT(1)

there should be a loop function instead of doing this manually, but right now this is how it's done. I might create an iterator function to remove this confusion.

I'm redoing this code anyway to remove the use of CClient. it's not even required here.

[Cheeseh]

ps. It might be good to know which map is causing the "if ( m_nSetupTimeLength )" error

[JRob]

You've correctly converted from 1-based index to a 0-based index there, but I am telling you your loop is off by 1.

You terminate the loop at m_iStatsIndex > gpGlobals->maxClients when it should be terminated at m_iStatsIndex = gpGlobals->maxClients.

gpGlobals->maxClients is 33 so the client array goes from 0 to 32. And m_iStatsIndex tries to access index 33.

CODE

inline operator edict_t * const ()

I don't follow how doing !m_pAmmo would execute this code. That code should only run when the variable is casted or put into a parameter that requires an edict_t*. I believe you would have to overload the ! operator to do that.

Also I believe the m_nSetupTimeLength length was happening on random maps.

[JRob]

I have tried to merge my code with nightcore's fork.

I'm not sure if I forgot to tell you this, but this null check needs to be changed to !CBotGlobals::entityIsValid(pSentryEnemy).

https://sourceforge.net/p/rcbot2/code/480/t...tress.cpp#l7493

[JRob]

Found another issue here. A null edict_t is being passed to CBotTF2HealSched.

Shouldn't these task functions check if the edict is valid? Also these look like callbacks. Shouldn't they check if the edict is still valid? Shouldn't they all be using EHandles?

#0 0xe6788a88 in CBaseEdict::GetIServerEntity (this=0x0) at ../sdk/hl2sdk-ob-valve/public/edict.h:238
#1 0xe678931c in CClassInterfaceValue::getBool (this=0xffbc4d14, edict=0x0, defaultvalue=244, bIsEdict=false) at bot_getprop.h:196
#2 0xe68224a8 in CFindPathTask::debugString (this=0xb8648d30, string=0x0) at bot_task.cpp:2046
#3 0xe68144b1 in CBotTF2HealSched::CBotTF2HealSched (this=0xb3de6f40, pHeal=0x0) at bot_schedule.cpp:121
#4 0xe67c6eda in CBotFortress::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:596
#5 0xe67d25bb in CBotTF2::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:3945
#6 0xe683a141 in CBotVisibles::updateVisibles (this=0xb2c8c880) at bot_visibles.cpp:316
#7 0xe679771d in CBot::think (this=0xeb37f000) at bot.cpp:957
#8 0xe679fcf7 in CBots::botThink () at bot.cpp:3548
#9 0xe680a4b2 in RCBotPluginMeta::Hook_GameFrame (this=0xe68d8ce0 <g_RCBotPluginMeta>, simulating=true) at bot_plugin_meta.cpp:1413

[JRob]

I thought I mentioned this

https://sourceforge.net/p/rcbot2/code/480/t...2_mod.cpp#l1542

this needs to be ENTINDEX(pOwner) - 1

[Cheeseh]

Found another issue here. A null edict_t is being passed to CBotTF2HealSched.

Shouldn't these task functions check if the edict is valid? Also these look like callbacks. Shouldn't they check if the edict is still valid? Shouldn't they all be using EHandles?

#0 0xe6788a88 in CBaseEdict::GetIServerEntity (this=0x0) at ../sdk/hl2sdk-ob-valve/public/edict.h:238
#1 0xe678931c in CClassInterfaceValue::getBool (this=0xffbc4d14, edict=0x0, defaultvalue=244, bIsEdict=false) at bot_getprop.h:196
#2 0xe68224a8 in CFindPathTask::debugString (this=0xb8648d30, string=0x0) at bot_task.cpp:2046
#3 0xe68144b1 in CBotTF2HealSched::CBotTF2HealSched (this=0xb3de6f40, pHeal=0x0) at bot_schedule.cpp:121
#4 0xe67c6eda in CBotFortress::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:596
#5 0xe67d25bb in CBotTF2::setVisible (this=0xeb37f000, pEntity=0xec0000d8, bVisible=true) at bot_fortress.cpp:3945
#6 0xe683a141 in CBotVisibles::updateVisibles (this=0xb2c8c880) at bot_visibles.cpp:316
#7 0xe679771d in CBot::think (this=0xeb37f000) at bot.cpp:957
#8 0xe679fcf7 in CBots::botThink () at bot.cpp:3548
#9 0xe680a4b2 in RCBotPluginMeta::Hook_GameFrame (this=0xe68d8ce0 <g_RCBotPluginMeta>, simulating=true) at bot_plugin_meta.cpp:1413

I don't want to check every function. Some functions should just expect a non NULL pointer. if there is a crash then that's a sign of something going wrong in the code above. I don't know why it's going into DebugString function, it seems to be going wrong in there

I thought I mentioned this

https://sourceforge.net/p/rcbot2/code/480/t...2_mod.cpp#l1542

this needs to be ENTINDEX(pOwner) - 1

cheers

[Cheeseh]

I don't want to check every function. Some functions should just expect a non NULL pointer. if there is a crash then that's a sign of something going wrong in the code above. I don't know why it's going into DebugString function, it seems to be going wrong in there
cheers

weel the medic crash was caused by the change we made to the ehandle by making the edit NULL . the best idea is not to change it. I changed entityIsValid and the setvisible bValid Boolean and it's not crashing now

[JRob]

weel the medic crash was caused by the change we made to the ehandle by making the edit NULL . the best idea is not to change it. I changed entityIsValid and the setvisible bValid Boolean and it's not crashing now

I don't think you should remove the ehandle changing it to null, because in that case you are returning null anyway if the serial number doesn't match.

In the rare chance that it becomes valid in the future, you will be accessing the wrong entity.

Also do you know for sure that the serial number can't be 0?

[Cheeseh]

I don't think you should remove the ehandle changing it to null, because in that case you are returning null anyway if the serial number doesn't match.

In the rare chance that it becomes valid in the future, you will be accessing the wrong entity.

Also do you know for sure that the serial number can't be 0?

this ehandle code comes from HL1 not HL2 so there may be some differences, Apparently hl2 invalid serial number is -1 (INVALID_EHANDLE_INDEX) so I might change it to that.

the problem with changing the edict to NULL is, in that in the code above I can assign an ehandle to an edict expecting the ehandle to have a valid edict, although once it goes into the assign code within the ehandle it will nullify the ehandle, therefore making the expected non null ehandle null, causing all sorts of problems. It doesn't matter whether or not its null as it will always return null anyway if its invalid, but the previous invalid edict will still be stored albeit inaccessible.

[JRob]

this ehandle code comes from HL1 not HL2 so there may be some differences, Apparently hl2 invalid serial number is -1 (INVALID_EHANDLE_INDEX) so I might change it to that.

the problem with changing the edict to NULL is, in that in the code above I can assign an ehandle to an edict expecting the ehandle to have a valid edict, although once it goes into the assign code within the ehandle it will nullify the ehandle, therefore making the expected non null ehandle null, causing all sorts of problems. It doesn't matter whether or not its null as it will always return null anyway if its invalid, but the previous invalid edict will still be stored albeit inaccessible.

I'm not sure I understand.

How do you assign an ehandle to an edict before the engine gives you a valid edict? As I understand it, edicts are given to you by the engine and you can't create them without a valid serial number.

Also I don't see the assignment operator checking the serial number. The edict needs to have a valid serial number when it is put into the ehandle because that's the only way and the only time it is saved to m_iSerialNumber.

MyEHandle ( edict_t *pent )
{
m_pEnt = pent;

if ( pent )
m_iSerialNumber = pent->m_NetworkSerialNumber;
else
m_iSerialNumber = 0;
}


inline edict_t * operator = ( edict_t *pent )
{
m_pEnt = pent;

if ( pent )
m_iSerialNumber = pent->m_NetworkSerialNumber;
else
m_iSerialNumber = 0;

return m_pEnt;
}