ACE3 icon indicating copy to clipboard operation
ACE3 copied to clipboard

Frag - Rewrite (event handlers and dev functions)

Open lambdatiger opened this issue 1 year ago • 5 comments

This is a fork and revert of some of the features added in https://github.com/acemod/ACE3/pull/9728. Specifically, the spalling and fragmentation simulation are the same as the original with minor bug fixes, but the event handlers, dev functions, and some other parts of the framework are newer.

When merged, this pull request will:

  • Removes old system of tracking rounds by using a per frame event handler and implements BI projectile event handlers.
    • Removed spalling requiring multiple object "hitPart" event handlers to be added and removed.
  • Separates spall and frag config parameters into separate files.
  • More completely defines vanilla ammunition frag parameters.
  • Replaces debug functions that used per frame event handlers with those found in https://github.com/acemod/ACE3/pull/9728
    • Small changes to these from the other PR include the removal of reference to fragCount config variable.
  • Moves repeated config checks and common parameters to separate functions leveraging hashmaps.
  • Since blacklisting is local, we can remove the event handlers to avoid having to check projectile variables multiple times.
  • Cleaned some magic numbers out of both doSpall and frago functions.
  • Removed unused variables and lines of code with no effects in frago.
  • Minor changes from https://github.com/acemod/ACE3/pull/9728.
    • Implemented better method for getting list of targets in frago.
    • Checking to make sure fragments aren't targeted at playable logic in frago.
    • Minor calculation optimizations in frago and doSpall.
  • Fragments and spall now inherit shotParents parameters.
    • ~~If someone would like the first parameter (vehicle) of setShotParents/getShotParents to be preserved, let me know.~~ Dedmen does a good job describing the uses for the first parameter (owner, not vehicle) on the ACE discord here.
  • Updated documentation to properly note that config value ace_frag_enabled doesn't do anything.

As always, I appreciate feedback and criticism!

Todo

  • [x] MP testing - small scale (2x player + dedicated)
  • [x] Re-review when more awake

IMPORTANT

  • [x] If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • [x] Development Guidelines are read, understood and applied.
  • [x] Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

lambdatiger avatar Jul 30 '24 04:07 lambdatiger

I believe I've found everything I am not blind to after looking at and/or writing it. I'll put it up for review tomorrow unless there's any reason provided not to. Thanks as always for your help johnb432.

lambdatiger avatar Jul 30 '24 21:07 lambdatiger

All changes in shouldFrag for vanilla ammo
[BombCluster_03_Ammo_F] before:true now:false
[BombCluster_02_Cap_Ammo_F] before:true now:false
[M_Mo_230mm_AT_LG] before:true now:false
[HelicopterExploSmall] before:true now:false
[M_Air_AA_MI02] before:false now:true
[HelicopterExploBig] before:true now:false
[UnderwaterMine_Range_Ammo] before:true now:false
[ammo_Missile_mim145] before:true now:false
[BombCluster_01_Ammo_F] before:true now:false
[G_20mm_HE] before:true now:false
[UnderwaterMineAB_Range_Ammo] before:true now:false
[BombDemine_01_Ammo_F] before:true now:false
[BombCluster_02_UXO_deploy] before:true now:false
[BombDemine_01_SubAmmo_F] before:true now:false
[ACE_SLAMDirectionalMine_Timer_Ammo] before:true now:false
[ClaymoreDirectionalMine_Remote_Ammo_Scripted] before:true now:false
[G_40mm_HEDP_Bullet] before:true now:false
[M_Titan_AA_long] before:false now:true
[SLAMDirectionalMine_Wire_Ammo] before:true now:false
[M_Mo_82mm_AT] before:true now:false
[ammo_Missile_s750] before:true now:false
[BombCluster_01_UXO_deploy] before:true now:false
[G_40mm_HE] before:true now:false
[BombCluster_03_UXO_deploy] before:true now:false
[DemoCharge_Remote_Ammo_Scripted] before:true now:false
[ACE_G_40mm_HE] before:true now:false
[Drone_explosive_ammo_dummy] before:true now:false
[Drone_explosive_ammo] before:true now:false
[R_80mm_HE] before:false now:true
[M_Zephyr_air] before:false now:true
[M_Zephyr_Mi06] before:false now:true
[ClaymoreDirectionalMine_Remote_Ammo] before:true now:false
[M_Zephyr] before:false now:true
[ammo_Missile_LongRangeAABase] before:true now:false
[R_60mm_HE] before:false now:true
[SatchelCharge_Remote_Ammo] before:true now:false
[M_Titan_AA] before:false now:true
[M_Air_AA_MI06] before:false now:true
[M_Mo_230mm_AT] before:true now:false
[ammo_Penetrator_AGM_01] before:true now:false
[SatchelCharge_Remote_Ammo_Scripted] before:true now:false
[M_Mo_82mm_AT_LG] before:true now:false
[DemoCharge_Remote_Ammo] before:true now:false
[Missile_AA_04_F] before:false now:true
[Mo_cluster_AP] before:true now:false
[BombCluster_02_Ammo_F] before:true now:false
[Mo_cluster_AP_UXO_deploy] before:true now:false
[M_Air_AA] before:false now:true
[ACE_SLAMDirectionalMine_Command_Ammo] before:true now:false
[UXO_deploy_BombCluster_base_F] before:true now:false
[UXO_deploy_base_f] before:true now:false
[ammo_Penetrator_AGM_02] before:true now:false
[ACE_G_40mm_HEDP] before:true now:false
[G_40mm_HEDP] before:true now:false
[M_Titan_AA_static] before:false now:true
[Missile_AA_03_F] before:false now:true
[APERSBoundingMine_Range_Ammo] before:true now:false
[UnderwaterMinePDM_Range_Ammo] before:true now:false

