Add selling, undeploying and harvesting conditions to DiscardOn

[DeathFishAtEase]

• Similar..., but ...
  • DiscardOn...
    • selling: Discard when the building to which the effect is attached is sold.
    • undeploying: Discard when the building to which the effect is attached performs undeploy.
    • harvesting: Discard when the object the effect is attached is harvesting ore.

[AttachEffectTypes]
+=HarvestChecker
+=NormalChecker
[WeaponTypes]
+=NormalCheckWp
+=HarvestCheckWp
[Wearheads]
+=NormalCheckWh
+=HarvestCheckWh
[HARV]
AttachEffect.AttachTypes=HarvestChecker
[HarvestChecker]
Duration=-1
DiscardOn=harvesting
ExpireWeapon=NormalCheckWp
ExpireWeapon.TriggerOn=discard
[NormalCheckWp]
Projectile=InvisibleAll
Warhead=NormalCheckWh
[NormalCheckWh]
AttachEffect.AttachTypes=NormalChecker
Convert.From=HARV
Convert.To=CMIN
[NormalChecker]
Duration=-1
DiscardOn=move
ExpireWeapon=HarvestCheckWp
ExpireWeapon.TriggerOn=discard
[HarvestCheckWp]
Projectile=InvisibleAll
Warhead=HarvestCheckWh
[HarvestCheckWh]
AttachEffect.AttachTypes=HarvestChecker
Convert.From=CMIN
Convert.To=HARV
[CMIN]
Locomotor=Drive
Teleporter=false

[github-actions]

Nightly build for this pull request:

• compiled-dll-b0ac2adc9c3ed8f6df54960e5daf8f71c8953fca.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.

[phoboscn-bot]

To Chinese users:
This pull request has been mentioned on Phobos CN. There might be relevant details there:

致中文用户：
此拉取请求已在 Phobos CN 上被提及。那里可能有相关详细信息：

https://www.phoboscn.top/t/topic/583/1

[Coronia]

While the logic is fine by itself, I'm wondering how should DiscardOn be proceeded further. There'll certainly be more and more conditions that could be used as a discard check, but bloating the enum might not be a long-lasting solution: you'll just get more enum keywords to be filled and handled, and these conditions have to be checked no matter if they're used or not

A similar issue was happened in AutoDeath before and I was trying to resolved it by a generic condition handler, but that's a failure either since checking lots of conditions per frame can result in performance drop. Not sure if there could be a better design for such features that can be triggered by multiple conditions

[DeathFishAtEase]

This is indeed a problem that requires careful consideration, especially since conditions like move and inrange will also need to account for combinations in the future. I considered encapsulating each condition as a reusable class, instantiating it only when the AE has configured that condition, but that only increases code fragmentation and brings no substantial performance improvement or maintainability gain. In fact, many approaches are not better than the current handling. :(

[TaranDahl]

While the logic is fine by itself, I'm wondering how should DiscardOn be proceeded further. There'll certainly be more and more conditions that could be used as a discard check, but bloating the enum might not be a long-lasting solution: you'll just get more enum keywords to be filled and handled, and these conditions have to be checked no matter if they're used or not

A similar issue was happened in AutoDeath before and I was trying to resolved it by a generic condition handler, but that's a failure either since checking lots of conditions per frame can result in performance drop. Not sure if there could be a better design for such features that can be triggered by multiple conditions

Event system , which was drafted and dead

[Metadorius]

I think you folks really need to cooperate and start generalizing the features, not fearing learning new stuff.

We can't implement every single idea in existence if we do them as one-offs without reusability or generalization. We need to be providing building bricks, not an endless amount of houses.

[DeathFishAtEase]

Tested by:

• Selling & Undeploying (regular): https://www.phoboscn.top/t/topic/583/13?u=noble_fish

  • Selling:
    
  • Undeploying:
    

• Selling (AutoDeath.Behavior=sell): https://www.phoboscn.top/t/topic/583/29?u=noble_fish
  

• Selling & Undeploying (map trigger actions): https://www.phoboscn.top/t/topic/583/30?u=noble_fish

  • Action#60:
    
    In map file:

    [Tags]
    01000005=0,Sellling 1,01000004
    [Triggers]
    01000004=Americans,<none>,Sellling,0,1,1,1,0
    [Events]
    01000004=1,13,0,1
    [Actions]
    01000004=1,60,0,0,0,0,0,0,A

  • Action#511
    
    In map file:

    [Tags]
    01000005=0,Sellling 1,01000004
    [Triggers]
    01000004=Americans,<none>,Sellling,0,1,1,1,0
    [Events]
    01000004=1,13,0,1
    [Actions]
    01000004=1,511,10,GACNST,0,0,0,0,CU

• Harvesting: https://www.phoboscn.top/t/topic/583/10?u=noble_fish
  
  In rulesmd.ini:

  [AttachEffectTypes]
  +=HarvestChecker
  +=NormalChecker
  [WeaponTypes]
  +=NormalCheckWp
  +=HarvestCheckWp
  [Wearheads]
  +=NormalCheckWh
  +=HarvestCheckWh
  [HARV]
  AttachEffect.AttachTypes=HarvestChecker
  [HarvestChecker]
  Duration=-1
  DiscardOn=harvesting
  ExpireWeapon=NormalCheckWp
  ExpireWeapon.TriggerOn=discard
  [NormalCheckWp]
  Projectile=InvisibleAll
  Warhead=NormalCheckWh
  [NormalCheckWh]
  AttachEffect.AttachTypes=NormalChecker
  Convert.From=HARV
  Convert.To=CMIN
  [NormalChecker]
  Duration=-1
  DiscardOn=move
  ExpireWeapon=HarvestCheckWp
  ExpireWeapon.TriggerOn=discard
  [HarvestCheckWp]
  Projectile=InvisibleAll
  Warhead=HarvestCheckWh
  [HarvestCheckWh]
  AttachEffect.AttachTypes=HarvestChecker
  Convert.From=CMIN
  Convert.To=HARV
  [CMIN]
  Locomotor=Drive
  Teleporter=false

[TaranDahl]

@Metadorius So what is your decision?

[Starkku]

I think you folks really need to cooperate and start generalizing the features, not fearing learning new stuff.

We can't implement every single idea in existence if we do them as one-offs without reusability or generalization. We need to be providing building bricks, not an endless amount of houses.

My suggestion for this is as follows, uncertain if it is close to what you had in mind or not but whatever.

• Create new class/struct called ConditionType(Class/Struct) or TechnoConditionType(Class/Struct) since it is more or less specifically evaluating various techno variables. Generalizing it fully for other object types is probably doable but uncertain how necessary or useful that is.
  • This approach instead of enum because a wrapper class/struct for bools is infinitely expandable unlike 32-bit integer enum.
• Replace bespoke condition handling in AE DiscardOn and probably AutoDeath as well with this class/struct and a helper function that evaluates whether or not the conditions are fulfilled.
• Explore possibility to use overloading and/or templating to make it so that the same helper function(s) can be used for both short-circuit evaluation (return at first opportunity) or check all conditions etc. with minimal code duplication or unnecessary branching.

I am willing to do the grunt work for this if we can agree on something even remotely approaching an actionable plan first.