Reflective Flags

[Major Cooke]

https://github.com/rheit/zdoom/pull/183

This adds a number of flags that deal with reflectivity.

(All of these require the REFLECTIVE flag to work.

• THRUREFLECT - Projectiles are reflected, but they pass through the enemy instead of bouncing back. The velocity is not lost. Takes precedence over all others reflection modifying flags.
• MIRRORREFLECT - Projectiles perform a direct 180 turn-around.
• AIMREFLECT - Projectiles are aimed straight back in the direction of the original shooter. The speed of the projectile is unmodified.

[D2JK]

Maybe a dumb question, but are the "HIT" flags related to Ed the Bat's "Set victim as tracer?" thread? I was just reading it up, and wondered about the same thing.

If so, brilliant!

[Major Cooke]

Yep.

[Nightfall]

This is never, ever, EVER going to work if it's implemented as a hack. Frankly, the whole thing is in despeate need of being cleaned up, unfortunately that will break lots of existing mods.
Really, the only solution I see is to add a few new pointers with completely unambiguous semantics and NO WRITE ACCESS from DECORATE at all!

Whatever happened to this ideology?

[Major Cooke]

This is never, ever, EVER going to work if it's implemented as a hack.

Nightfall:

1. I'm not Graf.
2. I don't view this as a hack. I view it as the simplest solution to that suggestion. What Graf was talking about (correct me if I'm wrong Graf) was the whole pointers and introducing new ones that involved working solely for the purpose of projectile deaths.

[Fishytza]

I don't view it as a hack either. Although Graf did say "NO WRITE ACCESS from DECORATE at all" which I assume includes what you just implemented. (Actually, It's probably not implemented yet at the moment of typing.)

I'm not complaining, just a little amused that some things the devs rejected for various reasons such as "Not Needed, Too specific/niche, supports modder laziness" etc, you've managed to implement without much objection.
Maybe it's just me though.

Also, I think 'ideology' is a bit too strong if not outright dangerous word to use.
'Standard' or 'design philosophy' seem like a better description IMO.

EDIT: I was just wondering, could you tell me what's the actual usefulness of THRUREFLECT? What's the point of a projectile that passes through and changes its target field to the thing it just past through? Seems kind of odd.

[Gez]

The difference is that what Graf was talking about was a suggestion to make all projectiles unconditionally store the actor they hit in a pointer. This is not the case here, because it's conditional, depending on having one of these flags. It will have no gameplay effects on any existing projectile. What Graf was saying was that unconditional storing would have to be in a new pointer made specifically for that, precisely so as to have no gameplay effect on any existing projectile.

The "write access from DECORATE" thing was that this new pointer would be put outside the reach of functions such as [wiki]A_RearrangePointers[/wiki].

I'm not complaining, just a little amused that some things the devs rejected for various reasons such as "Not Needed, Too specific/niche, supports modder laziness" etc, you've managed to implement without much objection.

Writing the code goes a long way towards making unneeded, niche-specific suggestions get accepted after all.

[Fishytza]

Writing the code goes a long way towards making unneeded, niche-specific suggestions get accepted after all.

Apparently. This is why I find it amusing.

[Graf Zahl]

What's amusing? If someone who NEEDS the niche feature and goes out spending his own time implementing and testing it is a whole lot different than having a dev spend their time on it - someone who doesn't have a major interest in this feature.

That said, I think that some cleanup will be in order after this gets added. There's too many reflection-specific things spread out through the main flags now, they better get consolidated into their own ReflectionFlags field, just like it was done for bouncing flags.

[Fishytza]

What's amusing?

Well, I always thought that one had to argue with the devs (or other 'influential users' who the devs respect) about whether a feature gets to be implemented in the first place. Even if the feature suggester was the one coding-in the feature.

