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.

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

3319: Fixing Unofficial On Map Predesignate Invalid Turn Processing NPEs #3414

Merged
merged 8 commits into from
Jan 31, 2022
162 changes: 70 additions & 92 deletions megamek/src/megamek/client/bot/princess/PathRanker.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/*
* MegaMek - Copyright (C) 2000-2011 Ben Mazur (bmazur@sev.org)
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 of the License, or (at your option)
* any later version.
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
* Software Foundation; either version 2 of the License, or (at your option)
* any later version.
*
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
* This program is distributed in the hope that it will be useful, but
* WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
* or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* for more details.
*/
package megamek.client.bot.princess;

Expand All @@ -32,10 +32,9 @@
import java.util.List;

public abstract class PathRanker implements IPathRanker {

//TODO: Introduce PathRankerCacheHelper class that contains "global" path ranker state
//TODO: Introduce FireControlCacheHelper class that contains "global" Fire Control state
//PathRanker classes should be pretty stateless, except pointers to princess and such
// TODO: Introduce PathRankerCacheHelper class that contains "global" path ranker state
// TODO: Introduce FireControlCacheHelper class that contains "global" Fire Control state
// PathRanker classes should be pretty stateless, except pointers to princess and such

/**
* The possible path ranker types.
Expand All @@ -53,28 +52,14 @@ public PathRanker(Princess princess) {
owner = princess;
}

/**
* Gives the "utility" of a path; a number representing how good it is.
* Rankers that extend this class should override this function
*/
RankedPath rankPath(MovePath path, Game game) {
Windchild292 marked this conversation as resolved.
Show resolved Hide resolved
double fallTolerance = getOwner().getBehaviorSettings().getFallShameIndex() / 10d;
Entity me = path.getEntity();
int maxWeaponRange = me.getMaxWeaponRange();
List<Entity> enemies = getOwner().getEnemyEntities();
List<Entity> friends = getOwner().getFriendEntities();
Coords allyCenter = calcAllyCenter(me.getId(), friends, game);

return rankPath(path, game, maxWeaponRange, fallTolerance, enemies, allyCenter);
}

abstract RankedPath rankPath(MovePath path, Game game, int maxRange, double fallTolerance,
List<Entity> enemies, Coords friendsCoords);
protected abstract RankedPath rankPath(MovePath path, Game game, int maxRange,
double fallTolerance, List<Entity> enemies,
Coords friendsCoords);

@Override
public ArrayList<RankedPath> rankPaths(List<MovePath> movePaths, Game game, int maxRange,
double fallTolerance,
List<Entity> enemies, List<Entity> friends) {
double fallTolerance, List<Entity> enemies,
List<Entity> friends) {
// No point in ranking an empty list.
if (movePaths.isEmpty()) {
return new ArrayList<>();
Expand All @@ -85,8 +70,7 @@ public ArrayList<RankedPath> rankPaths(List<MovePath> movePaths, Game game, int

// Let's try to whittle down this list.
List<MovePath> validPaths = validatePaths(movePaths, game, maxRange, fallTolerance);
LogManager.getLogger().debug("Validated " + validPaths.size() + " out of " +
movePaths.size() + " possible paths.");
LogManager.getLogger().debug("Validated " + validPaths.size() + " out of " + movePaths.size() + " possible paths.");

Coords allyCenter = calcAllyCenter(movePaths.get(0).getEntity().getId(), friends, game);

Expand All @@ -100,16 +84,15 @@ public ArrayList<RankedPath> rankPaths(List<MovePath> movePaths, Game game, int
for (MovePath path : validPaths) {
count = count.add(BigDecimal.ONE);

RankedPath rankedPath = rankPath(path, game, maxRange, fallTolerance, enemies,
allyCenter);
RankedPath rankedPath = rankPath(path, game, maxRange, fallTolerance, enemies, allyCenter);

returnPaths.add(rankedPath);

// we want to keep track of if any of the paths we've considered have some kind of damage potential
pathsHaveExpectedDamage |= (rankedPath.getExpectedDamage() > 0);

BigDecimal percent = count.divide(numberPaths, 2, RoundingMode.DOWN).multiply(new BigDecimal(100))
.round(new MathContext(0, RoundingMode.DOWN));
.round(new MathContext(0, RoundingMode.DOWN));
if (percent.compareTo(interval) >= 0) {
if (LogManager.getLogger().getLevel().isLessSpecificThan(Level.INFO)) {
getOwner().sendChat("... " + percent.intValue() + "% complete.");
Expand All @@ -120,21 +103,23 @@ public ArrayList<RankedPath> rankPaths(List<MovePath> movePaths, Game game, int

Entity mover = movePaths.get(0).getEntity();
UnitBehavior behaviorTracker = getOwner().getUnitBehaviorTracker();
boolean noDamageButCanDoDamage = !pathsHaveExpectedDamage && (FireControl.getMaxDamageAtRange(mover, 1, false, false) > 0);

boolean noDamageButCanDoDamage = !pathsHaveExpectedDamage
&& (FireControl.getMaxDamageAtRange(mover, 1, false, false) > 0);

// if we're trying to fight, but aren't going to be doing any damage no matter how we move
// then let's try to get closer
if (noDamageButCanDoDamage && behaviorTracker.getBehaviorType(mover, getOwner()) == BehaviorType.Engaged) {

if (noDamageButCanDoDamage
&& (behaviorTracker.getBehaviorType(mover, getOwner()) == BehaviorType.Engaged)) {
behaviorTracker.overrideBehaviorType(mover, BehaviorType.MoveToContact);
return rankPaths(getOwner().getMovePathsAndSetNecessaryTargets(mover, true), game, maxRange, fallTolerance,
enemies, friends);
return rankPaths(getOwner().getMovePathsAndSetNecessaryTargets(mover, true),
game, maxRange, fallTolerance, enemies, friends);
}

return returnPaths;
}

private List<MovePath> validatePaths(List<MovePath> startingPathList, Game game, int maxRange, double fallTolerance) {
private List<MovePath> validatePaths(List<MovePath> startingPathList, Game game, int maxRange,
double fallTolerance) {
if (startingPathList.isEmpty()) {
// Nothing to validate here, might as well return the empty list
// straight away.
Expand All @@ -144,13 +129,12 @@ private List<MovePath> validatePaths(List<MovePath> startingPathList, Game game,
Entity mover = startingPathList.get(0).getEntity();

Targetable closestTarget = findClosestEnemy(mover, mover.getPosition(), game);
int startingTargetDistance = (closestTarget == null ?
Integer.MAX_VALUE :
closestTarget.getPosition().distance(mover.getPosition()));
int startingTargetDistance = (closestTarget == null) ? Integer.MAX_VALUE
: closestTarget.getPosition().distance(mover.getPosition());

List<MovePath> returnPaths = new ArrayList<>(startingPathList.size());
boolean inRange = (maxRange >= startingTargetDistance);
boolean inRange = maxRange >= startingTargetDistance;

boolean isAirborneAeroOnGroundMap = mover.isAirborneAeroOnGroundMap();
boolean needToUnjamRAC = mover.canUnjamRAC();
int walkMP = mover.getWalkMP();
Expand All @@ -165,16 +149,16 @@ private List<MovePath> validatePaths(List<MovePath> startingPathList, Game game,

try {
// if we are an aero unit on the ground map, we want to discard paths that keep us at altitude 1 with no bombs
if (isAirborneAeroOnGroundMap) {
// if we have no bombs, we want to make sure our altitude is above 1
// if we do have bombs, we may consider altitude bombing (in the future)
if (path.getEntity().getBombs(BombType.F_GROUND_BOMB).isEmpty()
if (isAirborneAeroOnGroundMap) {
// if we have no bombs, we want to make sure our altitude is above 1
// if we do have bombs, we may consider altitude bombing (in the future)
if (path.getEntity().getBombs(BombType.F_GROUND_BOMB).isEmpty()
&& (path.getFinalAltitude() < 2)) {
msg.append("\n\tNo bombs but at altitude 1. No way.");
continue;
}
}
msg.append("\n\tNo bombs but at altitude 1. No way.");
continue;
}
}

Coords finalCoords = path.getFinalCoords();

// Make sure I'm trying to get/stay in range of a target.
Expand Down Expand Up @@ -231,18 +215,13 @@ private List<MovePath> validatePaths(List<MovePath> startingPathList, Game game,
* @return "Best" out of those paths
*/
@Override
public RankedPath getBestPath(List<RankedPath> ps) {
if (ps.size() == 0) {
return null;
}
return Collections.max(ps);
public @Nullable RankedPath getBestPath(List<RankedPath> ps) {
return ps.isEmpty() ? null : Collections.max(ps);
}


/**
* Performs initialization to help speed later calls of rankPath for this
* unit on this turn. Rankers that extend this class should override this
* function
* Performs initialization to help speed later calls of rankPath for this unit on this turn.
* Rankers that extend this class should override this function
*/
@Override
public void initUnitTurn(Entity unit, Game game) {
Expand All @@ -257,15 +236,18 @@ public Targetable findClosestEnemy(Entity me, Coords position, Game game) {
* Find the closest enemy to a unit with a path
*/
@Override
public Targetable findClosestEnemy(Entity me, Coords position, Game game, boolean includeStrategicTargets) {
public Targetable findClosestEnemy(Entity me, Coords position, Game game,
boolean includeStrategicTargets) {
int range = 9999;
Targetable closest = null;
List<Entity> enemies = getOwner().getEnemyEntities();
for (Entity e : enemies) {
// Skip airborne aero units as they're further away than they seem and hard to catch.
// Also, skip withdrawing enemy bot units, to avoid humping disabled tanks and ejected mechwarriors
// Also, skip withdrawing enemy bot units, to avoid humping disabled tanks and ejected
// MechWarriors
if (e.isAirborneAeroOnGroundMap() ||
getOwner().getHonorUtil().isEnemyBroken(e.getTargetId(), e.getOwnerId(), getOwner().getForcedWithdrawal())) {
getOwner().getHonorUtil().isEnemyBroken(e.getTargetId(), e.getOwnerId(),
getOwner().getForcedWithdrawal())) {
continue;
}

Expand Down Expand Up @@ -310,22 +292,20 @@ protected double getMovePathSuccessProbability(MovePath movePath, StringBuilder
double successProbability = 1.0;
msg.append("\n\tCalculating Move Path Success");
for (TargetRoll roll : pilotingRolls) {

// Skip the getting up check. That's handled when checking for being immobile.
// Skip the getting up check. That's handled when checking for being immobile.
if (roll.getDesc().toLowerCase().contains("getting up")) {
continue;
}
if (roll.getDesc().toLowerCase().contains("careful stand")) {
} else if (roll.getDesc().toLowerCase().contains("careful stand")) {
continue;
}
boolean naturalAptPilot = movePath.getEntity().hasAbility(OptionsConstants.PILOT_APTITUDE_PILOTING);
if (naturalAptPilot) {
msg.append("\n\t\tPilot has Natural Aptitude Piloting");
}

msg.append("\n\t\tRoll ").append(roll.getDesc()).append(" ").append(roll.getValue());
double odds = Compute.oddsAbove(roll.getValue(), naturalAptPilot) / 100;
msg.append(" (").append(NumberFormat.getPercentInstance().format(odds)).append(")");
msg.append("\n\t\tRoll ").append(roll.getDesc()).append(' ').append(roll.getValue());
double odds = Compute.oddsAbove(roll.getValue(), naturalAptPilot) / 100d;
msg.append(" (").append(NumberFormat.getPercentInstance().format(odds)).append(')');
successProbability *= odds;
}

Expand All @@ -334,9 +314,9 @@ protected double getMovePathSuccessProbability(MovePath movePath, StringBuilder
msg.append("\n\t\tMASC ");
int target = pathCopy.getEntity().getMASCTarget();
msg.append(target);
// todo Does Natural Aptitude Piloting apply to this? I assume not.
double odds = Compute.oddsAbove(target) / 100;
msg.append(" (").append(NumberFormat.getPercentInstance().format(odds)).append(")");
// TODO : Does Natural Aptitude Piloting apply to this? I assume not.
double odds = Compute.oddsAbove(target) / 100d;
msg.append(" (").append(NumberFormat.getPercentInstance().format(odds)).append(')');
successProbability *= odds;
}
msg.append("\n\t\tTotal = ").append(NumberFormat.getPercentInstance().format(successProbability));
Expand Down Expand Up @@ -383,16 +363,16 @@ public int distanceToHomeEdge(Coords position, CardinalEdge homeEdge, Game game)
break;
}
default: {
LogManager.getLogger().warn("Invalid home edge. Defaulting to NORTH.");
LogManager.getLogger().warn("Invalid home edge. Defaulting to NORTH.");
distance = position.getY();
}
}

return distance;
}

private String validRange(Coords finalCoords, Targetable target, int startingTargetDistance, int maxRange,
boolean inRange) {
private String validRange(Coords finalCoords, Targetable target, int startingTargetDistance,
int maxRange, boolean inRange) {
if (target == null) {
return null;
}
Expand All @@ -403,7 +383,6 @@ private String validRange(Coords finalCoords, Targetable target, int startingTar
if (finalDistanceToTarget > startingTargetDistance) {
return "INVALID: Not in range and moving further away.";
}

} else { // If I am in range, discard any path that takes me out of range.
if (finalDistanceToTarget > maxRange) {
return "INVALID: In range and moving out of range.";
Expand All @@ -414,12 +393,12 @@ private String validRange(Coords finalCoords, Targetable target, int startingTar
}

/**
* Check the path being moved to see if there is a danger of building collapse. Allows a margin of error of 10
* tons in case someone decides to shoot at the building. If jumping, only the landing point is checked. For
* all other move types, the entire path is checked.
* todo reread the rules on basement collapse
* todo skip basement check if random basement option is turned off
* todo incorporate test for building damage just from moving through building
* Check the path being moved to see if there is a danger of building collapse. Allows a margin
* of error of 10 tons in case someone decides to shoot at the building. If jumping, only the
* landing point is checked. For all other move types, the entire path is checked.
* TODO : reread the rules on basement collapse
* TODO : skip basement check if random basement option is turned off
* TODO : incorporate test for building damage just from moving through building
*
* @param path The {@link MovePath} being traversed.
* @param game The {@link Game} being played.
Expand Down Expand Up @@ -468,8 +447,7 @@ private boolean willBuildingCollapse(MovePath path, Game game) {
return false;
}

@Nullable
Coords calcAllyCenter(int myId, List<Entity> friends, Game game) {
protected @Nullable Coords calcAllyCenter(int myId, @Nullable List<Entity> friends, Game game) {
if ((friends == null) || friends.isEmpty()) {
return null;
}
Expand Down Expand Up @@ -520,7 +498,7 @@ protected Princess getOwner() {

/**
* Convenience property to access bot-wide state information.
* @return
* @return the owner's path ranker state
*/
protected PathRankerState getPathRankerState() {
return owner.getPathRankerState();
Expand Down
9 changes: 7 additions & 2 deletions megamek/src/megamek/client/bot/princess/Princess.java
Original file line number Diff line number Diff line change
Expand Up @@ -1981,8 +1981,13 @@ private void unloadTransportedInfantry(MovePath path) {
return;
}

Entity movingEntity = path.getEntity();
Coords pathEndpoint = path.getFinalCoords();
final Entity movingEntity = path.getEntity();
final Coords pathEndpoint = path.getFinalCoords();
if (pathEndpoint == null) {
Windchild292 marked this conversation as resolved.
Show resolved Hide resolved
LogManager.getLogger().error("Can't unload infantry from " + movingEntity.getDisplayName()
+ " because of null path endpoint.");
return;
}
Targetable closestEnemy = getPathRanker(movingEntity).findClosestEnemy(movingEntity, pathEndpoint, getGame(), false);

// if there are no enemies on the board, then we're not unloading anything.
Expand Down
11 changes: 1 addition & 10 deletions megamek/src/megamek/common/Compute.java
Original file line number Diff line number Diff line change
Expand Up @@ -6125,16 +6125,7 @@ public static Entity getSwarmMissileTarget(Game game, int aeId,
return null;
}

/**
* Gets a new target hex for a flight of smoke missiles fired at a hex, if
* there are remaining missiles.
*/

/**
* * STUFF FOR VECTOR MOVEMENT CALCULATIONS **
*/
public static Coords getFinalPosition(Coords curpos, int[] v) {

public static @Nullable Coords getFinalPosition(Coords curpos, int... v) {
if ((v == null) || (v.length != 6) || (curpos == null)) {
return curpos;
}
Expand Down
Loading