Refresh Tile Stack Overflow?

Unity Engine
1

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.

using UnityEngine;
using UnityEditor;
using System.Collections;
using System.Collections.Generic;
using ExplodingRabbit;

[System.Serializable]
public class TerrainTile4x1 : BaseTile {
    [SerializeField] List<Sprite> sprites;
    [SerializeField] Axis axis;

    enum Axis { Horizontal, Vertical }

    Vector3Int minOffset;
    Vector3Int maxOffset;

    #region ScriptableObject
         void OnEnable() {
        if (axis == Axis.Horizontal) {
            minOffset = new Vector3Int(-1, 0, 0);
            maxOffset = new Vector3Int(1, 0, 0);
        } else {
            minOffset = new Vector3Int(0, -1, 0);
            maxOffset = new Vector3Int(0, 1, 0);
        }
    }
    #endregion

    #region BaseTile
    public override void RefreshTile(Vector3Int location, ITileMap tileMap) {
        var targetLocation = location + minOffset;
        if (tileMap.HasTileAtLocation(targetLocation, this))
            RefreshTile(targetLocation, tileMap);
        if (tileMap.HasTileAtLocation(location, this))
            base.RefreshTile(location, tileMap);
        targetLocation = location + maxOffset;
        if (tileMap.HasTileAtLocation(targetLocation, this))
            RefreshTile(targetLocation, tileMap);
    }

    public override bool GetTileData(Vector3Int location, ITileMap tileMap, ref TileData tileData) {
        var index = tileMap.HasTileAtLocation(location + maxOffset, this) ? 1 : 0;
        index += tileMap.HasTileAtLocation(location + minOffset, this) ? 2 : 0;
        tileData.sprite = sprites[index];
        return true;
    }

    #if UNITY_EDITOR
    public override Sprite GetPreview() {
        return sprites.Count > 0 ? sprites[0] : null;
    }
    #endif
    #endregion
}

public static class TileMapExtensions {
        public static bool HasTileAtLocation(this ITileMap tileMap, Vector3Int location, BaseTile tile) {
            return tileMap.GetTile(location) == tile;
        }
}
2

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

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?

public override void RefreshTile(Vector3Int location, ITileMap tileMap)
{
    for (int yd = -1; yd <= 1; yd++)
        for (int xd = -1; xd <= 1; xd++)
        {
            Vector3Int position = new Vector3Int(location.x + xd, location.y + yd, location.z);
            if (TileValue(tileMap, position))
                tileMap.RefreshTile(position);
        }
}

private bool TileValue(ITileMap tileMap, Vector3Int position)
{
    BaseTile tile = tileMap.GetTile(position);
    return (tile != null && tile == this);
}
4

:hushed::hushed::hushed::hushed: 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… :frowning:

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

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

tileMap.UpdateTileData()?

7

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

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.

2 Likes
9

I’m necroing this thread to reiterate and emphasize the main point. TileBase.RefreshTile must be renamed or documented accurately.

As far as I can tell, and I’ve been debugging this for a few days trying to figure out why things aren’t working the way I expected, TileBase.RefreshTile is only ever called once, by TileMap.SetTile. If this is true, it should be renamed to something akin to OnSetTile, OnTileCreated, or OnSetOnTileMap.

The function TileMap.RefreshTile could stay the same but it has to be made clear that it doesn’t, itself, trigger a call to TileBase.RefreshTile, which, honestly, I think it should. If it did, you could leave everything named the same and just document the logic flow clearly.

If someone picks up this thread and I’ve understood or explained things incorrectly, please do let me know.

3 Likes