(I remember something about being careful about what feature goes in, making sure it's not redundant and unnecessary because "it could be done much better with scripting", i.e. DoomScript/ZScript.)

But I guess I was totally wrong about that.

A bit tired at this point, but I think I'll say it anyway.
@Major Cooke
Maybe I'm wrong about this but you claim that all the new 'reflective' flags require REFLECTIVE to work.
However, i've noticed that apparently REFLECTIVE isn't needed for THRUREFLECT?

---

p_map.cpp, PIT_CheckThing:

Code: Select all

        if ((thing->flags7 & MF7_THRUREFLECT) && (tm.thing->flags & MF_MISSILE))
        {
            if (tm.thing->flags2 & MF2_SEEKERMISSILE)
            {
                tm.thing->tracer = tm.thing->target;
            }
            tm.thing->target = thing;
            return true;
        }

...and P_BounceActor:

Code: Select all

    if (BlockingMobj && (BlockingMobj->flags7 & MF7_THRUREFLECT)) return true;

...and P_PushUp:

Code: Select all

        if ((thing->flags & MF_MISSILE) && (intersect->flags7 & MF7_THRUREFLECT))
            continue;

It seems to me that missiles will pass through and change their target even if the actor that is being past through lacks REFLECTIVE.
Again, kinda tired. Might have missed something. Apologies in advance.

[Graf Zahl]

You clearly misunderstand our motivations completely. Code submissions can and will be rejected if they are considered harmful to future stability or require constant maintenance in the future. What Major Cooke is doing here is far from both.

[Fishytza]

So to clarify.

Stability, functionality, and minimum maintenance are far more important to you than preventing redundancy and bloating the code?

[Gez]

Redundancy is mostly incompatible with minimum maintenance. When you have redundant code, it means you have to change two parts of code (or more, depending on level of redundancy) instead of just one when there is some maintenance to do there.

Stability, functionality, and minimum maintenance seem to be good priorities. Don't forget that one man's bloat is another man's dream feature, and vice-versa.

[Fishytza]

To give examples:

It is more important that:
Functions such as A_DamageTarget, A_DamageTracer, A_DamageSelf are reasonable stable and bug free.
(to be clear, I'm not saying they should be removed since, at this point, it's far too late.)

Rather than:
Seriously considering if the aforementioned functions are in fact redundant/code-bloat and are therefore unnecessary since a generic A_Damage function would be a much better candidate. After all, maintaining one is easier than maintaining three.
(A_DamageMaster in an exception because it's been around for a long time and must be maintained for backwards compatibility.)

I'm just trying (and apparently failing) to understand the policy about what sort of suggested features and/or code submissions are acceptable to implement.

[Gez]

Yeah I'm not sure either why there are all these variants, but it's not really code redundancy. Behind the hood, they're all the same function.

---

Code: Select all

//===========================================================================
//
// Common A_Damage handler
//
// A_Damage* (int amount, str damagetype, int flags)
// Damages the specified actor by the specified amount. Negative values heal.
//
//===========================================================================

enum DMSS
{
	DMSS_FOILINVUL			= 1,
	DMSS_AFFECTARMOR		= 2,
	DMSS_KILL				= 4,
	DMSS_NOFACTOR			= 8,
	DMSS_FOILBUDDHA			= 16,
	DMSS_NOPROTECT			= 32,
};

static void DoDamage(AActor *dmgtarget, AActor *self, int amount, FName DamageType, int flags)
{
	int dmgFlags = 0;
	if (flags & DMSS_FOILINVUL)
		dmgFlags += DMG_FOILINVUL;
	if (flags & DMSS_FOILBUDDHA)
		dmgFlags += DMG_FOILBUDDHA;
	if ((flags & DMSS_KILL) || (flags & DMSS_NOFACTOR)) //Kill implies NoFactor
		dmgFlags += DMG_NO_FACTOR;
	if (!(flags & DMSS_AFFECTARMOR) || (flags & DMSS_KILL)) //Kill overrides AffectArmor
		dmgFlags += DMG_NO_ARMOR;
	if (flags & DMSS_KILL) //Kill adds the value of the damage done to it. Allows for more controlled extreme death types.
		amount += dmgtarget->health;
	if (flags & DMSS_NOPROTECT) //Ignore PowerProtection.
		dmgFlags += DMG_NO_PROTECT;

	if (amount > 0)
		P_DamageMobj(dmgtarget, self, self, amount, DamageType, dmgFlags); //Should wind up passing them through just fine.

	else if (amount < 0)
	{
		amount = -amount;
		P_GiveBody(dmgtarget, amount);
	}
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageSelf)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	DoDamage(self, self, amount, DamageType, flags);
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageTarget)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	if (self->target != NULL) DoDamage(self->target, self, amount, DamageType, flags);
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageTracer)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	if (self->tracer != NULL) DoDamage(self->tracer, self, amount, DamageType, flags);
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageMaster)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	if (self->master != NULL) DoDamage(self->master, self, amount, DamageType, flags);
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageChildren)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	TThinkerIterator<AActor> it;
	AActor * mo;

	while ( (mo = it.Next()) )
	{
		if (mo->master == self) DoDamage(mo, self, amount, DamageType, flags);
	}
}

//===========================================================================
//
//
//
//===========================================================================
DEFINE_ACTION_FUNCTION_PARAMS(AActor, A_DamageSiblings)
{
	ACTION_PARAM_START(3);
	ACTION_PARAM_INT(amount, 0);
	ACTION_PARAM_NAME(DamageType, 1);
	ACTION_PARAM_INT(flags, 2);

	TThinkerIterator<AActor> it;
	AActor * mo;

	if (self->master != NULL)
	{
		while ((mo = it.Next()))
		{
			if (mo->master == self->master && mo != self) DoDamage(mo, self, amount, DamageType, flags);
		}
	}
}

See? Only _Siblings and _Children actually have some logic on their own, so as to iterate over the multiple valid targets. Having just one A_Damage function wouldn't really reduce actual code bloat.