[No] Virtualize all canonical monster-specific attacks

Moderator: GZDoom Developers

Re: Virtualize all canonical monster-specific attacks

Postby Graf Zahl » Thu Mar 28, 2019 1:18 pm

We cannot gratuitously change how those functions work without breaking stuff. It doesn't really matter if you are inconvenienced by it, it is magnitudes more important to keep old stuff working.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Virtualize all canonical monster-specific attacks

Postby gramps » Thu Mar 28, 2019 1:42 pm

I understand the importance of keeping old stuff working. I'm not suggesting breaking it. Although, I suppose deprecating missiletype also broke old stuff, didn't it? But I understand if the oldest stuff takes precedence.

All I'm suggesting is that it's worth looking into ways of doing this that don't sanctify using large chunks of copypasted code as the go-to way to change a missiletype. Not because it's inconvenient, but because it promotes poor practices, the exact kind of thing the wiki warns strongly against on nearly every page: "Wait! Don't copy this giant thing into your mod!"

But if there's no other way to do it, there's no other way.
gramps
 
Joined: 18 Oct 2018

Re: Virtualize all canonical monster-specific attacks

Postby Xaser » Thu Mar 28, 2019 2:21 pm

Going back to the OP (key word "virtualize"), what about DEHACKED support is preventing marking these functions as `virtual`, exactly?
User avatar
Xaser
anarchivist
 
 
 
Joined: 20 Jul 2003

Re: Virtualize all canonical monster-specific attacks

Postby Enjay » Thu Mar 28, 2019 2:56 pm

gramps wrote:Although, I suppose deprecating missiletype also broke old stuff, didn't it?

Probably not. MissileType still works.

Deprecating, at least in GZDoom, means "there is now a better way to do this, so don't use the old way, it is no longer supported". The old way is left as functional as possible (basically as functional as it was just before deprecation) but will no longer be supported in terms of additional features or refinements. It will usually even be maintained to the point that, where possible, its functionality will be preserved unless doing so actively inhibits a newer, better feature. Most deprecated features still work just as well as they ever did - but there are no guarantees that they will continue to do so. This does not mean, of course, that they should be used for new mods, merely that old mods that did use them are unlikely to break.
User avatar
Enjay
Everyone is a moon, and has a dark side which he never shows to anybody. Twain
 
 
 
Joined: 15 Jul 2003
Location: Scotland

Re: Virtualize all canonical monster-specific attacks

Postby Graf Zahl » Thu Mar 28, 2019 2:59 pm

Xaser wrote:Going back to the OP (key word "virtualize"), what about DEHACKED support is preventing marking these functions as `virtual`, exactly?


These functions can be used everywhere and are guaranteed to always do the same thing. If that can no longer be guaranteed we have a mess, not a feature.
Even something as banal as having a monster call both A_MissileAttack and one of the hard coded functions to fire a stock missile is something I have seen.
If you now change how e.g. A_TroopAttack works based on the MissileType property, not even having DoomImpBall as a default would be guaranteed to always do the right thing.

If you change how this function works you create error cases. And that's the end of the story.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Virtualize all canonical monster-specific attacks

Postby gramps » Thu Mar 28, 2019 3:04 pm

@Enjay: Hmm, maybe I'm mis-remembering, but I thought in the old days, you could extend a DoomImp actor, give it a MissileType of PlasmaBall, and voila, it shoots plasma balls now. Is that not how it worked before? It certainly doesn't work like that anymore, if I'm not mistaken, because I can't simply define MissileType as a default property of an Actor at all, it's no longer recognized. And even if I could, its missile type is clearly hard-coded now.

As for Xaser's suggestion, it sounds like what's being suggested here is that the dehacked part of the code would always call the base A_TroopAttack, and never the overridden one. But again I have no idea if that's possible, or at least not a giant pain.
gramps
 
Joined: 18 Oct 2018

Re: Virtualize all canonical monster-specific attacks

Postby Enjay » Thu Mar 28, 2019 3:11 pm

gramps wrote:@Enjay: Hmm, maybe I'm mis-remembering, but I thought in the old days, you could extend a DoomImp actor, give it a MissileType of PlasmaBall, and voila, it shoots plasma balls now. Is that not how it worked before? It certainly doesn't work like that anymore, if I'm not mistaken, because I can't simply define MissileType as a default property of an Actor at all, it's no longer recognized. And even if I could, its missile type is clearly hard-coded now.


My recollection is that it was always intended to be used in conjunction with pointers such as A_MissileAttack and A_ComboAttack which were not given any parameters directly but used the MissileType property instead (and that certainly still works). I don't *think* it could be used to override pointers such as A_TroopAttack with their hard coded missile types, but I'm prepared to be corrected on that.

Of course, all that was in DECORATE. I have no idea if MissileType can be used as a default property in ZScript but, given that would essentially mean extending its functionality, it wouldn't surprise me if it didn't work.
User avatar
Enjay
Everyone is a moon, and has a dark side which he never shows to anybody. Twain
 
 
 
Joined: 15 Jul 2003
Location: Scotland

Re: Virtualize all canonical monster-specific attacks

Postby gramps » Thu Mar 28, 2019 3:22 pm

Well, that would explain a lot, I suppose it would have had the same issues back then as it does now. My reading of the OP suggested that it did used to work like that, and then I thought I remembered it working like that as well, but maybe not.

