LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
289 stars 160 forks source link

CvUnit::isEnemy(...) #9407

Closed ilteroi closed 1 year ago

ilteroi commented 1 year ago

can somebody fix this please? i cannot make sense of it. i believe it's buggy or at least very easy to misuse.

the unit has an owner. there is a given team. the plot has an owner.

who is supposed to be whose enemy? what does getCombatOwner() do?

total mystery but it's used in a lot of places ...

bool CvUnit::isEnemy(TeamTypes eTeam, const CvPlot* pPlot) const
{
    VALIDATE_OBJECT
    if(NULL == pPlot)
    {
        pPlot = plot();
    }

    if(! pPlot)
    {
        return false;
    }
    return (atWar(GET_PLAYER(getCombatOwner(eTeam, *pPlot)).getTeam(), eTeam));
}
LessRekkless commented 1 year ago

Let's take some steps:

CvUnit::getCombatOwner() is the same as CvUnit::getOwner(); it only cares about pPlot when the following is true:

If that's the case, then we check if the unit has the IsAlwaysHostile promotion and is not in a city (this is where pPlot is used). If the unit has the IsAlwaysHostile promotion, then we return BARBARIAN_PLAYER as the owner.

That is:

return (atWar(BARBARIAN_TEAM, eTeam)) \\ always true if eTeam isn't BARBARIAN_TEAM

If the unit in question has the IsAlwaysHostile promotion and isn't targeting a city, then this function returns true, ie. the unit treats every other unit not on its team as an enemy and can attack it.

LessRekkless commented 1 year ago

in effect, getCombatOwner() is asking if, for the purposes of targeting in combat, the unit is effectively should be treated as if it were owned by the Barbarian Player.

ilteroi commented 1 year ago

thanks but this is still confusing as hell ... what is the usecase?

LessRekkless commented 1 year ago

The AlwaysHostile promotion exists in the base BNW code. I assume it was meant to be used in some scenario.

The promotion itself is there as an option to let a player create a unit that can attack other civilizations without having to declare war.

ilteroi commented 1 year ago

ok, it begins to make sense but the naming is still extremely poor and i'm quite sure there are lots of places in the code where this AlwaysHostile/FakeBarbarian flag is not considered ... question is what to do about it though.

edit: i renamed the parameters to make it clearer what's going on, best i can do