[Minor] Fix number of issues with health threshold checks

[Starkku]

• Warhead AffectsBelow/AbovePercent check is no longer bypassed in BulletClass::Logics() if bullet target and firer are same object.
• Removed incorrect CanTarget.Max/MinHealth check on firing techno itself in force weapon selection code.
• Warhead AffectsAbovePercent will now by default bypass the check if target has 0 health left. Can be overridden by setting AffectsAbovePercent.IncludeZero to false.
• Weapon CanTarget.MinHealth is no longer inclusive to be in line with Warhead AffectsAbovePercent.
• Weapon CanTarget.MinHealth will now by default bypass the check if target has 0 health left. Can be overridden by setting CanTarget.MinHealth.IncludeZero to false.
• None of the health threshold checks are now restricted to TechnoTypes only where appropriate (may be subject to further changes depending on if issues crop up with checking projectile/tree healths).
• Added missing Crit.AffectsAbovePercent to be in line with other checks.
• Unified the health threshold checking code.

Some considerations.
• Unsure if CanTarget.MinHealth.IncludeZero is necessary, it is my understanding that the fire error checks do not hard disallow targeting objects with zero health but is covering this edge case necessary? Crit is by design not able to occur on technos with zero health so that is why equivalent was not added for it.
• Uncertain if problems could occur with the health thresholds being applied on trees and projectiles in case of InterceptorWeapon and if reverting that part or adding more toggles would be necessary.

Fixes #1743

[github-actions]

Nightly build for this pull request:

• compiled-dll-52e6585de10d24ddbc259a9ab8636fdf27c6968e.zip
  These artifacts will expire in 90 days and will not be available for download after that time.

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

[CrimRecya]

The main part looks fine. But personally, I have some small questions.
Would it be better to place WeaponTypeExt::ExtData::IsHealthInThreshold‘s sanity check outside like others? (Then we can use abstract_cast<ObjectClass*, true>(pTarget) inside)
And regarding IncludeZero, I think negative numbers should be allowed? Then only need to provide a prompt in the document, without adding a new key.

[Metadorius]

And regarding IncludeZero, I think negative numbers should be allowed? Then only need to provide a prompt in the document, without adding a new key.

Not sure on this, why?

[Starkku]

The main part looks fine. But personally, I have some small questions. Would it be better to place WeaponTypeExt::ExtData::IsHealthInThreshold‘s sanity check outside like others? (Then we can use abstract_cast<ObjectClass*, true>(pTarget) inside) And regarding IncludeZero, I think negative numbers should be allowed? Then only need to provide a prompt in the document, without adding a new key.

When it comes to weapon targeting / selection stuff, the target is pretty much always AbstractClass pointer and I figured it is just cleaner to do the check (for ObjectClass) inside the WeaponExt-version of the health threshold function than add it to bunch of different locations.

---

Negative values already work. The main issue is that introduction of the health checks changes the default behaviour of.. well everything because zero health-objects were previously not excluded by default from warhead detonations etc. and now they are. Warranting modder config migration here is not feasible.

There are different ways to solve this issue including making the lower bound check inclusive so zero health is included if the minimum threshold is 0. I would not be against it (it would effectively be possible to make the check exclusive by using a very small value instead) and it would make certain degree of sense given that the upper bound check is but this didn't seem to be a viable option for Kerbiter when we discussed this matter.

[Coronia]

I agree that 0 health is a rare edgy case that shouldn't even be considered in actual modding, so IncludeZero could be removed and just let it always be included or not, which should be written in the doc tho

[CrimRecya]

I have reviewed the discussion on DC, so in fact, IncludeZero is designed to handle the situation after the death of the target object. In this case, I think there are some issues.
For example, in ExtraWarheads, the target is reacquired every time it detonates. When the target dies, AnnounceInvalidPointer is called, resulting in an empty Target. In this case, the target obtained afterwards is empty and can no longer participate in subsequent checks. Therefore, some special handling like "return true;" -> "return this->CanTarget_MinHealth_IncludeZero;" is needed in this situation. (The external of the function may also require some processing, but I haven't looked closely yet)

[Metadorius]

I agree that 0 health is a rare edgy case that shouldn't even be considered in actual modding, so IncludeZero could be removed and just let it always be included or not, which should be written in the doc tho

I mean, objects can have zero HP when dying, and we don't really know if there's any potential situation where that state should have different behavior, so it's best to leave it that way. Besides, it's a special case, so a special tag for special case makes sense.

[fagonghaiwo]

👀

[Metadorius]

@fagonghaiwo can you elaborate what is being shown on the screenshot?