HomeWildfire Games

Allow entities to be affected by Status Effects from ranged attacks.

Description

Allow entities to be affected by Status Effects from ranged attacks.

This implements a status effects receiver component (in a similar fashion to the damage receiver component). The plan is to further extend this component notably to handle graphical indication of status effects, and a variety of other effects.

Currently implemented: ranged attacks can inflict status effects that reduce HP over time.
This can be resisted by armour.

No units currently utilise this in-game but with proper graphics support that could be changed.

Patch By: Mate-86

Reviewed By: wraitii

References #1912

Differential Revision: https://code.wildfiregames.com/D1252

Details

Event Timeline

elexis raised a concern with this commit.Jun 24 2019, 6:49 PM
elexis added a subscriber: elexis.

StatusEffects name is problematic I'm afraid.

The code shows that it is exclusively about damage, so the name pretends to be a jack of all trades but it is a one-trick-pony.
Another case where "status effects" were intended to be added is in D1718 (health changed via timer).
We have to accept that not every component can do everything, the name must be restricted to what it actually does, can do, should do.
The name of the ticket:

Continuous damage after hit detection

was much more accurate.

The name was proposed in https://code.wildfiregames.com/D1252#77065

So you are going for a "Poisoned" damage, right? Regardless of whether the attacker dies or not? Basically you are adding "status modifiers" to a unit.

It was noticed here https://code.wildfiregames.com/D1252#78363

I didn't read above discussion, but the component name "Status" seems undescriptive. Entities have Owner status, Capture status, Health status, territory status, resource status, so what is Status?

Reply was:

The purpose of this component is to allow entities to suffer from status effects, a very standard concept. We could rename it to "StatusEffects", though I feared that would imply this was the bit giving status effects when it's actually receiving them.

Linking to https://en.wikipedia.org/wiki/Status_effect.

But that wiki page says

The term status effect can be applied both to changes that provide a character an advantage (increased attributes, defensive barriers, regeneration), and those that hinder the character (decreased attributes, incapacitation, degeneration).

So it's more related to what auras and technologies do, those are actually general.
If some component claims to be that general, it should be.
And if that component can achieve general status effects, it should *not* hardcode component references and come with some logic of every component, but work as generically as Tech and Aura works.

Further the wiki page states:

A status effect in the abstract is a persistent consequence of a certain in-game event or action, and as such innumerable variants exist across the gaming field. Status effects may result from one character performing a certain type of attack on another. Players may acquire status effects by consuming items, casting spells on themselves or each other, activating devices in the world, interacting with NPCs, or remaining in a particular location. Meeting certain criteria may result in the character acquiring a condition, which can have a status effect associated with it; for example: if their hunger level is high they may acquire a 'starving' condition, which produces a status effect that reduces their health regeneration.

The persistence and per-entity character of the 'statuseffect' is something that Auras Techs don't have, so it seems the intended stuff can't be achieved with that (more obvious when looking at the objective of the ticket).

I believe that if you really want to plan such a generic component that can persist per-entity 'statuseffects', then this should be well planned and not only work for this one case.

Converse:
There is also the question whether the objective of the ticket (dealing damage after the projectile hit) makes sense to have in such a generic component rather than somewhere in the attack/damage receiver code.
If the use case is to fire poison or flaming arrows that create damage beyond the time of impact, then it would be reasonable to not only have a "statuseffect" (i.e. modifying a property of a component over time), but possibly also to have some attack/damge/projectile specific code, for example handling additional VisualActors. No?

If you had long secret plans how to develop this component, at least they are not visible. What I see are about 10 sentences stating that this should be a one-fits-all while still providing only repeated-damage and removing the ability to specialize on that.

The previous version of the code looked much cleaner to me until there is a one-fits-all that actually does fit all without hardcoding all:
https://code.wildfiregames.com/D1252?id=7918

Sorry that I didn't follow this Mate-86, your patch is good, it should just not claim the StatusEffect name until there is good qualification for that IMO.
If you want to keep the StatusEffect name, you can also try to go forward and demonstrate that this code is corret for the generic-purpose use case, for example D1718, as long as planning is more thorough than what was presented in those two revision proposals by the two reviewers with regards to generic StatusEffects. (Actually I can't recommend working on that currently)

/ps/trunk/binaries/data/mods/public/simulation/components/Attack.js
7

Effects like poisioning or burning a unit

commonly called Damage, not StatusEffect

/ps/trunk/binaries/data/mods/public/simulation/components/Damage.js
143

The word "Inflict" implies damage, but not to positive or neutral 'effects'

/ps/trunk/binaries/data/mods/public/simulation/components/StatusEffectsReceiver.js
1

As far as I see this is the DamageReceiver, i.e. Armour if it is supposed to have different implementations per DamageReceiver, or the part that shall remain the same for all DamageReceivers would go to the attack-dealing component (Attack, perhaps CCmpProjectileManager)

56

=> Effect = Damage

This commit now has outstanding concerns.Jun 24 2019, 6:49 PM

I guess you don't understand the concept of an iteration or an MVP.

wraitii added inline comments.Jun 24 2019, 7:38 PM
/ps/trunk/binaries/data/mods/public/simulation/components/Attack.js
7

You appear to have never played Pokémon.

elexis removed an auditor: elexis.Fri, Sep 6, 10:36 AM
This commit no longer requires audit.Fri, Sep 6, 10:36 AM