[Vannila bug] PassengersClass::IndexOf InfiiteLoop

[hzqst]

Description

//473500
int __thiscall PassengersClass::IndexOf(PassengersClass *this, FootClass *pCandidate)
{
  FootClass *pPassenger; // ecx
  int result; // eax
  ObjectClass *NextObject; // edx

  pPassenger = this->FirstPassenger;
  result = 0;
  if ( !pPassenger )
    return 0;
  while ( 1 )
  {
    ++result;
    if ( pPassenger == pCandidate )
      break;
    NextObject = pPassenger->NextObject;
    if ( NextObject )
    {
      if ( (NextObject->AbstractFlags & 4) == 0 )
        return 0;
      pPassenger = (FootClass *)pPassenger->NextObject;
    }
    if ( !pPassenger )
      return 0;
  }
  return result;
}

There is a chance that game can run into infinite loop in PassengersClass::IndexOf and cause the game process to hang up when pPassenger->NextObject become nullptr for unknown reason.

** Possible Fix

The infinite loop gone after patching with a nullptr check, tested for about 10 games with my frnds (random strangers with no patch still got connection lost when we were in the same game):

int __fastcall HookPassengersClass_IndexOf(PassengersClass* pthis, int dummy, FootClass* pCandidate)
{
	auto pPassenger = pthis->FirstPassenger;

	int result = 0;

	if (!pPassenger)
		return 0;

	while (1)
	{
		++result;

		if (pPassenger == pCandidate)
			break;

		auto NextObject = pPassenger->NextObject;

		if (NextObject)
		{
			if (!(NextObject->AbstractFlags & AbstractFlags::Foot))
				return 0;

			pPassenger = (FootClass*)pPassenger->NextObject;
		}
		else
		{
			pPassenger = nullptr; // my fix
		}

		if (!pPassenger)
			return 0;
	}

	return result;

}

Conditions to reproduce

More likely to happen when too many infantries get into BattleFortress as passengers.

More likely to happen when The FIRST passenger in the BattleFortress is an infantry instead of a tank.

Steps to reproduce

1. Make HTNK OpenTopped
2. Make HTNK Passengers=200, SizeLimit=10
3. Put hundreds of infantries, and also some random tanks into HTNK.
4. Wait and see if game run into infinite loop on firing.

Expected behaviour

Game should not hang up

Actual behaviour

Game run into infinite loop and hang up

Additional context

.text:00473500                                     ; int __thiscall PassengersClass::IndexOf(PassengersClass *this, FootClass *pCandidate)

The only caller to PassengersClass::IndexOf is InfantryClass::GetFLH :

CoordStruct *__thiscall InfantryClass::GetFLH(
        InfantryClass *this,
        CoordStruct *a2,
        int idxWeapon,
        CoordStruct baseCoord)
{
  TechnoClass *Transporter; // eax
  int index; // eax
  int v7; // eax
  CoordStruct *v8; // ecx
  CoordStruct *result; // eax
  _BYTE v10[12]; // [esp+10h] [ebp-Ch] BYREF

  if ( this->InOpenToppedTransport
    && (Transporter = this->Transporter) != 0
    && (index = PassengersClass::IndexOf((PassengersClass *)Transporter->TechnoClass_Passengers.__Passengers, this)) != 0 )
  {
    v7 = ((int (__thiscall *)(TechnoClass *, _BYTE *, int, _DWORD, _DWORD, _DWORD))this->Transporter->GetFLH)(
           this->Transporter,
           v10,
           -index,
           0,
           0,
           0);
  }
  else
  {
    v7 = ((int (__stdcall *)(_BYTE *, int, _DWORD, _DWORD, _DWORD))FootClass::GetFLH)(v10, idxWeapon, 0, 0, 0);
  }
  v8 = (CoordStruct *)v7;
  result = a2;
  *a2 = *v8;
  return result;
}

Checklist

• The issue is not introduced by Phobos or any other engine extension, such as Ares, Kratos etc.
• The issue wasn't fixed in the most recent version of Ares/Phobos yet.
• I agree to elaborate the details if requested and provide thorough testing if the bugfix is implemented.
• I added a very descriptive title to this issue.
• I used the GitHub search and read the issue list to find a similar issue and didn't find it.
• I have attached as much information as possible (screenshots, gifs, videos, debug and exception logs, etc).