Virtualize all canonical monster-specific attacks
Moderator: GZDoom Developers
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: Virtualize all canonical monster-specific attacks
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.
Re: Virtualize all canonical monster-specific attacks
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.
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.
Re: Virtualize all canonical monster-specific attacks
Going back to the OP (key word "virtualize"), what about DEHACKED support is preventing marking these functions as `virtual`, exactly?
Re: Virtualize all canonical monster-specific attacks
Probably not. MissileType still works.gramps wrote:Although, I suppose deprecating missiletype also broke old stuff, didn't it?
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.
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: Virtualize all canonical monster-specific attacks
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.Xaser wrote:Going back to the OP (key word "virtualize"), what about DEHACKED support is preventing marking these functions as `virtual`, exactly?
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.
Re: Virtualize all canonical monster-specific attacks
@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.
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.
Re: Virtualize all canonical monster-specific attacks
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.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.
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.
Re: Virtualize all canonical monster-specific attacks
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.
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.
- 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: Virtualize all canonical monster-specific attacks
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.My reading of the OP suggested that it did used to work like that
Re: Virtualize all canonical monster-specific attacks
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:
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
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: Virtualize all canonical monster-specific attacks
Action functions cannot be virtual which kills this right away.
Re: Virtualize all canonical monster-specific attacks
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?
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?
- Graf Zahl
- Lead GZDoom+Raze Developer
- Posts: 49067
- Joined: Sat Jul 19, 2003 10:19 am
- Location: Germany
Re: Virtualize all canonical monster-specific attacks
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.
Re: Virtualize all canonical monster-specific attacks
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.
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.
Re: Virtualize all canonical monster-specific attacks
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.Graf Zahl wrote:Action functions cannot be virtual which kills this right away.
The vast majority of use cases, I imagine, are covered by calling one of the generalized attack functions, like so: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.
Code: Select all
class RocketImp : DoomImp replaces DoomImp
{
override void A_TroopAttack()
{
A_SpawnProjectile("Rocket");
}
}