Fix 5229: Multi-Trac units not firing energy weapons #5248
Conversation
…default Princess FiringPlan logging
| @@ -2598,7 +2598,8 @@ FiringPlan getBestFiringPlan(final Entity shooter, | |||
| ammoConservation); | |||
| final FiringPlan plan = determineBestFiringPlan(parameters); | |||
|
|
|||
| LogManager.getLogger().info(shooter.getDisplayName() + " at " + enemy | |||
| // Debug logging; Princess does her own info logging | |||
| LogManager.getLogger().debug(shooter.getDisplayName() + " at " + enemy | |||
There was a problem hiding this comment.
General optimization suggestion: for methods that are called many times, calling logging methods (especially within a loop) leads to a performance decrease, even if the logging level doesn't result in the message being logged. Let's make the adjustment to comment this out as we don't really need the info outside of direct debugging.
There was a problem hiding this comment.
I think we can just do the debug logging alongside the Princess-level .info() logging, which is once per unit per round.
There was a problem hiding this comment.
Sure. This particular logging is is once per unit per enemy per round. Granted, it's during the firing phase which doesn't really have performance issues usually, but still.
| @@ -78,6 +79,10 @@ public FiringPlan getBestFiringPlan(final Entity shooter, | |||
|
|
|||
| if (currentPlan.getUtility() > bestPlan.getUtility()) { | |||
| bestPlan = currentPlan; | |||
|
|
|||
| // Debug logging; Princess does her own info-level logging | |||
| LogManager.getLogger().debug(shooter.getDisplayName() + " - Best Firing Plan: " + | |||
There was a problem hiding this comment.
As per the other logging comment - this is in a loop so we want to eliminate it.
There were a couple issues in the MultiTargetFireControl planning track:
TODO: since MultiTargetFireControl iterates over every target, for every weapon, for every possible arc, there is a good chance that we are calculating the same to-hit chances several times for given target-weapon pairings. We could speed this up by caching results in a map, similar to TAGging is now being tracked.
This should not affect Energy-only Bay Weapons.
Testing:
Close: #5229