feat: add functions to get actors adjacent and with specified distance

[LordMidas]

Adds a more efficient way to get adjacent actors instead of having to iterate over all actors on the battlefield. Primarily targets issue #343.

Usage for adjacent actors:

// To get all adjacent actors: 
::Tactical.Entities.getAdjacentActors(_tile);

// To get ALLIED adjacent actors:
::Tactical.Entities.getAdjacentActors(_tile).filter(@(_, a) a.isAlliedWith(myActor));

// To get Hostile adjacent actors
::Tactical.Entities.getAdjacentActors(_tile).filter(@(_, a) !a.isAlliedWith(myActor));

// To get Same Faction adjacent actors:
::Tactical.Entities.getAdjacentActors(_tile).filter(@(_, a) a.getFaction() == myActor.getFaction());

// To get ALLIED non-faction adjacent actors:
::Tactical.Entities.getAdjacentActors(_tile).filter(@(_, a) a.isAlliedWith(myActor) && a.getFaction() != myActor.getFaction());

[Enduriel]

I don't actually think we should be adding a generic handler for distance here, we can optimize the distance=1 case, so let's, but otherwise we should allow modders to do it themselves, no reason to add extra code to handle this situation in my opinion.

[LordMidas]

I don't actually think we should be adding a generic handler for distance here, we can optimize the distance=1 case, so let's, but otherwise we should allow modders to do it themselves, no reason to add extra code to handle this situation in my opinion.

I don't think there's a clean way to handle the distance == 1 case without adding this extra code because the already present functions call getActorsByFunction which is distance agnostic and therefore always iterates on all actors. Secondly, we need a function to get all actors at a certain distance irrespective of faction allegiance - that doesn't currently exist.

[TaroEld]

I don't actually think we should be adding a generic handler for distance here, we can optimize the distance=1 case, so let's, but otherwise we should allow modders to do it themselves, no reason to add extra code to handle this situation in my opinion.

I'm of the opposite opinion, might as well make it useful.

q.getActorsWithinRange <- function( _tile, _max = 1, _min = 1)
{
	// add validation
	if (_max == 1)
	{
		local ret = [];

		if (_min == 0 &&  _tile.IsOccupiedByActor)
			ret.push(_tile.getEntity());

		for (local i = 0; i < 6; i++)
		{
			if (!_tile.hasNextTile(i))
				continue;

			local nextTile = _tile.getNextTile(i);
			if (nextTile.IsOccupiedByActor)
				ret.push(nextTile.getEntity());
		}

		return ret;
	}
	else
	{
		return this.getActorsByFunction(function(_actor) {
			if (!_actor.isPlacedOnMap())
				return false;
			local distance = _tile.getDistanceTo(_actor.getTile());
			return (distance <= _max && distance >= _min)
		})
	}
}
q.getAlliedActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.isAlliedWith(_actor));
}
q.getHostileActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) !a.isAlliedWith(_actor));
}
q.getSameFactionActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.getFaction() == _actor.getFaction());
}
q.getOtherFactionActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.getFaction() != _actor.getFaction());
}

// edited to grab the tile if none is specified for max convenience

[Enduriel]

Ok, fair, in that case I'd change reorganize the main function so that getNeighboringActors is separate. I don't feel great about having _actor, _tile = null, but to be fair I'm not really sure how to organize that differently

q.getActorsWithinRange <- function( _tile, _max = 1, _min = 1)
{
	// add validation
	if (_max == 1)
	{
		local ret = this.getNeighboringActors(_tile);
		if (_min == 0 && _tile.IsOccupiedByActor)
			ret.push(_tile);
		return ret;
	}
	else
	{
		return this.getActorsByFunction(function(_actor) {
			if (!_actor.isPlacedOnMap())
				return false;
			local distance = _tile.getDistanceTo(_actor.getTile());
			return (distance <= _max && distance >= _min)
		})
	}
}

q.getNeighboringActors <- function ( _tile ) {
	local ret = [];
	for (local i = 0; i < 6; i++)
	{
		if (!_tile.hasNextTile(i))
			continue;

		local nextTile = _tile.getNextTile(i);
		if (nextTile.IsOccupiedByActor)
			ret.push(nextTile.getEntity());
	}
	return ret;
}
q.getAlliedActorsWithinRange <- function( _tile , _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.isAlliedWith(_actor));
}
q.getHostileActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) !a.isAlliedWith(_actor));
}
q.getSameFactionActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.getFaction() == _actor.getFaction());
}
q.getOtherFactionActorsWithinRange <- function(_actor, _tile = null, _max = 1, _min = 1)
{
	return this.getActorsWithinRange(_tile == null ? _actor.getTile() : _tile, _max, _min).filter(@(_, a) a.getFaction() != _actor.getFaction());
}

[TaroEld]

getAlliedActorsWithinRange lacks the _actor parameter and null default value for the tile, otherwise LGTM. Also yes, I understand, but I think this still provides the path of least resistance for most applications.

[Darxo]

I prefer a version where it is apparent from the function name, that it gathers the actors at a distance of 1 tile, like q.getNeighboringActors <- function ( _tile ) from enduriels post.

Otherwise it will look in code like this: [...].getActorsWithinRange(actor.getTile())
And it's not clear what range that exactly is for someone unfamiliar with that msu function.
Adjacent actors is one likely interpretation but the range of the currently equipped weapons default attack or the viewing distance of that actor are not too far off.

[TaroEld]

With Endys version, we'll have the best of both worlds, the clearly specific neighbours and the more generic range option.

[LordMidas]

I have updated the PR with the improved functionality of within range. I don't think we need to add individual functions for each faction type - that can be easily done by the modder by using .filter on the output of these two functions getActorsWithinRange and getAdjacentActors.

Question 1 : Should we allow passing a function as a parameter into getActorsWithinRange so that the filtering happens much more efficiently i.e. the first time the actors are iterated upon rather than filtering them out later? So:

getActorsWithinRange(myTile, 5, 1, @(a) a.isAlliedWith(myActor));

// instead of the current:
getActorsWithinRange(myTile, 5, 1).filter(@(a) a.isAlliedWith(myActor));

Question 2: Should the default value of _min be 0? I think that makes more sense in terms of how vanilla works with tiles.

[TaroEld]

I'd not add the function parameter.

[Suor]

There is Tactical.queryActorsInRange() already, why not use that?

[TaroEld]

There is Tactical.queryActorsInRange() already, why not use that?

Yes, we too realized that, I think the final conclusion was 'ok we dont need this'.

[LordMidas]

Yeah these were created before we realized that the vanilla function exists. However, we still need to optimize our existing functions especially for the case of adjacent actors. Also a new function to get adjacent actors like getAdjacentActors is a compelling addition as that is a very commonly accessed situation.

Also, the vanilla function is slower than our implementation of getActorsByFunction (especially when there's fewer actors on the battlefield). But that's a minor point. Nevertheless we need to optimize our existing functions for the adjacency case.

[Suor]

Vanilla function lacks usability too.