It currently still has Health, but indeed it should have that stripped some day.

The other thing as noted today on IRC was that this function now does 2 rangemanager calls when one over all entities should suffice.

Also it sounds a bit fragile to modify the input argument players, it means you have to check that all current callers pass copies that are not used afterwards and then hope that the future callers also dont use an object they assumed would only be an input argument.

I don't know if its true, but wraitii said the function (today on IRC) is also used for capturing. If that is so, then I assume you want to check for the attack (status effect?) type which component interface ID should be passed to the rangemanager (IID_Health, IID_Capturable, ...).

Ideally one would do a range query over entities which only have the right IID indeed (although that also can lead to strange stuff).
I would vote for one range query, thus taking into account trees as well, however that allows one to "hide" in trees, which is cool but ought to be introduced differently I guess. But that is a design decision ;)

In rP23012#38534, @Freagarach wrote:

Ideally one would do a range query over entities which only have the right IID indeed (although that also can lead to strange stuff).

What strange stuff?

I would vote for one range query, thus taking into account trees as well, however that allows one to "hide" in trees, which is cool but ought to be introduced differently I guess. But that is a design decision ;)

Whut

Why not just merge the two queries and always filter for Health if its an attack and always filter for Capturing if its a capture attack? Trees shouldnt be attacked

In rP23012#38535, @elexis wrote:

What strange stuff?

A projectile could hit an entity farther away from the observed point of impact if the entity which is closer does not have the right IID.

In rP23012#38535, @elexis wrote:

Why not just merge the two queries and always filter for Health if its an attack and always filter for Capturing if its a capture attack? Trees shouldnt be attacked

One would need to pass the IID to this function and do more than one query. I just think it would be difficult. E.g. imagine an arrow which only has a status effect which influences the movement speed of an entity, one would need to check what IIDs can be affected by the attack and pass that to this function.

Also it shouldn't be IID_Health but IID_DamageReceiver which is implemented by IID_Armour, it seems like that changed to IID_Resistance (futile!).

A projectile could hit an entity farther away from the observed point of impact if the entity which is closer does not have the right IID.

It sounds more like a bug than hiding behind a tree, there is a camouflaging ticket and unfinished patch btw.

One would need to pass the IID to this function and do more than one query.

Passing the IID sounds right.
Why do more than one query? If you want to get attackable entities, search for attackable entities, regardless of playerID?

I just think it would be difficult. E.g. imagine an arrow which only has a status effect which influences the movement speed of an entity, one would need to check what IIDs can be affected by the attack and pass that to this function.

Which is one of the things one should think about before implementing, let alone committing that.
But I suppose you want the StatusEffectReceiver IID for that.

In rP23012#38537, @elexis wrote:

Why do more than one query? If you want to get attackable entities, search for attackable entities, regardless of playerID?

This is not about attackable entities, but about entities which can be hit by a stray projectile. But for every IID "in" the projectile one would need a query (or one combined one). I'm not talking about players anymore ;)

In rP23012#38537, @elexis wrote:

But I suppose you want the StatusEffectReceiver IID for that.

Oh yeah, ofcourse ^^

In rP23012#38538, @Freagarach wrote:

In rP23012#38537, @elexis wrote:

Why do more than one query?

For every IID "in" the projectile one would need a query (or one combined one).

I suppose the int requiredInterface in CCmpRangeManager.cpp could become a std::vector<int> if one wants to query multiple IIDs at the same time.
(But should question whether its really needed)

Agreed :)

reported at #5666