[tiled-user] Off-by-one bug when adding tilesets?

[Christian Henz]

On Thu, Jun 29, 2006 at 01:13:49PM +0200, Bjørn Lindeijer wrote:
> Hello Christian,
> 
> You are absolutely right, this behaviour creates gaps between the
> tilesets. While this is valid (it still makes sure gids do not
> overlap), it is indeed unnecessary and better avoided.
> 
> I will look into fixing the behaviour of getMaxTileId tonight.
> However, Tiled cannot guarantee that gids won't change and neither
> that gids are contiguous. You can only rely on these things if you are
> sure that the size of your tilesets won't change.
> 
> Thanks for reporting this issue.
> 

Here is a proper patch that reinstates the old behaviour. It
changes the implementation (and use) of getMaxId to comply 
with the documentation.

The problem now is that all maps recently edited in Tiled
will be broken in the same way my maps were.

The proper way to fix it would be to reinstate the old 
behaviour (this patch) and then make the map loading respect
the firstgid of a tileset.

BTW, indentation is broken in NumberedSet.java, which orignially
uses tabs, but the lines I changed use spaces.

cheers,
Christian
-------------- next part --------------
Index: src/tiled/mapeditor/MapEditor.java
===================================================================
--- src/tiled/mapeditor/MapEditor.java	(revision 685)
+++ src/tiled/mapeditor/MapEditor.java	(working copy)
@@ -1949,7 +1949,7 @@
                 while (it.hasNext() && firstTile == null) {
                     TileSet set = (TileSet) it.next();
                     int i = 0;
-                    while (firstTile == null && i < set.getMaxTileId()) {
+                    while (firstTile == null && i <= set.getMaxTileId()) {
                         firstTile = set.getTile(i);
                         i++;
                     }
Index: src/tiled/io/xml/XMLMapTransformer.java
===================================================================
--- src/tiled/io/xml/XMLMapTransformer.java	(revision 685)
+++ src/tiled/io/xml/XMLMapTransformer.java	(working copy)
@@ -373,7 +373,7 @@
                 }
                 else if (child.getNodeName().equalsIgnoreCase("tile")) {
                     Tile tile = unmarshalTile(set, child, tilesetBaseDir);
-                    if (!hasTilesetImage || tile.getId() >= set.getMaxTileId()) {
+                    if (!hasTilesetImage || tile.getId() > set.getMaxTileId()) {
                         set.addTile(tile);
                     } else {
                         Tile myTile = set.getTile(tile.getId());
Index: src/tiled/core/TileSet.java
===================================================================
--- src/tiled/core/TileSet.java	(revision 685)
+++ src/tiled/core/TileSet.java	(working copy)
@@ -213,7 +213,7 @@
      */
     public int addTile(Tile t) {
         if (t.getId() < 0) {
-            t.setId(tiles.getMaxId());
+            t.setId(tiles.getMaxId() + 1);
         }
 
         if (tileDimensions.width < t.getWidth()) {
@@ -295,7 +295,7 @@
     public Vector generateGaplessVector() {
         Vector gapless = new Vector();
 
-        for (int i = 0; i < getMaxTileId(); i++) {
+        for (int i = 0; i <= getMaxTileId(); i++) {
             if (getTile(i) != null) gapless.add(getTile(i));
         }
 
Index: src/tiled/util/NumberedSet.java
===================================================================
--- src/tiled/util/NumberedSet.java	(revision 685)
+++ src/tiled/util/NumberedSet.java	(working copy)
@@ -98,12 +98,37 @@
 	 * @return int
 	 */
 	public int getMaxId() {
+            /*
 		int id = -1;
 		for (int i = 0; i < data.size(); i++) {
 			if (data.get(i) != null) id = i;
 		}
 
 		return id + 1;
+            */
+
+            /* 
+               New implementation:
+               
+               Return the highest id available in the 
+               set or -1 if the set is empty, like the
+               doc says.
+               
+               Start at the back, since we are 
+               searching for the *last* id it
+               might be faster.
+            */            
+            int maxId = data.size() - 1;
+            
+            while(maxId >= 0) {
+                
+                if(data.get(maxId) != null)
+                    break;
+                
+                maxId--;
+            }
+            
+            return maxId;
 	}
 
 	/**
@@ -122,7 +147,7 @@
 	 * @return int
 	 */
 	public int add(Object o) {
-	  int id = getMaxId();
+	  int id = getMaxId() + 1;
 	  put(id, o);
 	  return id;
 	}
@@ -164,4 +189,4 @@
   	public int size() {
   		return data.size();
   	}
-}
\ No newline at end of file
+}