[up to 3.3.2] MeansOfDeath is in the wrong place

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.
User avatar
Matt
Posts: 9696
Joined: Sun Jan 04, 2004 5:37 pm
Preferred Pronouns: They/Them
Operating System Version (Optional): Debian Bullseye
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia
Contact:

[up to 3.3.2] MeansOfDeath is in the wrong place

Post by Matt »

(formerly "[3.3.2] A_CustomMeleeAttack ignores its own damagetype")

Steps: Let this imp kill you.

Expected: Obituary is just "melee" (or "claws" if you use the alternate line)
Actual: Obituary is just "none"

Code: Select all

class ObitImp:DoomImp{
	override string getobituary(actor victim,actor inflictor,name mod,bool playerattack){
		return mod;
	}
	states{
	melee:
		#### EE 4 A_FaceTarget();
		#### F 2;
		//#### G 8 A_CustomMeleeAttack(random(10,30),"imp/melee","","claws",true);
		#### G 8 A_CustomMeleeAttack(random(10,30));
		#### F 4;
		goto see;
	}
}
Last edited by Matt on Mon Apr 23, 2018 4:47 pm, edited 1 time in total.
Blue Shadow
Posts: 4949
Joined: Sun Nov 14, 2010 12:59 am

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Blue Shadow »

Tested it. It works fine on my end (GZDoom 3.3.2); I get the "expected" behavior.
_mental_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by _mental_ »

Matt wrote:Steps: Let this imp kill you.
How exactly? For melee attack the sample will print an expected value. For range one it will be none.
User avatar
Matt
Posts: 9696
Joined: Sun Jan 04, 2004 5:37 pm
Preferred Pronouns: They/Them
Operating System Version (Optional): Debian Bullseye
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia
Contact:

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Matt »

Turns out it's something in my playerpawn's Die override.

But it's weird, though: the culprit is a corpse actor that is spawned, set so that its master is the real playerpawn, then A_Die() is called. The ObitImp then takes whatever damagetype is from that A_Die() call rather than what actually happened to the player.

Why would calling A_Die on another actor do something like that?
EDIT: seems like any valid A_Die will do, it'll just take whichever one's the last... I can even kill the source with "if(source)source.A_Die("flubbs");" and "flubbs" will be the basis of the obituary.


Does anyone know how to get the "mod" from inside Die()?
Blue Shadow
Posts: 4949
Joined: Sun Nov 14, 2010 12:59 am

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Blue Shadow »

I think what's going on here is something to do with the MeansOfDeath global variable. When something is damaged, the damage type is stored in this variable. ClientObituary() (the obituary function) reads it to determine the damage type. Going by what I could piece from your post, let's say we have this:

Code: Select all

class TestPlayer : DoomPlayer
{
    override void Die (Actor source, Actor inflictor, int dmgflags)
    {
        if (source)
        {
            source.A_Die('Ice');
        }

        Super.Die(source, inflictor, dmgflags);
    }
}
When ObitImp damages the player, MeansOfDeath becomes whatever the damage type the imp inflicted (let's say "claw", in this case). If the imp's attack was enough to kill the player, TestPlayer.Die() is called, and the first thing it does is kill the imp with ice damage. Now MeansOfDeath changes to "Ice". Then, Super.Die() is called to do its thing, which includes calling ClientObituary(). At this point the damage type is "Ice", which what is used as the obituary message.

In TestPlayer.Die(), if you call the super function before killing 'source', the damage type and thus the obituary will be "claw".

Here is something people can load and test: summon ObitImp and let it kill you with its melee attack. Upon your death, the imp dies, and you get "Ice" printed as the obituary message.
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: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Graf Zahl »

This is a textbook example of why global variables are BAD. Too bad I overlooked it when I worked on this code to expose the suitable parts to scripting.
User avatar
Matt
Posts: 9696
Joined: Sun Jan 04, 2004 5:37 pm
Preferred Pronouns: They/Them
Operating System Version (Optional): Debian Bullseye
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia
Contact:

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Matt »

Is there any way to retrieve MeansOfDeath?
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: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Graf Zahl »

No, and there won't be. The entire thing looks like some lazy way of passing a parameter through more than one function level. It's better to just get rid of entirely.
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: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Graf Zahl »

So, here's the problem: To fix this bug (and it's indeed a serious bug with undefined behavior in several places), Actor.Die requires a new parameter. But this is a virtual override for scripting.
So it cannot be fixed without making that override incompatible. But not fixing it will leave the entire system open to undefined-ness.

The entire thing is just so that DamageMobj could pass a parameter to ClientObituary - while avoiding to add a new function argument to AActor::Die which is in the call chain between those two functions.
It's the kind of coding stupidity that has made the entire original software renderer such a nightmare to maintain.

So how can we solve this properly without breaking old scripts?
For now I committed the code to a branch for obvious reasons.
User avatar
Arctangent
Posts: 1235
Joined: Thu Nov 06, 2014 1:53 pm
Contact:

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Post by Arctangent »

... Isn't this exactly what ZScript versioning is for? Or is that less powerful of a tool than I'm imagining?
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: [up to 3.3.2] MeansOfDeath is in the wrong place

Post by Graf Zahl »

This one goes a bit deeper, unfortunately. I had to change the signature of one function, and that's something the current versioning system cannot handle. A signature needs to be unique with how things are implemented.
It's also a bit nasty to handle, because depending on what an actor defines, CallDie needs to make a decision about which of the two functions needs to be called.
Blue Shadow
Posts: 4949
Joined: Sun Nov 14, 2010 12:59 am

Re: [up to 3.3.2] MeansOfDeath is in the wrong place

Post by Blue Shadow »

How about making the variable an actor variable instead of a global one; each actor gets an instance which is set upon their death. It's not the best solution, but you won't need to extend Die() this way.
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: [up to 3.3.2] MeansOfDeath is in the wrong place

Post by Graf Zahl »

Because that's just moving the problem around, waiting for the next time to bite you in the ass.
User avatar
Xaser
 
 
Posts: 10772
Joined: Sun Jul 20, 2003 12:15 pm
Contact:

Re: [up to 3.3.2] MeansOfDeath is in the wrong place

Post by Xaser »

Since function overloading isn't a thing (and won't ever be, as it seems), here's an odd proposal: allow virtual overrides to omit trailing parameters in their function signature (with the obvious caveat that if you don't add the parameter, you can't use it). This would implicitly fix the problem since existing Die() overrides will work after the base implementation adds a new parameter.

This is certainly a departure from the C-ish way of doing things (I'm pulling this idea from Javascript functions, so this is probably evil), and there's probably a technical challenge (or impossibility) to this, but it's a thought.
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: [up to 3.3.2] MeansOfDeath is in the wrong place

Post by Graf Zahl »

The idea is not bad, but that is only feasible if the omission is versioned, i.e. if you declare a newer script version the omission is illegal, but unlike any other idea I had this is doable.
Post Reply

Return to “Closed Bugs [GZDoom]”