About OP / Xaser's suggestion, even if dehacked could safely call the original function in the base class while allowing overrides on subclasses, you'd still need to copypaste the entire attack function in order to change that string literal; all it would save is copying part of the state block to call your new function. So I'm not sure of the value in that really, which is why I went off on the MissileType tangent in the first place.
gramps
 
Joined: 18 Oct 2018

Re: Virtualize all canonical monster-specific attacks

Postby Matt » Thu Mar 28, 2019 4:38 pm

My reading of the OP suggested that it did used to work like that
Come to think of it I was probably wrong about that. It was an afterthought example that occurred to me at the last moment of typing.
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: Virtualize all canonical monster-specific attacks

Postby Xaser » Fri Mar 29, 2019 9:28 am

Yeah, I'm really not following. Everyone's still focusing on the "use missiletype" non-sequitur -- that's not at all what was actually being suggested in the title/OP.

If, say, A_TroopAttack is virtual, and you do the following two things:
  • Extend HellKnight and make him fire a salvo of imp fireballs with A_TroopAttack
  • Extend DoomImp and override A_TroopAttack to make it fire a rocket

...then the Hellknight fires Imp balls while the Imp shoots rockets, just as expected. A_TroopAttack behaves exactly as it always has, except in the case where it's explicitly told to do something different in a derived class.
User avatar
Xaser
anarchivist
 
 
 
Joined: 20 Jul 2003

Re: Virtualize all canonical monster-specific attacks

Postby Graf Zahl » Fri Mar 29, 2019 9:52 am

Action functions cannot be virtual which kills this right away.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Virtualize all canonical monster-specific attacks

Postby gramps » Fri Mar 29, 2019 11:04 am

How is that really useful anyway? "Extend DoomImp and override A_TroopAttack to make it fire a rocket" still involves copying the entire A_TroopAttack function into your extended DoomImp.

Is this just about not wanting to copy part of the state block to call a renamed A_TroopAttack2? That doesn't seem particularly important if you're already copying that entire function anyway.

Granted, there's not much going on in A_TroopAttack, but by the time you're copying the mancubus attack or something, copying a few lines of a state block is basically nothing in comparison. Manc's missile state is 8 LOC, but the functions you'd need to copy are about 70 LOC. Archvile's another nasty one, 7 LOC in missile state, about 50 LOC of copied functions.

Point being, avoiding copying a small part of the state block seems like going after the wrong thing here. Something interesting to look at is the pain elemental. The function called from his state block is very short, because he's organized in a more granular way; the A_PainAttack function from the state block actually delegates all the work to a much longer A_PainShootSkull function. But you don't need to copy that, because it's not called directly from the state block. If some of the other monsters were organized like that, that would cut down a whole lot on the need for cargo-coding copypasta. Maybe something like that would be a more reasonable idea for a feature suggestion?
gramps
 
Joined: 18 Oct 2018

Re: Virtualize all canonical monster-specific attacks

Postby Graf Zahl » Fri Mar 29, 2019 12:52 pm

You are missing something here: All A_FatShot functions and A_PainAttack have a parameter for the missile type, and so does A_VileTarget for the flame. A_VileAttack is even fully configurable. The simple ones like A_TroopAttack are not worth it because they are far too simple and can be done with generic functions instead. If it wasn't for Dehacked backwards compatibility they would have been deleted long, long ago.
User avatar
Graf Zahl
Lead GZDoom Developer
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: Virtualize all canonical monster-specific attacks

Postby gramps » Fri Mar 29, 2019 1:06 pm

Ahh, I was totally missing that!

So the entire issue here is wanting to avoid copying ~8 LOC in a state block, then? That doesn't seem like too much of a burden at all, honestly.

I mean, I guess the imp could get that parameter too, even though it's simple, just so we don't have to look up the the details of its attack to be sure we set it up properly. But that'd be an issue for a separate ticket.
gramps
 
Joined: 18 Oct 2018

Re: Virtualize all canonical monster-specific attacks

Postby Xaser » Fri Mar 29, 2019 3:36 pm

Graf Zahl wrote:Action functions cannot be virtual which kills this right away.

True, though interestingly, the legacy monster attack functions (or at least the few I poked around at, like A_TroopAttack) don't seem to be marked "action" anymore -- I guess it's not needed since none of them use the invoker pointer.

gramps wrote:How is that really useful anyway? "Extend DoomImp and override A_TroopAttack to make it fire a rocket" still involves copying the entire A_TroopAttack function into your extended DoomImp.

The vast majority of use cases, I imagine, are covered by calling one of the generalized attack functions, like so:
Code: Select allExpand view
class RocketImp : DoomImp replaces DoomImp
{
    override void A_TroopAttack()
    {
        A_SpawnProjectile("Rocket");
    }
}


The old attack functions do have all their logic inline (probably a "why fix what's not broken?" kinda thing), but pretty much all of them have a generalized version somewhere -- e.g. if you want the Imp's "melee if the player is too close" behavior, there's A_CustomComboAttack, and so forth.
User avatar
Xaser
anarchivist
 
 
 
Joined: 20 Jul 2003

PreviousNext

Return to Closed Feature Suggestions

Who is online

Users browsing this forum: No registered users and 1 guest