"G_40mm_HE" call ace_frag_fnc_shouldFrag = false is the big one that I think will be noticed IMHO we should force it so it works like before

PabstMirror avatar Aug 22 '24 18:08 PabstMirror

Just a possibility but we could use #10243 's addExplosionEventHandler roughly done in https://github.com/acemod/ACE3/commit/0642d3724dc724ff359b7203469badae9506d312

This will automatically pickup submunitions Reduce fired eh code as we only have to check should frag on explosions No race condition on blacklisting

PabstMirror avatar Aug 22 '24 18:08 PabstMirror

All changes in shouldFrag for vanilla ammo [long list] "G_40mm_HE" call ace_frag_fnc_shouldFrag = false is the big one that I think will be noticed IMHO we should force it so it works like before

Good catch, I missed forcing the 40mm again. Here's a classification of all of the items mentioned above if you want to provide further feedback on it.

Reverted to before
[G_40mm_HE] before:true now:false
[G_40mm_HEDP_Bullet] before:true now:false
[ACE_G_40mm_HE] before:true now:false
[ACE_G_40mm_HEDP] before:true now:false
[G_20mm_HE] before:true now:false
[ammo_Missile_LongRangeAABase] before:true now:false
A number just produce submunitions
[BombCluster_03_Ammo_F] before:true now:false 
[BombCluster_02_Cap_Ammo_F] before:true now:false
[BombCluster_01_Ammo_F] before:true now:false
[BombDemine_01_Ammo_F] before:true now:false
[BombCluster_02_UXO_deploy] before:true now:false
[BombCluster_03_UXO_deploy] before:true now:false
[BombCluster_01_UXO_deploy] before:true now:false
[BombCluster_02_Ammo_F] before:true now:false
[Mo_cluster_AP_UXO_deploy] before:true now:false
[UXO_deploy_BombCluster_base_F] before:true now:false (Base class)
[UXO_deploy_base_f] before:true now:false  (Base class)
Some would not have created fragments before as previously submunitions did not, but could
[Mo_cluster_AP] before:true now:false 
[APERSBoundingMine_Range_Ammo] before:true now:false
[UnderwaterMinePDM_Range_Ammo] before:true now:false
[UnderwaterMine_Range_Ammo] before:true now:false
[UnderwaterMineAB_Range_Ammo] before:true now:false
Some shouldn't produce fragments
[ammo_Penetrator_AGM_01] before:true now:false -> penetrators shouldn't produce frag, spall is ok
[ammo_Penetrator_AGM_02] before:true now:false -> penetrators shouldn't produce frag, spall is ok
[SatchelCharge_Remote_Ammo] before:true now:false -> Bricks of explosive in a fabric bag
[SatchelCharge_Remote_Ammo_Scripted] before:true now:false
[DemoCharge_Remote_Ammo] before:true now:false -> Bricks of explosive taped together
[DemoCharge_Remote_Ammo_Scripted] before:true now:false
[Drone_explosive_ammo_dummy] before:true now:false -> dummy
Some could produce few fragments in reality and may not be worth simulating
[HelicopterExploSmall] before:true now:false -> this one is up to everyone, but I removed the gas station explosions fragmentation which this inherits from
[HelicopterExploBig] before:true now:false -> this one is up to everyone, but I removed the gas station explosions fragmentation which this inherits from
[ammo_Missile_mim145] before:true now:false -> designed to be a direct contact missile / not many good reasons to sim frag
[ammo_Missile_s750] before:true now:false -> S-350, same reason as MIM-145
[ClaymoreDirectionalMine_Remote_Ammo_Scripted] before:true now:false
[ClaymoreDirectionalMine_Remote_Ammo] before:true now:false
[BombDemine_01_SubAmmo_F] before:true now:false
[ACE_SLAMDirectionalMine_Timer_Ammo] before:true
[ACE_SLAMDirectionalMine_Command_Ammo] before:true now:false
[SLAMDirectionalMine_Wire_Ammo] before:true now:false
[Drone_explosive_ammo] before:true now:false
Added because they could be
[M_Air_AA_MI02] before:false now:true
[M_Titan_AA_long] before:false now:true
[R_80mm_HE] before:false now:true
[M_Zephyr_air] before:false now:true
[M_Zephyr_Mi06] before:false now:true
[M_Zephyr] before:false now:true
[R_60mm_HE] before:false now:true
[M_Titan_AA] before:false now:true
[M_Air_AA_MI06] before:false now:true
[Missile_AA_04_F] before:false now:true
[M_Air_AA] before:false now:true

