[Freecol-developers] [PATCH] common: model: encapsulate checks for ability to build

[Enrico Weigelt, metux IT consult]

BuildableType's and Colony's are often checked for whether they are
able to build certain BuildableType in general (different from whether
a Colony can do it right now, eg. required resources available).

Instead of directly going to the low-level ability test, introduce an
semantic interface, se we can change implementation beind easily
(eg. add some caching in front of the expensive ability computations)
---
.../sf/freecol/client/gui/panel/BuildQueuePanel.java | 4 ++--
src/net/sf/freecol/common/model/BuildableType.java | 20 ++++++++++++++++++++
src/net/sf/freecol/common/model/BuildingType.java | 2 +-
src/net/sf/freecol/common/model/Colony.java | 20 ++++++++++++++++++++
.../sf/freecol/common/model/FeatureContainer.java | 11 +++++++++++
src/net/sf/freecol/common/model/UnitType.java | 8 ++++----
src/net/sf/freecol/server/ai/ColonyPlan.java | 2 +-
7 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/src/net/sf/freecol/client/gui/panel/BuildQueuePanel.java b/src/net/sf/freecol/client/gui/panel/BuildQueuePanel.java
index f1b6237fbd9..414e1f2ca23 100644
--- a/src/net/sf/freecol/client/gui/panel/BuildQueuePanel.java
+++ b/src/net/sf/freecol/client/gui/panel/BuildQueuePanel.java
@@ -869,8 +869,8 @@ public class BuildQueuePanel extends FreeColPanel implements ItemListener {
if (!checkAbilities(ut, reasons)) unbuildableTypes.add(ut);

// Missing unit build ability?
- if (!this.featureContainer.hasAbility(Ability.BUILD, ut, Turn.UNDEFINED)
- && !this.colony.hasAbility(Ability.BUILD, ut, turn)) {
+ if (!this.featureContainer.ableToBuild(ut)
+ && !this.colony.ableToBuild(ut, turn)) {
Ability buildAbility = find(spec.getAbilities(Ability.BUILD),
a -> (a.appliesTo(ut)
&& a.getValue()
diff --git a/src/net/sf/freecol/common/model/BuildableType.java b/src/net/sf/freecol/common/model/BuildableType.java
index f8d55245006..ed99077e876 100644
--- a/src/net/sf/freecol/common/model/BuildableType.java
+++ b/src/net/sf/freecol/common/model/BuildableType.java
@@ -235,6 +235,26 @@ public abstract class BuildableType extends FreeColSpecObjectType {
}

/**
+ * Returns true if this Object is in general able to build the given
+ * BuildableType. This is different to canBuild() in that it just
+ * checks the general ability, not other factors, eg. available goods etc.
+ *
+ * @param buildableType a {@code BuildableType} value
+ * @return a {@code boolean} value
+ */
+ public final boolean ableToBuild(BuildableType buildableType) {
+ return hasAbility(Ability.BUILD, buildableType);
+ }
+
+ public final boolean ableToBuild(BuildableType buildableType, int turn) {
+ return hasAbility(Ability.BUILD, buildableType, turn);
+ }
+
+ public final boolean ableToBuild() {
+ return hasAbility(Ability.BUILD);
+ }
+
+ /**
* Check to see if this buildable type can be built in a colony
* based on the buildings or units available.
*
diff --git a/src/net/sf/freecol/common/model/BuildingType.java b/src/net/sf/freecol/common/model/BuildingType.java
index d8ab71471d6..72375ef7576 100644
--- a/src/net/sf/freecol/common/model/BuildingType.java
+++ b/src/net/sf/freecol/common/model/BuildingType.java
@@ -428,7 +428,7 @@ public final class BuildingType extends BuildableType
&& upgradesTo.equals(toBuild)) return index;

// Don't go past a unit this building can build.
- if (this.hasAbility(Ability.BUILD, toBuild)) {
+ if (this.ableToBuild(toBuild)) {
return index;
}
}
diff --git a/src/net/sf/freecol/common/model/Colony.java b/src/net/sf/freecol/common/model/Colony.java
index ccbdef04374..d9c59d3e725 100644
--- a/src/net/sf/freecol/common/model/Colony.java
+++ b/src/net/sf/freecol/common/model/Colony.java
@@ -980,6 +980,26 @@ public class Colony extends Settlement implements Nameable, TradeLocation {
}

/**
+ * Returns true if this Colony is in general able to build the given
+ * BuildableType. This is different to canBuild() in that it just
+ * checks the general ability, not other factors, eg. available goods etc.
+ *
+ * @param buildableType a {@code BuildableType} value
+ * @return a {@code boolean} value
+ */
+ public final boolean ableToBuild(BuildableType buildableType) {
+ // FIXME: we could add some caching here, but for that we'd have to
+ // work out the actual keys and invalidation criteria. Hopefully,
+ // that should be a global cache, not per Colony. But that really
+ // needs some deeper thoughts
+ return hasAbility(Ability.BUILD, buildableType);
+ }
+
+ public final boolean ableToBuild(BuildableType buildableType, int turn) {
+ return hasAbility(Ability.BUILD, buildableType, turn);
+ }
+
+ /**
* Return the reason why the give {@code BuildableType} can
* not be built.
*
diff --git a/src/net/sf/freecol/common/model/FeatureContainer.java b/src/net/sf/freecol/common/model/FeatureContainer.java
index ca2de20a884..a08f8971bd0 100644
--- a/src/net/sf/freecol/common/model/FeatureContainer.java
+++ b/src/net/sf/freecol/common/model/FeatureContainer.java
@@ -250,6 +250,17 @@ public final class FeatureContainer {
}
}

+ /**
+ * Returns true if this Object is in general able to build the given
+ * BuildableType. This is different to canBuild() in that it just
+ * checks the general ability, not other factors, eg. available goods etc.
+ *
+ * @param buildableType a {@code BuildableType} value
+ * @return a {@code boolean} value
+ */
+ public final boolean ableToBuild(BuildableType buildableType) {
+ return hasAbility(Ability.BUILD, buildableType, Turn.UNDEFINED);
+ }

/**
* Gets the set of modifiers with the given identifier from this
diff --git a/src/net/sf/freecol/common/model/UnitType.java b/src/net/sf/freecol/common/model/UnitType.java
index d386854b203..25d32371666 100644
--- a/src/net/sf/freecol/common/model/UnitType.java
+++ b/src/net/sf/freecol/common/model/UnitType.java
@@ -536,8 +536,8 @@ public final class UnitType extends BuildableType implements Consumer {
List<BuildableType> assumeBuilt) {
// Non-person units need a BUILD ability, present or assumed.
if (!hasAbility(Ability.PERSON)
- && !colony.hasAbility(Ability.BUILD, this)
- && none(assumeBuilt, bt -> bt.hasAbility(Ability.BUILD, this))) {
+ && !colony.ableToBuild(this)
+ && none(assumeBuilt, bt -> bt.ableToBuild(this))) {
return Colony.NoBuildReason.MISSING_BUILD_ABILITY;
}
return Colony.NoBuildReason.NONE;
@@ -548,7 +548,7 @@ public final class UnitType extends BuildableType implements Consumer {
ListModel<BuildableType> buildQueue = buildQueueList.getModel();
if (colony.canBuild(this)) return 0;
for (int index = 0; index < buildQueue.getSize(); index++) {
- if (buildQueue.getElementAt(index).hasAbility(Ability.BUILD, this)) return index + 1;
+ if (buildQueue.getElementAt(index).ableToBuild(this)) return index + 1;
}
return UNABLE_TO_BUILD;
}
@@ -570,7 +570,7 @@ public final class UnitType extends BuildableType implements Consumer {
for (int index = 0; index < buildQueue.getSize(); index++) {
BuildableType toBuild = buildQueue.getElementAt(index);
if (toBuild == this) continue;
- if (toBuild.hasAbility(Ability.BUILD, this)) {
+ if (toBuild.ableToBuild(this)) {
return buildQueueLastPos;
}
}
diff --git a/src/net/sf/freecol/server/ai/ColonyPlan.java b/src/net/sf/freecol/server/ai/ColonyPlan.java
index f0d0232012d..bdd7733831a 100644
--- a/src/net/sf/freecol/server/ai/ColonyPlan.java
+++ b/src/net/sf/freecol/server/ai/ColonyPlan.java
@@ -722,7 +722,7 @@ public class ColonyPlan {
prioritize(type, FISH_WEIGHT, factor);
}

- if (type.hasAbility(Ability.BUILD)) {
+ if (type.ableToBuild()) {
double factor = ("building".equals(advantage)) ? 1.1 : 1.0;
double support = (any(type.getAbilities(Ability.BUILD),
Ability::hasScope)) ? 0.1 : 1.0;