[up to 3.3.2] MeansOfDeath is in the wrong place

Is there something that doesn't work right in the latest GZDoom? Post about it here.

Moderator: Developers

Forum rules
Please construct and post a simple demo whenever possible for all bug reports. Please provide links to everything.

If you can include a wad demonstrating the problem, please do so. Bug reports that include fully-constructed demos have a much better chance of being investigated in a timely manner than those that don't.

[up to 3.3.2] MeansOfDeath is in the wrong place

Postby Matt » Sun Apr 22, 2018 2:50 am

(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 allExpand view
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 5:47 pm, edited 1 time in total.
User avatar
Matt
Putting the XD into *xdeath since 2007
 
Joined: 04 Jan 2004
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Blue Shadow » Sun Apr 22, 2018 3:13 am

Tested it. It works fine on my end (GZDoom 3.3.2); I get the "expected" behavior.
User avatar
Blue Shadow
 
 
 
Joined: 14 Nov 2010

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby _mental_ » Sun Apr 22, 2018 3:17 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Matt » Sun Apr 22, 2018 5:18 pm

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()?
User avatar
Matt
Putting the XD into *xdeath since 2007
 
Joined: 04 Jan 2004
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Blue Shadow » Mon Apr 23, 2018 4:47 am

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 allExpand view
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
Blue Shadow
 
 
 
Joined: 14 Nov 2010

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Graf Zahl » Mon Apr 23, 2018 4:57 am

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
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Matt » Mon Apr 23, 2018 1:48 pm

Is there any way to retrieve MeansOfDeath?
User avatar
Matt
Putting the XD into *xdeath since 2007
 
Joined: 04 Jan 2004
Location: Gotham City SAR, Wyld-Lands of the Lotus People, Dominionist PetroConfederacy of Saudi Canadia

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Graf Zahl » Mon Apr 23, 2018 2:06 pm

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 Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Graf Zahl » Mon Apr 23, 2018 2:23 pm

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
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [3.3.2] A_CustomMeleeAttack ignores its own damagetype

Postby Arctangent » Mon Apr 23, 2018 5:22 pm

... Isn't this exactly what ZScript versioning is for? Or is that less powerful of a tool than I'm imagining?
User avatar
Arctangent
squawky
 
Joined: 06 Nov 2014
Discord: SquawkyAtan#2371

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

Postby Graf Zahl » Tue Apr 24, 2018 1:03 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

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

Postby Blue Shadow » Tue Apr 24, 2018 1:54 am

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
Blue Shadow
 
 
 
Joined: 14 Nov 2010

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

Postby Graf Zahl » Tue Apr 24, 2018 3:13 am

Because that's just moving the problem around, waiting for the next time to bite you in the ass.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

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

Postby Xaser » Tue Apr 24, 2018 11:41 am

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
Xaser
anarchivist
 
 
 
Joined: 20 Jul 2003

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

Postby Graf Zahl » Tue Apr 24, 2018 11:58 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany


Return to Bugs

Who is online

Users browsing this forum: Endie and 1 guest