lambdatiger avatar Aug 23 '24 02:08 lambdatiger

Just a possibility but we could use #10243 's addExplosionEventHandler roughly done in 0642d37

This will automatically pickup submunitions Reduce fired eh code as we only have to check should frag on explosions No race condition on blacklisting

~~I've elaborated on why I think it may be an issue on your PR, it's why I didn't stick with init event handlers in https://github.com/acemod/ACE3/pull/9728. If I'm wrong and it doesn't cause issues, I'd be happy to switch to it.~~

~~Let me spend a bit looking back through my notes, there was a reason I moved away from the init event handlers for frag, that depended on changes in locality.~~

Alright I found the reason

https://github.com/user-attachments/assets/72093daf-053e-4e90-873b-46bd1dd6f0ad

Part of why I wanted to spin out the original PR into more PRs was not only because it'd make reviews easier past this first one, but because it'd implement the changes I wanted besides the fragmentation ones that I would prefer to use the init event handler for (or your exploded), as well as gives me more time while I was looking for munitions statistics.

lambdatiger avatar Aug 23 '24 02:08 lambdatiger

~~I'll run through and check command cases once HEMTT's winget repository is updated.~~ Should be all good

lambdatiger avatar Sep 04 '24 03:09 lambdatiger

Will switch to the common explode event handler when it hits the main branch: https://github.com/lambdatiger/ace_frag-overhaul/tree/ace-explode-event-handler

lambdatiger avatar Sep 04 '24 19:09 lambdatiger

#10243 was merged, would it be preferred to create a separate PR or just commit to this PR when migrating to the common explode event?

lambdatiger avatar Sep 07 '24 23:09 lambdatiger

Just commit on this branch.

LinkIsGrim avatar Sep 08 '24 21:09 LinkIsGrim

Alright, one comment here. There is a setShotParents on in the doSpall function that is run locally. The current master version works the same way and creates spall locally, and I didn't change that, but I had added the setShotParents. So setting shotParents won't work off server in MP, and may just create config spam (not 100% certain if a message gets spit out).

lambdatiger avatar Sep 12 '24 21:09 lambdatiger

I think we have a wrapper for setShotParents in common (either event or function), so that should be used instead

LinkIsGrim avatar Sep 13 '24 05:09 LinkIsGrim

I think we have a wrapper for setShotParents in common (either event or function), so that should be used instead

The issue isn't that, but that the projectiles are createVehicleLocal so while the simulated damage and effects propagate, the projectiles don't and aren't something you have access to on the server as far as I have tried (the same way as the master version doSpall and frago).

lambdatiger avatar Sep 14 '24 00:09 lambdatiger