Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 5229: Multi-Trac units not firing energy weapons #5248

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Fix 5229: Multi-Trac units not firing energy weapons #5248

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
899b89f
Add energy weapon firing calcs for multi-trac; normalize Princess log…
Sleet01 Mar 14, 2024
e58efc3
Align MultiTargetFireControl with recent changes; skip DEBUG test in …
Sleet01 Mar 14, 2024
5108dd9
Reduce getLogger calls and consolidate Firing Plan logging in Princess
Sleet01 Mar 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions megamek/src/megamek/client/bot/princess/FireControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can just do the debug logging alongside the Princess-level .info() logging, which is once per unit per round.

Copy link
Member

@NickAragua NickAragua Mar 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.getDisplayName() + " - Best Firing Plan: " + plan.getDebugDescription(true));
if ((null == bestPlan) || (plan.getUtility() > bestPlan.getUtility())) {
bestPlan = plan;
Expand Down Expand Up @@ -3611,7 +3612,7 @@ private int switchMissileMode(Mounted weapon) {
* @param wtype that uses ammo that is not tracked, or not actually ammo
* @return true if wtype doesn't actually track ammo
*/
private static boolean effectivelyAmmoless(WeaponType wtype) {
protected static boolean effectivelyAmmoless(WeaponType wtype) {
List<Integer> atypes = Arrays.asList(
AmmoType.T_NA,
AmmoType.T_INFANTRY
Expand Down
43 changes: 29 additions & 14 deletions megamek/src/megamek/client/bot/princess/MultiTargetFireControl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import megamek.common.*;
import megamek.common.options.OptionsConstants;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;

/**
Expand Down Expand Up @@ -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: " +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per the other logging comment - this is in a loop so we want to eliminate it.

bestPlan.getDebugDescription(true));
}

// check the plan where the shooter flips its arms
Expand Down Expand Up @@ -111,29 +116,39 @@ WeaponFireInfo getBestShot(Entity shooter, Mounted weapon) {
WeaponFireInfo bestShot = null;

for (Targetable target : getTargetableEnemyEntities(shooter, owner.getGame(), owner.getFireControlState())) {
WeaponFireInfo betterShot = null;
final int ownerID = (target instanceof Entity) ? ((Entity) target).getOwnerId() : -1;
if (owner.getHonorUtil().isEnemyBroken(target.getId(), ownerID, owner.getBehaviorSettings().isForcedWithdrawal())) {
LogManager.getLogger().info(target.getDisplayName() + " is broken - ignoring");
continue;
}

ArrayList<Mounted> ammos;
if (weapon.getBayWeapons().isEmpty()) {
ammos = shooter.getAmmo(weapon);
if (effectivelyAmmoless((WeaponType) weapon.getType())) {
betterShot = buildWeaponFireInfo(shooter, target, weapon, null, owner.getGame(), false);
} else {
ammos = new ArrayList<>();
ammos.add(null);
}
ArrayList<Mounted> ammos;
if (weapon.getBayWeapons().isEmpty()) {
ammos = shooter.getAmmo(weapon);
} else {
ammos = new ArrayList<>();
ammos.add(null);
}

for (Mounted ammo: ammos) {
WeaponFireInfo shot = buildWeaponFireInfo(shooter, target, weapon, ammo, owner.getGame(), false);
for (Mounted ammo : ammos) {
WeaponFireInfo shot = buildWeaponFireInfo(shooter, target, weapon, ammo, owner.getGame(), false);

// this is a better shot if it has a chance of doing damage and the damage is better than the previous best shot
if ((shot.getExpectedDamage() > 0) &&
((bestShot == null) || (shot.getExpectedDamage() > bestShot.getExpectedDamage()))) {
bestShot = shot;
// this is a better shot if it has a chance of doing damage and the damage is better than the previous best shot
if ((shot.getExpectedDamage() > 0) &&
((betterShot == null) || (shot.getExpectedDamage() > betterShot.getExpectedDamage()))) {
betterShot = shot;
}
}
}
// Now do the same comparison for the better shot of all these shots to determine the *best* shot
if ((betterShot != null && betterShot.getExpectedDamage() > 0) &&
((bestShot == null) || (betterShot.getExpectedDamage() > bestShot.getExpectedDamage()))) {
bestShot = betterShot;
}
}

return bestShot;
Expand Down Expand Up @@ -176,7 +191,7 @@ void calculateUtility(final FiringPlan firingPlan,
firingPlan.setUtility(utility);
}

FiringPlan calculateFiringPlan(Entity shooter, List<Mounted> weaponList) {
protected FiringPlan calculateFiringPlan(Entity shooter, List<Mounted> weaponList) {
FiringPlan retVal = new FiringPlan();

List<WeaponFireInfo> shotList = new ArrayList<>();
Expand All @@ -199,7 +214,7 @@ FiringPlan calculateFiringPlan(Entity shooter, List<Mounted> weaponList) {
retVal = calculateIndividualWeaponFiringPlan(shooter, shotList, shooterIsLarge);
}

calculateUtility(retVal, calcHeatTolerance(shooter, shooter.isAero()), true);
calculateUtility(retVal, calcHeatTolerance(shooter, shooter.isAero()), shooter.isAero());
return retVal;
}

Expand Down
3 changes: 2 additions & 1 deletion megamek/src/megamek/client/bot/princess/Princess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,8 +609,9 @@ protected void calculateFiringTurn() {
getFireControl(shooter).loadAmmo(shooter, plan);
plan.sortPlan();

// FireControl instances will handle debug logging
LogManager.getLogger().info(shooter.getDisplayName() + " - Best Firing Plan: " +
plan.getDebugDescription(LogManager.getLogger().getLevel().isLessSpecificThan(Level.DEBUG)));
plan.getDebugDescription(false));

// Add expected damage from the chosen FiringPlan to the
// damageMap for the target enemy.
Expand Down
Loading