Skip to content

Commit

Permalink
Fix a few bugs in libtiled-java (mapeditor#1840)
Browse files Browse the repository at this point in the history
 * Make sure the properties of MapLayer is not null
 * Prevent MapObject's from being added twice (once manually, once with JXB)
 * Don't need to read properties again (already read with JXB)
 * Null-pointer issues with MapLayer.getBounds if x/y coordinate is null
 * Null-pointer issues when accessing objects list in ObjectGroup class
 * Add getProperty overload in Property class to allow a default value
 * Fix image loading issue in TMXMapReader when loading from an InputStream.
   Apparently this function was setting `xmlPath` to a url, not a path.
 * Add code to write object group attributes (previously it was completely
   omitted). A writeLayerAttributes method was added to write common attributes
   in both a tile-layer and object-group.
 * Fix writing of offsetx and offsety attributes.
  • Loading branch information
theKidOfArcrania authored and bjorn committed Jan 12, 2018
1 parent 7971fa8 commit ddbd31b
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public void setOffset(int x, int y) {
* @return the layer bounds in tiles
*/
public Rectangle getBounds() {
return new Rectangle(x, y, width, height);
return new Rectangle(x == null ? 0 : x, y == null ? 0 : y, width, height);
}

/**
Expand All @@ -196,6 +196,15 @@ public void getBounds(Rectangle rect) {
rect.height = this.height;
}

/** {@inheritDoc} */
@Override
public Properties getProperties() {
if (properties == null) {
properties = new Properties();
}
return super.getProperties();
}

/**
* A convenience method to check if a point in tile-space is within the
* layer boundaries.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@
import java.awt.Shape;
import java.awt.geom.Ellipse2D;
import java.awt.geom.Rectangle2D;
import java.util.ArrayList;
import java.util.LinkedList;
import java.util.Iterator;
import java.util.List;

import javax.xml.bind.annotation.XmlAccessType;
import javax.xml.bind.annotation.XmlAccessorType;
Expand Down Expand Up @@ -100,15 +102,15 @@ public ObjectGroup(Rectangle area) {
* @return a boolean.
*/
public boolean isEmpty() {
return objects.isEmpty();
return getObjects().isEmpty();
}

/** {@inheritDoc} */
@Override
public Object clone() throws CloneNotSupportedException {
ObjectGroup clone = (ObjectGroup) super.clone();
clone.objects = new LinkedList<>();
for (MapObject object : objects) {
for (MapObject object : getObjects()) {
final MapObject objectClone = (MapObject) object.clone();
clone.objects.add(objectClone);
objectClone.setObjectGroup(clone);
Expand All @@ -122,7 +124,7 @@ public Object clone() throws CloneNotSupportedException {
* @param o a {@link org.mapeditor.core.MapObject} object.
*/
public void addObject(MapObject o) {
objects.add(o);
getObjects().add(o);
o.setObjectGroup(this);
}

Expand All @@ -132,14 +134,14 @@ public void addObject(MapObject o) {
* @param o a {@link org.mapeditor.core.MapObject} object.
*/
public void removeObject(MapObject o) {
objects.remove(o);
getObjects().remove(o);
o.setObjectGroup(null);
}

/** {@inheritDoc} */
@Override
public Iterator<MapObject> iterator() {
return objects.iterator();
return getObjects().iterator();
}

/**
Expand All @@ -150,7 +152,7 @@ public Iterator<MapObject> iterator() {
* @return a {@link org.mapeditor.core.MapObject} object.
*/
public MapObject getObjectAt(double x, double y) {
for (MapObject obj : objects) {
for (MapObject obj : getObjects()) {
// Attempt to get an object bordering the point that has no width
if (obj.getWidth() == 0 && obj.getX() + this.x == x) {
return obj;
Expand Down Expand Up @@ -184,7 +186,7 @@ public MapObject getObjectNear(int x, int y, double zoom) {
Rectangle2D mouse = new Rectangle2D.Double(x - zoom - 1, y - zoom - 1, 2 * zoom + 1, 2 * zoom + 1);
Shape shape;

for (MapObject obj : objects) {
for (MapObject obj : getObjects()) {
if (obj.getWidth() == 0 && obj.getHeight() == 0) {
shape = new Ellipse2D.Double(obj.getX() * zoom, obj.getY() * zoom, 10 * zoom, 10 * zoom);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,23 @@ public void setProperty(String name, String value) {
* @return a {@link java.lang.String} object.
*/
public String getProperty(String name) {
return getProperty(name, null);
}

/**
* Gets a property with a default value if this property is not found
*
* @param name a {@link java.lang.String} object.
* @param defaultValue the string value to return if property is not found
* @return a {@link java.lang.String} object.
*/
public String getProperty(String name, String defaultValue) {
for (Property property : properties) {
if (name.equals(property.getName())) {
return property.getValue();
}
}
return null;
return defaultValue;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -499,20 +499,17 @@ private ObjectGroup unmarshalObjectGroup(Node t) throws Exception {
final int offsetY = getAttribute(t, "y", 0);
og.setOffset(offsetX, offsetY);

// Add all objects from the objects group
NodeList children = t.getChildNodes();
// Manually parse the objects in object group
og.getObjects().clear();

NodeList children = t.getChildNodes();
for (int i = 0; i < children.getLength(); i++) {
Node child = children.item(i);
if ("object".equalsIgnoreCase(child.getNodeName())) {
og.addObject(readMapObject(child));
}
}

Properties props = new Properties();
readProperties(children, props);
og.setProperties(props);

return og;
}

Expand Down Expand Up @@ -710,10 +707,9 @@ private void buildMap(Document doc) throws Exception {
throw new Exception("Couldn't load map.");
}

// Load properties
readProperties(mapNode.getChildNodes(), map.getProperties());
// Don't need to load properties again.

// Clear untl they are loaded correctly
// We need to load layers and tilesets manually so that they are loaded correctly
map.getTileSets().clear();
map.getLayers().clear();

Expand Down Expand Up @@ -805,7 +801,8 @@ public Map readMap(String filename) throws Exception {
* @throws java.lang.Exception if any.
*/
public Map readMap(InputStream in) throws Exception {
xmlPath = makeUrl(".");
//xmlPath = makeUrl(".");
xmlPath = System.getProperty("user.dir") + File.separatorChar;

Map unmarshalledMap = unmarshal(in);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ private void writeMap(Map map, XMLWriter w, String wp) throws IOException {
w.endElement();
}

private static void writeProperties(Properties props, XMLWriter w) throws
private void writeProperties(Properties props, XMLWriter w) throws
IOException {
if (props != null && !props.isEmpty()) {
final Set<Object> propertyKeys = new TreeSet<>();
Expand Down Expand Up @@ -347,30 +347,43 @@ private void writeTileset(TileSet set, XMLWriter w, String wp)
w.endElement();
}

private static void writeObjectGroup(ObjectGroup o, XMLWriter w, String wp)
private void writeObjectGroup(ObjectGroup o, XMLWriter w, String wp)
throws IOException {
w.startElement("objectgroup");

if (o.getColor() != null && o.getColor().isEmpty()) {
w.writeAttribute("color", o.getColor());
}
if (o.getDraworder() != null && !o.getDraworder().equalsIgnoreCase("topdown")) {
w.writeAttribute("draworder", o.getDraworder());
}
writeLayerAttributes(o, w);
writeProperties(o.getProperties(), w);

Iterator<MapObject> itr = o.getObjects().iterator();
while (itr.hasNext()) {
writeMapObject(itr.next(), w, wp);
}

w.endElement();
}

/**
* Writes this layer to an XMLWriter. This should be done <b>after</b> the
* first global ids for the tilesets are determined, in order for the right
* gids to be written to the layer data.
* Writes all the standard layer attributes to the XML writer.
* @param l the map layer to write attributes
* @param w the {@code XMLWriter} instance to write to.
* @throws IOException if an error occurs while writing.
*/
private void writeMapLayer(TileLayer l, XMLWriter w, String wp) throws IOException {
private void writeLayerAttributes(MapLayer l, XMLWriter w) throws IOException {
Rectangle bounds = l.getBounds();

w.startElement("layer");

w.writeAttribute("name", l.getName());
if (bounds.width != 0) {
w.writeAttribute("width", bounds.width);
}
if (bounds.height != 0) {
w.writeAttribute("height", bounds.height);
if (l instanceof TileLayer) {
if (bounds.width != 0) {
w.writeAttribute("width", bounds.width);
}
if (bounds.height != 0) {
w.writeAttribute("height", bounds.height);
}
}
if (bounds.x != 0) {
w.writeAttribute("x", bounds.x);
Expand All @@ -388,6 +401,25 @@ private void writeMapLayer(TileLayer l, XMLWriter w, String wp) throws IOExcepti
w.writeAttribute("opacity", opacity);
}

if (l.getOffsetX() != null && l.getOffsetX() != 0) {
w.writeAttribute("offsetx", l.getOffsetX());
}
if (l.getOffsetY() != null && l.getOffsetY() != 0) {
w.writeAttribute("offsety", l.getOffsetY());
}
}

/**
* Writes this layer to an XMLWriter. This should be done <b>after</b> the
* first global ids for the tilesets are determined, in order for the right
* gids to be written to the layer data.
*/
private void writeMapLayer(TileLayer l, XMLWriter w, String wp) throws IOException {
Rectangle bounds = l.getBounds();

w.startElement("layer");

writeLayerAttributes(l, w);
writeProperties(l.getProperties(), w);

final TileLayer tl = l;
Expand Down Expand Up @@ -535,7 +567,7 @@ private void writeAnimation(Sprite s, XMLWriter w) throws IOException {
w.endElement();
}

private static void writeMapObject(MapObject mapObject, XMLWriter w, String wp)
private void writeMapObject(MapObject mapObject, XMLWriter w, String wp)
throws IOException {
w.startElement("object");
w.writeAttribute("name", mapObject.getName());
Expand All @@ -554,6 +586,13 @@ private static void writeMapObject(MapObject mapObject, XMLWriter w, String wp)
w.writeAttribute("height", mapObject.getHeight());
}

if (mapObject.getTile() != null) {
Tile t = mapObject.getTile();
w.writeAttribute("gid", firstGidPerTileset.get(t.getTileSet()) + t.getId());
} else if (mapObject.getGid() != null) {
w.writeAttribute("gid", mapObject.getGid());
}

writeProperties(mapObject.getProperties(), w);

if (mapObject.getImageSource().length() > 0) {
Expand Down
10 changes: 5 additions & 5 deletions util/java/libtiled-java/src/main/resources/bindings.xjb
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,11 @@
</jxb:javadoc>
</jxb:property>
</jxb:bindings>
<jxb:bindings node="xs:attribute[@name='offsetx']">
<jxb:property name="offsetX"/>
</jxb:bindings>
<jxb:bindings node="xs:attribute[@name='offsety']">
<jxb:property name="offsetY"/>
<jxb:bindings node="xs:attribute[@name='offsetx']">
<jxb:property name="offsetX"/>
</jxb:bindings>
<jxb:bindings node="xs:attribute[@name='offsety']">
<jxb:property name="offsetY"/>
</jxb:bindings>
</jxb:bindings>

Expand Down

0 comments on commit ddbd31b

Please sign in to comment.