Search Unity

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:
    190
    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,124
    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:
    190
    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:
    190
    :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:
    190
    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.