Replace spectator cache clear with selective invalidation in Tile::addThing

[ramon-bernardo]

While reviewing the spectator cache logic in the map system, I noticed that when Tile::addThing is called, we currently perform a full cache clear.

I’m considering replacing these calls with a selective cache invalidation approach instead of clearing the entire cache every time.

void Map::clearSpectatorCache() { spectatorCache.clear(); }
void Map::clearPlayersSpectatorCache() { playersSpectatorCache.clear(); }

This ensures consistency but can be costly since all cached spectator data is discarded and needs to be recalculated, even when only a small area is affected.

Here’s the proposed selective invalidation approach:

void Map::invalidateSpectatorCacheForCreature(const Creature* creature)
{
	const auto& creaturePos = creature->getPosition();

	auto invalidateCache = [&](SpectatorCache& cache) {
		std::vector<Position> positionsToInvalidate;
		for (auto& [centerPos, _] : cache) {
			if (std::abs(centerPos.x - creaturePos.x) <= maxViewportX &&
			    std::abs(centerPos.y - creaturePos.y) <= maxViewportY) {
				positionsToInvalidate.push_back(centerPos);
			}
		}

		for (const auto& pos : positionsToInvalidate) {
			cache.erase(pos);
		}
	};

	invalidateCache(spectatorCache);

	if (creature->getPlayer()) {
		invalidateCache(playersSpectatorCache);
	}
}

This would only remove cache entries affected by the moving creature’s position, keeping the rest of the cache valid.

Questions
When invalidating, what’s the better approach?
A) Recalculate everything using getSpectatorsInternal(), ensuring correctness but increasing CPU usage.
B) Simply erase the affected positions from the cache and let them be lazily recalculated later when requested.

[MillhioreBT]

I've already tried it for a very long time, it's literally not worth it, at least not with the map structure we use in TFS.
While the cache is very good, for this specific case it is only good for avoiding multiple recursive calculations in the same frame, that's the only thing it helps us with.
Adding all the complexity of invalidating by regions, only in certain positions, etc... etc... is simply much much more costly than simply recalculating.

My recommendation is that if you want to work on this topic, focus on making the cache accept any variety of inputs,
keep the same logic of doing "clear" in "addThing" etc...
but make all calls to getSpectator cached and not just those that are "maxViewportX/maxViewportY"
this way the cache can also help us in lua scripts that can use getSpectator with different values and avoid writing a couple of extra lines, like saving cache in local variables, etc...

---

I don't want to discourage you, if you think you have a much better idea in mind, go ahead and make a PR with the changes and let's do performance tests to compare.
I'm also curious to know what ideas you have