ZDoom

Posted: **Sun Jan 26, 2020 5:23 pm**

Exactly what it says in the thread title. Someone on the ZDoom Discord (Jekyll Grim Payne) was having issues with this. Taking the ReadyWeapon away and setting ReadyWeapon to null was making his mod crash.
This is caused by the "newcaller" pointer in GetPSprite being set to null, which it seems to not like. (Specifically, one of the weapons in the mod was calling SetPSprite from DoEffect even when not selected)

No idea what would be the best course of action here. To me, it seems like this may have to cause a VM abort. I hope I'm wrong and it can be made to work with a null ReadyWeapon, though, as the ReadyWeapon isn't even the only thing that can create PSprites now anyways.

Steps to reproduce:

Start a new game with the attached example PK3.
Give yourself "Test" in the console.

Posted: **Sun Jan 26, 2020 7:49 pm**

It's been a while since I worked with psprite code but if I remember correctly, too many things depend on ReadyWeapon not being null for some reason. I'm not sure what the best course of action here would be, either, but my opinion is that it would be best to force the code to accept a null ReadyWeapon without errors (or in operations where one absolutely must be present, contingency operations for when it isn't).

Posted: **Mon Jan 27, 2020 1:05 am**

Well, when I see code like this

Code: Select all

	if (layer >= PSP_TARGETCENTER)
	{
		if (mo != nullptr)
		{
			newcaller = mo->FindInventory(NAME_PowerTargeter, true);
		}
	}
	else if (layer == PSP_STRIFEHANDS)
	{
		newcaller = mo;
	}
	else
	{
		newcaller = ReadyWeapon;
	}

	assert(newcaller != nullptr);

I automatically get suspicious. This function was written under the assumption that it can never fail(TM) and logically nearly all calls to it actually don't check if it returns something valid, even in this special case where it simply cannot return something other than NULL. GetPSprite gets called in roughly 30 places - not a single one of them performs validation on the result. The function was "designed to succeed" even though it cannot give that guaerantee.

Posted: **Mon Jan 27, 2020 4:37 am**

Yikes, so I remembered correctly...

Posted: **Mon Jan 27, 2020 9:04 am**

Isn't that why FindPSprite was made?

Posted: **Mon Jan 27, 2020 10:18 am**

Not really. FindPSprite only checks if something already exists, not whether it can exist.

Posted: **Sat Mar 14, 2020 5:04 am**

It was fixed in a638cfb + 4201c4f.

ZDoom

player_t::GetPSprite crashes if ReadyWeapon is null

player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Re: player_t::GetPSprite crashes if ReadyWeapon is null