If it's not the only function that overrides an inherited function, clang will complain. It wants override for either all overriding functions of a class or none.

Wondering if the the ObstructionType should be the argument, and not just the component.

It could be made generic to consume the component, then it could also use the other components,
and perhaps it could receive an optional function as an argument that performs the obstruction-specific checks?

Otherwise if we don't care to make it reusable for other random use cases, the argument could also be removed to add more specializations to the one use case.

I see it's copied code, so it's okay to keep it consistent, otherwise I'd have said that it should complain if it receives broken values instead of pretending it didnt happen.

Okay, the other functions in this file iterate over all gaia or player-owned entities, so this one is faster because it filters sooner.

But you also only want player-owned or (ally-owned too) entities, so you can skip even more entities here, right?

You obtained the obstruction component here, so I wonder if CheckEntityInRect could be replaced with the collision check in the future. Doesn't have to be scheduled at all unless there is data to back it up that it's actually relevant.

Forward declaration ?

Yeah, it makes sense. I suppose we need to update virtual > override in future.

It's a good point to make more common, but I'd prefer to not pass the filter function as an argument. But use templates instead (for simpler compiler optimizations). Like:

PickEntitiesWithComponentInRect<StaticObstructionFilter>(simulation, IID_Obstruction, ...);

But you also only want player-owned or (ally-owned too) entities, so you can skip even more entities here, right?

I thought about that. I think it's not a wrong behaviour to allow to a player to place a building near an ancient temple (which belongs to gaia).

Yes, it's possible to replace, but it requires a more complex collision check.

There are only a typedef and a constant, so no.

Should we really do that, or call the function correctly ?

EDIT: nvm saw @elexis's comment below.

Any reason to call that ?

I suppose no, thank you!

No pb :)

check if not null ?

Not here, if it's needed. We request all entities with such component and it's wrong if we can get nullptr here (that means that an entity doesn't have such component).

Alright.

"[...] with a given component" instead of obstruction, or isn't the below function generic ?

You're right.

Range-for loop