I have attached a patch to let Unit inherit from GoodsLocation. However,
it runs out of heap space when calling disposeList() in
server.ai.mission.TransportMissionTest. And I am too stupid to see where
it loops or runs into an infinite recursion.
Regards
Michael
Index: src/net/sf/freecol/server/model/ServerUnit.java
===================================================================
--- src/net/sf/freecol/server/model/ServerUnit.java (revision 10111)
+++ src/net/sf/freecol/server/model/ServerUnit.java (working copy)
@@ -124,7 +124,7 @@
visibleGoodsCount = -1;
if (type.canCarryGoods()) {
- goodsContainer = new GoodsContainer(game, this);
+ setGoodsContainer(new GoodsContainer(game, this));
}
UnitType newType = type.getTargetType(ChangeType.CREATION, owner);
Index: src/net/sf/freecol/common/model/GoodsLocation.java
===================================================================
--- src/net/sf/freecol/common/model/GoodsLocation.java (revision 10111)
+++ src/net/sf/freecol/common/model/GoodsLocation.java (working copy)
@@ -28,6 +28,8 @@
import javax.xml.stream.XMLStreamReader;
import javax.xml.stream.XMLStreamWriter;
+import org.w3c.dom.Element;
+
/**
* The <code>GoodsLocation</code> is a place where {@link Unit}s and
* {@link Goods} can be put. The GoodsLocation can not store any other
@@ -71,6 +73,19 @@
}
/**
+ * Initialize this object from an XML-representation of this object.
+ *
+ * @param game The <code>Game</code> in which this <code>Unit</code>
+ * belong.
+ * @param e An XML-element that will be used to initialize this object.
+ */
+ // Only Unit needs this
+ public GoodsLocation(Game game, Element e) {
+ super(game, e);
+ readFromXMLElement(e);
+ }
+
+ /**
* Creates a new <code>GoodsLocation</code> instance.
*
* @param game a <code>Game</code> value
@@ -279,17 +294,11 @@
goodsContainer = null;
}
objects.addAll(super.disposeList());
+
return objects;
}
/**
- * Dispose of this GoodsLocation.
- */
- public void dispose() {
- disposeList();
- }
-
- /**
* {@inheritDoc}
*/
protected void writeChildren(XMLStreamWriter out, Player player,
Index: src/net/sf/freecol/common/model/Unit.java
===================================================================
--- src/net/sf/freecol/common/model/Unit.java (revision 10111)
+++ src/net/sf/freecol/common/model/Unit.java (working copy)
@@ -62,14 +62,14 @@
* Every <code>Unit</code> is owned by a {@link Player} and has a
* {@link Location}.
*/
-public class Unit extends FreeColGameObject
- implements Consumer, Locatable, Location, Movable, Nameable, Ownable {
+public class Unit extends GoodsLocation
+ implements Consumer, Locatable, Movable, Nameable, Ownable {
private static final Logger logger = Logger.getLogger(Unit.class.getName());
private static final EquipmentType horsesEq[] = { null, null };
private static final EquipmentType musketsEq[] = { null, null };
-
+
/** A comparator to order units by skill level. */
private static Comparator<Unit> skillLevelComp
= new Comparator<Unit>() {
@@ -285,10 +285,6 @@
protected String ethnicity = null;
- protected List<Unit> units = Collections.emptyList();
-
- protected GoodsContainer goodsContainer;
-
protected Location entryLocation;
protected Location location;
@@ -433,23 +429,24 @@
}
/**
- * Returns a name for this unit, as a location.
+ * Returns true if this unit can carry treasure (like a treasure train)
*
- * @return A name for this unit, as a location.
+ * @return <code>true</code> if this <code>Unit</code> is capable of
+ * carrying treasure.
*/
- public StringTemplate getLocationName() {
- return StringTemplate.template("onBoard")
- .addStringTemplate("%unit%", getLabel());
+ public boolean canCarryTreasure() {
+ return unitType.hasAbility(Ability.CARRY_TREASURE);
}
/**
- * Returns the name for this unit, as a location, for a particular player.
+ * Returns a name for this unit, as a location.
*
- * @param player The <code>Player</code> to prepare the name for.
* @return A name for this unit, as a location.
*/
- public StringTemplate getLocationNameFor(Player player) {
- return getLocationName();
+ @Override
+ public StringTemplate getLocationName() {
+ return StringTemplate.template("onBoard")
+ .addStringTemplate("%unit%", getLabel());
}
/**
@@ -1462,7 +1459,7 @@
: search(start, threatDecider, CostDeciders.avoidIllegal(),
reverseRange, getCarrier());
}
-
+
/**
* Checks if there is a credible threatening unit to this unit
* within a range of moves.
@@ -1786,7 +1783,7 @@
// Do not block for war, bringing gifts is allowed
return (!allowContact(settlement))
? MoveType.MOVE_NO_ACCESS_CONTACT
- : (goodsContainer.getGoodsCount() > 0)
+ : (getGoodsContainer().getGoodsCount() > 0)
? MoveType.ENTER_SETTLEMENT_WITH_CARRIER_AND_GOODS
: MoveType.MOVE_NO_ACCESS_GOODS;
} else {
@@ -1914,7 +1911,7 @@
*/
public int getSpaceTaken() {
int space = unitType.getSpaceTaken() + getGoodsCount();
- for (Unit u : units) space += u.getSpaceTaken();
+ for (Unit u : getUnitList()) space += u.getSpaceTaken();
return space;
}
@@ -2044,17 +2041,14 @@
+ unit.toString()
+ " on " + this.toString());
}
- if (units.contains(locatable)) {
+ if (super.contains(locatable)) {
logger.warning("Already on carrier: " + unit.toString());
return true;
}
- if (units.equals(Collections.emptyList())) {
- units = new ArrayList<Unit>();
- }
spendAllMoves();
unit.setState(UnitState.SENTRY);
- return units.add(unit);
+ return super.add(unit);
} else if (locatable instanceof Goods) {
if (!canCarryGoods()) {
throw new IllegalStateException("Can not carry goods: "
@@ -2067,7 +2061,7 @@
+ " on " + this.toString());
}
spendAllMoves();
- return goodsContainer.addGoods(goods);
+ return getGoodsContainer().addGoods(goods);
} else {
throw new IllegalStateException("Can not be added to unit: "
+ ((FreeColGameObject) locatable).toString());
@@ -2085,10 +2079,10 @@
throw new IllegalArgumentException("Locatable must not be 'null'.");
} else if (locatable instanceof Unit && canCarryUnits()) {
spendAllMoves();
- return units.remove(locatable);
+ return getUnitList().remove(locatable);
} else if (locatable instanceof Goods && canCarryGoods()) {
spendAllMoves();
- return goodsContainer.removeGoods((Goods) locatable) != null;
+ return getGoodsContainer().removeGoods((Goods) locatable) != null;
} else {
logger.warning("Tried to remove a 'Locatable' from a non-carrier unit.");
}
@@ -2096,28 +2090,6 @@
}
/**
- * Checks if this <code>Unit</code> contains the specified
- * <code>Locatable</code>.
- *
- * @param locatable The <code>Locatable</code> to test the presence of.
- * @return
- * <ul>
- * <li><i>true</i> if the specified <code>Locatable</code> is
- * on this <code>Unit</code> and
- * <li><i>false</i> otherwise.
- * </ul>
- */
- public boolean contains(Locatable locatable) {
- if (locatable instanceof Unit && canCarryUnits()) {
- return units.contains(locatable);
- } else if (locatable instanceof Goods && canCarryGoods()) {
- return goodsContainer.contains((Goods) locatable);
- } else {
- return false;
- }
- }
-
- /**
* Checks wether or not the specified locatable may be added to this
* <code>Unit</code>. The locatable cannot be added is this
* <code>Unit</code> if it is not a carrier or if there is no room left.
@@ -2138,6 +2110,14 @@
}
}
+ public int getGoodsCapacity() {
+ throw new RuntimeException("Do not call this method, unless we implement spoilage.");
+ }
+
+ public List<Goods> getGoodsList() {
+ return getGoodsContainer().getGoods();
+ }
+
/**
* Returns the amount of a GoodsType that could be loaded.
*
@@ -2159,25 +2139,16 @@
}
/**
- * Gets the amount of Units at this Location.
- *
- * @return The amount of Units at this Location.
- */
- public int getUnitCount() {
- return units.size();
- }
-
- /**
* Gets the first <code>Unit</code> beeing carried by this
* <code>Unit</code>.
*
* @return The <code>Unit</code>.
*/
public Unit getFirstUnit() {
- if (units.isEmpty()) {
+ if (getUnitList().isEmpty()) {
return null;
} else {
- return units.get(0);
+ return getUnitList().get(0);
}
}
@@ -2188,10 +2159,10 @@
* @return The <code>Unit</code>.
*/
public Unit getLastUnit() {
- if (units.isEmpty()) {
+ if (getUnitList().isEmpty()) {
return null;
} else {
- return units.get(units.size() - 1);
+ return getUnitList().get(getUnitList().size() - 1);
}
}
@@ -2216,55 +2187,6 @@
}
/**
- * Gets a <code>Iterator</code> of every <code>Unit</code> directly
- * located on this <code>Location</code>.
- *
- * @return The <code>Iterator</code>.
- */
- public Iterator<Unit> getUnitIterator() {
- return new ArrayList<Unit>(units).iterator();
- }
-
- /**
- * Gets a list of the units in this carrier unit.
- *
- * @return The list of units in this carrier unit.
- */
- public List<Unit> getUnitList() {
- return new ArrayList<Unit>(units);
- }
-
- /**
- * Gets a <code>Iterator</code> of every <code>Unit</code> directly
- * located on this <code>Location</code>.
- *
- * @return The <code>Iterator</code>.
- */
- public Iterator<Goods> getGoodsIterator() {
- if (canCarryGoods()) {
- return goodsContainer.getGoodsIterator();
- } else {
- return EmptyIterator.getInstance();
- }
- }
-
- /**
- * Returns a <code>List</code> containing the goods carried by this unit.
- * @return a <code>List</code> containing the goods carried by this unit.
- */
- public List<Goods> getGoodsList() {
- if (canCarryGoods()) {
- return goodsContainer.getGoods();
- } else {
- return Collections.emptyList();
- }
- }
-
- public GoodsContainer getGoodsContainer() {
- return goodsContainer;
- }
-
- /**
* Sets the units location without updating any other variables
*
* @param newLocation The new Location
@@ -2634,6 +2556,9 @@
getTeacher().setStudent(null);
setTeacher(null);
}
+ if (newUnitType.canCarryGoods() && getGoodsContainer() == null) {
+ setGoodsContainer(new GoodsContainer(getGame(), this));
+ }
} else {
// ColonialRegulars only available after independence is declared
logger.warning("Units of type: " + newUnitType
@@ -3093,7 +3018,7 @@
}
public int getGoodsCount() {
- return canCarryGoods() ? goodsContainer.getGoodsCount() : 0;
+ return canCarryGoods() ? getGoodsContainer().getGoodsCount() : 0;
}
/**
@@ -3103,8 +3028,8 @@
* @param u The unit to move to the front.
*/
public void moveToFront(Unit u) {
- if (canCarryUnits() && units.remove(u)) {
- units.add(0, u);
+ if (canCarryUnits() && getUnitList().remove(u)) {
+ getUnitList().add(0, u);
}
}
@@ -3293,16 +3218,6 @@
}
/**
- * Returns true if this unit can carry treasure (like a treasure train)
- *
- * @return <code>true</code> if this <code>Unit</code> is capable of
- * carrying treasure.
- */
- public boolean canCarryTreasure() {
- return unitType.hasAbility(Ability.CARRY_TREASURE);
- }
-
- /**
* Returns true if this unit is a ship that can capture enemy goods.
*
* @return <code>true</code> if this <code>Unit</code> is capable of
@@ -3360,9 +3275,14 @@
*/
public List<FreeColGameObject> disposeList() {
List<FreeColGameObject> objects = new ArrayList<FreeColGameObject>();
- while (units.size() > 0) {
- objects.addAll(units.remove(0).disposeList());
+ if (getGoodsContainer() != null) {
+ objects.addAll(getGoodsContainer().disposeList());
+ setGoodsContainer(null);
}
+ for (Unit unit : getUnitList()) {
+ objects.addAll(unit.disposeList());
+ }
+ objects.add(this);
if (location != null) {
location.remove(this);
@@ -3383,22 +3303,10 @@
getOwner().invalidateCanSeeTiles();
getOwner().removeUnit(this);
- if (unitType.canCarryGoods()) {
- objects.addAll(goodsContainer.disposeList());
- }
-
- objects.addAll(super.disposeList());
return objects;
}
/**
- * Removes all references to this object.
- */
- public void dispose() {
- disposeList();
- }
-
- /**
* Return how many turns left to be repaired
*
* @return turns to be repaired
@@ -3587,7 +3495,7 @@
}
return tile;
}
-
+
/**
* Get a settlement by id, validating as much as possible.
* Designed for message unpacking where the id should not be trusted.
@@ -3600,7 +3508,7 @@
public Settlement getAdjacentSettlementSafely(String settlementId)
throws IllegalStateException {
Game game = getOwner().getGame();
-
+
Settlement settlement = game.getFreeColGameObject(settlementId,
Settlement.class);
if (settlement == null) {
@@ -3689,9 +3597,9 @@
private void unitsToXML(XMLStreamWriter out, Player player,
boolean showAll, boolean toSavedGame)
throws XMLStreamException {
- if (!units.isEmpty()) {
+ if (!getUnitList().isEmpty()) {
out.writeStartElement(UNITS_TAG_NAME);
- for (Unit unit : units) {
+ for (Unit unit : getUnitList()) {
unit.toXML(out, player, showAll, toSavedGame);
}
out.writeEndElement();
@@ -3800,12 +3708,12 @@
if (full) {
unitsToXML(out, player, showAll, toSavedGame);
if (getType().canCarryGoods()) {
- goodsContainer.toXML(out, player, showAll, toSavedGame);
+ getGoodsContainer().toXML(out, player, showAll, toSavedGame);
}
} else {
if (getType().canCarryGoods()) {
out.writeAttribute("visibleGoodsCount", Integer.toString(getGoodsCount()));
- goodsContainer.toXML(out, player, showAll, toSavedGame);
+ getGoodsContainer().toXML(out, player, showAll, toSavedGame);
}
}
@@ -3914,8 +3822,8 @@
entryLocation = newLocation(in.getAttributeValue(null, "entryLocation"));
location = newLocation(in.getAttributeValue(null, "location"));
- units.clear();
- if (goodsContainer != null) goodsContainer.removeAll();
+ getUnitList().clear();
+ if (getGoodsContainer() != null) getGoodsContainer().removeAll();
equipment.clear();
setWorkImprovement(null);
}
@@ -3924,19 +3832,19 @@
Game game = getGame();
while (in.nextTag() != XMLStreamConstants.END_ELEMENT) {
if (in.getLocalName().equals(UNITS_TAG_NAME)) {
- units = new ArrayList<Unit>();
+ getUnitList().clear();
while (in.nextTag() != XMLStreamConstants.END_ELEMENT) {
if (in.getLocalName().equals(Unit.getXMLElementTagName())) {
- units.add(updateFreeColGameObject(in, Unit.class));
+ getUnitList().add(updateFreeColGameObject(in, Unit.class));
}
}
} else if (in.getLocalName().equals(GoodsContainer.getXMLElementTagName())) {
- goodsContainer = game.getFreeColGameObject(in.getAttributeValue(null, ID_ATTRIBUTE),
- GoodsContainer.class);
- if (goodsContainer != null) {
- goodsContainer.readFromXML(in);
+ setGoodsContainer(game.getFreeColGameObject(in.getAttributeValue(null, ID_ATTRIBUTE),
+ GoodsContainer.class));
+ if (getGoodsContainer() != null) {
+ getGoodsContainer().readFromXML(in);
} else {
- goodsContainer = new GoodsContainer(game, this, in);
+ setGoodsContainer(new GoodsContainer(game, this, in));
}
} else if (in.getLocalName().equals(EQUIPMENT_TAG)) {
String xLength = in.getAttributeValue(null, ARRAY_SIZE);
@@ -3961,9 +3869,9 @@
}
// ensure all carriers have a goods container, just in case
- if (goodsContainer == null && getType().canCarryGoods()) {
- logger.warning("Carrier with ID " + getId() + " did not have a \"goodsContainer\"-tag.");
- goodsContainer = new GoodsContainer(game, this);
+ if (getGoodsContainer() == null && getType().canCarryGoods()) {
+ logger.warning("Carrier with ID " + getId() + " did not have a \"getGoodsContainer()\"-tag.");
+ setGoodsContainer(new GoodsContainer(game, this));
}
setRole();
Index: src/net/sf/freecol/common/model/UnitLocation.java
===================================================================
--- src/net/sf/freecol/common/model/UnitLocation.java (revision 10111)
+++ src/net/sf/freecol/common/model/UnitLocation.java (working copy)
@@ -29,6 +29,8 @@
import javax.xml.stream.XMLStreamReader;
import javax.xml.stream.XMLStreamWriter;
+import org.w3c.dom.Element;
+
/**
* The <code>UnitLocation</code> is a place where a <code>Unit</code>
* can be put. The UnitLocation can not store any other Locatables,
@@ -136,6 +138,12 @@
super(game, id);
}
+ // Only Unit needs this
+ public UnitLocation(Game game, Element e) {
+ super(game, e);
+ readFromXMLElement(e);
+ }
+
/**
* Removes all references to this object.
*
@@ -151,13 +159,6 @@
}
/**
- * Dispose of this UnitLocation.
- */
- public void dispose() {
- disposeList();
- }
-
- /**
* Gets the current space taken by the units in this location.
*
* @return The sum of the space taken by the units in this location.
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Freecol-developers mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/freecol-developers
