player_t::GetPSprite crashes if ReadyWeapon is null

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.
Post Reply
User avatar
phantombeta
Posts: 2084
Joined: Thu May 02, 2013 1:27 am
Operating System Version (Optional): Windows 10
Graphics Processor: nVidia with Vulkan support
Location: Brazil

player_t::GetPSprite crashes if ReadyWeapon is null

Post by phantombeta »

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.
Attachments
GZDoom_GetPSpriteCrash.pk3
(356 Bytes) Downloaded 34 times
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by Rachael »

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).
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by Graf Zahl »

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.
User avatar
Rachael
Posts: 13530
Joined: Tue Jan 13, 2004 1:31 pm
Preferred Pronouns: She/Her
Contact:

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by Rachael »

Yikes, so I remembered correctly...
User avatar
Major Cooke
Posts: 8170
Joined: Sun Jan 28, 2007 3:55 pm
Preferred Pronouns: He/Him
Location: QZDoom Maintenance Team

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by Major Cooke »

Isn't that why FindPSprite was made?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by Graf Zahl »

Not really. FindPSprite only checks if something already exists, not whether it can exist.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: player_t::GetPSprite crashes if ReadyWeapon is null

Post by _mental_ »

It was fixed in a638cfb + 4201c4f.
Post Reply

Return to “Closed Bugs [GZDoom]”