Search Unity

  1. We would like to hear your feedback about Unity and our products. Click here for more information.
    Dismiss Notice

Tile Map Refresh Tile Stack Overflow?

Discussion in '2D Experimental Preview' started by Jay-Pavlina, Jun 10, 2016.

  1. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    194
    I am trying to make a terrain tile that is just a single row or column with different sprites for edges, middle, and single. When I place a tile near one of the same tiles on the map, Unity becomes unresponsive and eventually crashes.

    Here is the code for my tile. I think the problem has something to do with RefreshTile, but I can't figure out what I'm doing differently than the sample terrain tile code.

    Code (CSharp):
    1. using UnityEngine;
    2. using UnityEditor;
    3. using System.Collections;
    4. using System.Collections.Generic;
    5. using ExplodingRabbit;
    6.  
    7. [System.Serializable]
    8. public class TerrainTile4x1 : BaseTile {
    9.     [SerializeField] List<Sprite> sprites;
    10.     [SerializeField] Axis axis;
    11.  
    12.     enum Axis { Horizontal, Vertical }
    13.  
    14.     Vector3Int minOffset;
    15.     Vector3Int maxOffset;
    16.  
    17.     #region ScriptableObject
    18.          void OnEnable() {
    19.         if (axis == Axis.Horizontal) {
    20.             minOffset = new Vector3Int(-1, 0, 0);
    21.             maxOffset = new Vector3Int(1, 0, 0);
    22.         } else {
    23.             minOffset = new Vector3Int(0, -1, 0);
    24.             maxOffset = new Vector3Int(0, 1, 0);
    25.         }
    26.     }
    27.     #endregion
    28.  
    29.     #region BaseTile
    30.     public override void RefreshTile(Vector3Int location, ITileMap tileMap) {
    31.         var targetLocation = location + minOffset;
    32.         if (tileMap.HasTileAtLocation(targetLocation, this))
    33.             RefreshTile(targetLocation, tileMap);
    34.         if (tileMap.HasTileAtLocation(location, this))
    35.             base.RefreshTile(location, tileMap);
    36.         targetLocation = location + maxOffset;
    37.         if (tileMap.HasTileAtLocation(targetLocation, this))
    38.             RefreshTile(targetLocation, tileMap);
    39.     }
    40.  
    41.     public override bool GetTileData(Vector3Int location, ITileMap tileMap, ref TileData tileData) {
    42.         var index = tileMap.HasTileAtLocation(location + maxOffset, this) ? 1 : 0;
    43.         index += tileMap.HasTileAtLocation(location + minOffset, this) ? 2 : 0;
    44.         tileData.sprite = sprites[index];
    45.         return true;
    46.     }
    47.  
    48.     #if UNITY_EDITOR
    49.     public override Sprite GetPreview() {
    50.         return sprites.Count > 0 ? sprites[0] : null;
    51.     }
    52.     #endif
    53.     #endregion
    54. }
    55.  
    56. public static class TileMapExtensions {
    57.         public static bool HasTileAtLocation(this ITileMap tileMap, Vector3Int location, BaseTile tile) {
    58.             return tileMap.GetTile(location) == tile;
    59.         }
    60. }
     
  2. superpig

    superpig

    Quis aedificabit ipsos aedificatores? Unity Technologies

    Joined:
    Jan 16, 2011
    Posts:
    4,154
    Consider when axis == Axis.Horizontal. So minOffset is (-1, 0, 0) and maxOffset is (1, 0, 0). Also consider that (0, 0, 0) has a tile already set in it.

    You click at (1, 0, 0). Line 31 computes the min-neighbour tile as (0, 0, 0), which is present, so RefreshTile is called for it. That call then gets to line 36, computing the max-neighbour tile as (1, 0, 0), which is present (as it's the one you just painted), so RefreshTile is called on it. You end up recursively ping-ponging back and forth between the two tiles until you run out of stack space.
     
  3. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    194
    Yes I understand why the stack overflow is happening. What I don't understand is why it is not happening in the example in TerrainTile.cs. Why does it happen with my tile but not with this one?

    Code (CSharp):
    1. public override void RefreshTile(Vector3Int location, ITileMap tileMap)
    2. {
    3.     for (int yd = -1; yd <= 1; yd++)
    4.         for (int xd = -1; xd <= 1; xd++)
    5.         {
    6.             Vector3Int position = new Vector3Int(location.x + xd, location.y + yd, location.z);
    7.             if (TileValue(tileMap, position))
    8.                 tileMap.RefreshTile(position);
    9.         }
    10. }
    11.  
    12. private bool TileValue(ITileMap tileMap, Vector3Int position)
    13. {
    14.     BaseTile tile = tileMap.GetTile(position);
    15.     return (tile != null && tile == this);
    16. }
    17.  
     
  4. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    194
    :eek::eek::eek::eek: I think I just figured it out. It's because I'm calling RefreshTile directly. In the example, it is calling tileMap.RefreshTile!

    Wasted so much time on this... :(

    Edit: Maybe generate a warning for people that make the same mistake in the future? I think it's an easy mistake to make.
     
  5. keely

    keely

    Joined:
    Sep 9, 2010
    Posts:
    966
    Naming them differently would make the mistake a lot harder to make.

    tilemap.RefreshTile() means triggering a GetTileData for that position so perhaps we should rename it to something that communicates that. InvokeGetTileData() is a bit verbose. Any suggestions?
     
  6. Jay-Pavlina

    Jay-Pavlina

    Joined:
    Feb 19, 2012
    Posts:
    194
    tileMap.UpdateTileData()?
     
  7. JasonBricco

    JasonBricco

    Joined:
    Jul 15, 2013
    Posts:
    918
    This was never fixed... and I myself just wasted a lot of time because of it. It was even acknowledged that it was badly named, and it wasn't fixed. I am amazed.
     
  8. rafek1241

    rafek1241

    Joined:
    Oct 21, 2018
    Posts:
    1
    The same for me - i know that this is old topic but when someone would like to check how scriptable tiles are working - this mistake still can be made pretty easily.
     
    maclark86 likes this.