I do disagree because we're checking for the current unit (this) which is the unit that's moving. The case you're talking about is handled before, with this: if (!pLoopUnit->canCoexistWithEnemyUnit(getTeam()))
canCoexistWithEnemyUnit check if a unit is invisible (and only that).
There's also this in CvUnit::canCreateUnit.
if (plot()->isVisibleEnemyUnit(getOwnerINLINE())) // keeps invisible units from CtDing summoning on top of enemies
I can't reproduce this CtDing but I ignored it and had a mage summon a unit under influence of nox noctis on top of an enemy unit.
*Using the previous code the enemy was moved. Totally unnecessary as the summoned unit would also be invisible on the plot.
*Using my code the enemy stayed in place, as it should.
Might still want that check or a similar one, to prevent an invisible unit summoning non-invisible units, thus moving whole enemy stacks. Not sure if it's possible in FfH 2 but if it is and is repeatable it would become a major abuse.
I have not managed to get the game to crash. I have however managed to get it to move units when it shouldn't, which is enough to call it a bug. To easily see this in action use the ctrl + shift + left click to place a unit on a plot where it would be invisible (like by nox noctis). Without my code change the enemies are moved. With it they are not.
Looking over the code more closely I see that a CtD could occur, but I think only with alwaysInvisible units doing one of:
moving into a city
moving into a plot whos owner can see invisible.
(see CvUnit::isInvisible)
This I consider to be a totally different bug. alwaysInvisible should always be invisible - that's what the code intended. As there are no alwaysInvisible units in FfH 2 at all it's a non-issue except possibly in modmods.
Ps CvUnit::changeInvisibleFromPromotion doesn't take into account if we're invisible by other means. This could even kill the unit if no valid withdrawal location is found.
)
.
