CModelInfoSA::IsValid has mixed logic, probably because of expanded GTA limits

Describe the bug CModelInfoSA::IsValid has mixed logic, where any model ID between 20000 and MODELINFO_MAX is always considered valid. This causes a problem because it seems interface isn't always initialized (correctly)? Calling certain model info using a model ID (i.e. 20000) results in a crash (see #446 for crash info).

See https://github.com/multitheftauto/mtasa-blue/blob/master/Client/game_sa/CModelInfoSA.cpp#L517

bool CModelInfoSA::IsValid()
{
    if (m_dwModelID >= 20000 && m_dwModelID < MODELINFO_MAX)
        return true;
    return ppModelInfo[m_dwModelID] != 0;
}

To reproduce

uint        uiModelID = 20000;
ushort      usModelID = CModelNames::ResolveModelID(uiModelID);
CModelInfo* pModelInfo = g_pGame->GetModelInfo(usModelID);
if (pModelInfo)
    float fDistance = pModelInfo->GetLODDistance(); // Crash

Expected behaviour Properly return false if the ID is not actually valid.

Version 1.5.6-release-16542.0.000

Additional context

See #446 -> #299
We need to remove any boundary checks (usModelID < 20000) outside CModelInfoSA in the future, as CModelInfoSA::IsValid should return correct status.

https://github.com/multitheftauto/mtasa-blue/pull/299#issuecomment-416843826 @qaisjp commented:

This bug seems to be associated with https://github.com/multitheftauto/mtasa-blue/pull/155/files.

It returns true on this line: https://github.com/multitheftauto/mtasa-blue/pull/155/files#diff-3beea7b726cdd57f96a6d8226af4eb9fR538

Removing the offending line does not fix the problem. Returning false fixes the problem. Is return true a typo?

https://github.com/multitheftauto/mtasa-blue/pull/299#issuecomment-418256833 @Sergeanur commented:

I don't quite remember, but I think CModelInfoSA::IsValid was used on animation request, so return true was required.

https://github.com/multitheftauto/mtasa-blue/pull/784#issuecomment-466710930 @forkerer commented:

As far as I'm aware, (and in the IDB i'm using), 20000 is original size of that array in gta sa, trying to change physical properties of anything above that crashes the client. I think the additional space reserved in CGameSA (up to 26000) was made either by mistake, or in case of mta team wanting to expand some limits.

multitheftauto / mtasa-blue

CModelInfoSA::IsValid has mixed logic, probably because of expanded GTA limits #837