Agreed. In fact, what I've done is:
- Subclassed CharacterHealth as AdvancedCharacterHealth. In AdvancedCharacterHealth, as you point out, if I simply wanted straight damage multipliers, I would have just overriden Damage(), specifically the 8 argument version that actually did the work, altering the damage and invoking the base.Damage(). Whenever I create a new NPC, I have to swap out the pre-added CharacterHealth for AdvancedCharacterHealth. However, as I'm looking at adding damage *types* as well, I had to introduce a 9 argument version and then alter where it gets called, which leads me to...
- Subclassed Projectile as SpellProjectile. Here, I override OnCollision(), copying the code (unfortunately) all so that I could get at the call to Health.Damage(). Of course, I'm also now calling hitGameObject.GetCachedParentComponent<AdvancedCharacterHealth>() in order to fetch the AdvancedCharacterHealth. If I were extra careful, I suppose I would check for Health as a fallback, but I don't plan to leave the stock CharacterHealth on any of my targetable creations.
- Subclassed UsableItem as SpellItem, copying ShootableWeapon as a base. I took the lazy approach here. I thought there might be a lot I would have to do here and didn't want to blow away ShootableWeapon to do it. At present, all of my "weapons" are planned to be spells, so I'm fine with this. And of course, I had to alter HitscanFire() in order to make the same change I made in SpellProjectile.OnCollision() to switch to AdvancedCharacterHealth.
- And I've had to modify ItemBuilder to make SpellItem available within your system.
An aside: This, by the way, is one of those personal coder-OCD things that annoyed me... a lot of the code between Projectile.OnCollision() and ShootableWeapon.HitscanFire() is similar in terms of collecting information on the target and applying the damage. It might have been nice to have a DamageManager class that would:
- Have a Damage() method (or suite of Damage(...) methods as currently exists) that accepts the same information and applies damage to the target, from the source, based on provided parameters.
- Add an additional Damage() method that accepts an Object[] userParms or somesuch, to allow passage of arbitrary data into the Damage call, so that...
- Items could be granted an 'additionalDamageParameters' Object[] (again, or something) attribute, which could be populated at design time with anything the developer wanted (in my case, perhaps a ScriptableObject containing the damage type and a list of conditions to apply), which would be passed into that Damage() method and subsequently...
- A developer could listen to a ModifyDamage event, use any userParms data, alter the damage via an out parameter (perhaps even altering the damage type if that were added), as well as...
- The same developer could listen to a PostDamage event, using the same userParms data to apply conditions or any other post-hit effects that aren't "Effects"
In the case where no such additional damage parameters are defined (i.e. the attribute is a null or zero length array), the standard Damage call (i.e. one that doesn't pass the userParms argument) would be made and aside from any overhead related to checking for event listeners, I wouldn't expect the result to take any longer than normal. In the case where more work is done (i.e. there *are* some parameters defined and listeners to use them), that becomes the purview of the developer to optimize.
In this scenario, I as a developer and user of your framework, can expand on the entire damage system without needing modify core pieces of your framework to do so, also allowing me to freely update as you release fixes and the like without having to worry that I'm going to need to very carefully merge such changes into code I've tweaked.