LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
286 stars 159 forks source link

Naval units starting out embarked. #1317

Closed LoneGazebo closed 8 years ago

LoneGazebo commented 8 years ago

@ilteroi,

Sometimes, when a city finishes a naval unit, it starts embarked, and there's no easy way to 'disembark' it. Might have something to do with hills, maybe? Seems related to new naval unit city/canal change.

Iamblichos commented 8 years ago

Only way to get them to "disembark" is to put them all out in the water and reload save.

ilteroi commented 8 years ago

will check tonight.

other news:

i've seen at least 30 captures in the last turns here:

image

ilteroi commented 8 years ago

which reminds me, in another game they even razed an original capital after some back and forth, don't think that's supposed to happen

sorry for the hijacking ... and notice sukhothai building a university in the middle of all this image

Iamblichos commented 8 years ago

@LoneGazebo It doesn't seem to matter if the city that trained unit is a hill, flatland, next to a river or not, everyone I build comes out embarked.

LoneGazebo commented 8 years ago

@ilteroi, I fixed that flip flop issue in current release. Was a civilian issue.

LoneGazebo commented 8 years ago

Thankfully the embark thing seems cosmetic.

Iamblichos commented 8 years ago

...until your Polynesia or Denmark and your Destroyers look like Ancient Tech boats! :)~

LoneGazebo commented 8 years ago

Hehe. Yeah, it's not ideal, but it also doesn't seem to affect their efficacy, so gameplay is unaffected. What an odd bug!

Iamblichos commented 8 years ago
#if defined(MOD_BALANCE_EMBARKED_SHIPS)
    if (pNewPlot && getDomainType()==DOMAIN_SEA && IsCombatUnit())
    {
        if (pNewPlot->isFriendlyCityOrPassableImprovement(getOwner()))
            setEmbarked(true);
        else
            setEmbarked(false);
    }
#endif

in CvUnit::setXY

LoneGazebo commented 8 years ago

Does that fix it? I haven't looked at the code yet.

Iamblichos commented 8 years ago

No that just is the code that causes it I believe.

LoneGazebo commented 8 years ago

I think I've fixed it, @ilteroi. Since ships weren't made available for 'caneverembark,' they were never viable for toggling the embark/disembark flip. I've added that, and now it 'flips' from embarked to disembarked once you move it into a city or passable improvement.

LoneGazebo commented 8 years ago

@ilteroi, In response to:

The first issue does seem better now. The second, though, is still a little iffy in some situations. Naval operations seem to have regressed somewhat, as the AI seems content to throw melee ships at cities until they die. It also seems to have a hard time organizing navies for sneak operations (and/or proper assaults on cities). I'm running archipelago maps to see if I can track it down. I feel like we had navies in a good spot a few months back, so I don't see where we messed up.

Iamblichos commented 8 years ago

did you edit this code:

bool CvUnit::CanEverEmbark() const
{
    VALIDATE_OBJECT

    return (getDomainType() == DOMAIN_LAND && IsHasEmbarkAbility() && !IsHoveringUnit() && !canMoveAllTerrain() && !isCargo() );
}

Did you just add an || for DOMAIN_SEA?

LoneGazebo commented 8 years ago

Yep. Though it didn't fix it all, sadly, as cargo ships still show as embarked, regardless of location.

LoneGazebo commented 8 years ago

I also edited:

bool CvPlot::needsEmbarkation(const CvUnit* pUnit) const
{
    if (pUnit==NULL)
        return isWater() && !isIce() && !IsAllowsWalkWater();
    else
    {
        //only land units need to embark
        if (pUnit->getDomainType()==DOMAIN_LAND)
        {
            return isWater() && !isIce() && !IsAllowsWalkWater() && !pUnit->canMoveAllTerrain() && !pUnit->canMoveImpassable() && !pUnit->canLoad(*this);
        }
#if defined(MOD_BALANCE_EMBARKED_SHIPS)
        else if(pUnit->getDomainType() == DOMAIN_SEA)
        {
            return isCityOrPassableImprovement(pUnit->getOwner(), false);
        }
#endif
        else
            return false;
    }
}
Iamblichos commented 8 years ago

Regarding cargo ships, strange as the above code in setXY checks for combat unit.

LoneGazebo commented 8 years ago

Can you reproduce that? They use the embarked icon on my end.

Iamblichos commented 8 years ago

Hmmm...I tried your code above and ships (combat units) are still coming out embarked

(I also edited caneverembark)

LoneGazebo commented 8 years ago

Out of cities?

Iamblichos commented 8 years ago

yes. Did you also edit IsHasEmbarkAbility?

LoneGazebo commented 8 years ago

Oh, duh, forgot one more thing for you to change (see below):

//  --------------------------------------------------------------------------------
void CvUnit::move(CvPlot& targetPlot, bool bShow)
{
    VALIDATE_OBJECT
    CvAssert(canMoveOrAttackInto(targetPlot) || isOutOfAttacks());

    CvPlot* pOldPlot = plot();
    CvAssertMsg(pOldPlot, "pOldPlot needs to have a value");

    bool bShouldDeductCost = true;
    int iMoveCost = targetPlot.movementCost(this, plot());

    // we need to get our dis/embarking on
    bool bChangeEmbarkedState = CanEverEmbark() && (targetPlot.needsEmbarkation(this) != pOldPlot->needsEmbarkation(this));
    if (bChangeEmbarkedState)
    {
#if defined(MOD_BALANCE_EMBARKED_SHIPS)
        if(isEmbarked() && getDomainType() != DOMAIN_SEA && !targetPlot.needsEmbarkation(this)) // moving from water to the land
#else
        if(isEmbarked() && !targetPlot.needsEmbarkation(this)) // moving from water to the land
#endif
        {
            if (m_unitMoveLocs.size())  // If we have some queued moves, execute them now, so that the disembark is done at the proper location visually
                PublishQueuedVisualizationMoves();

            disembark(pOldPlot);

#if defined(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST)
            TeamTypes eUnitTeam = getTeam();
            CvTeam& kUnitTeam = GET_TEAM(eUnitTeam);
            UnitHandle pUnit = GET_PLAYER(getOwner()).getUnit(GetID());
            //If city, and player has disembark to city at reduced cost...
            if(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST && targetPlot.isCity() && (targetPlot.getOwner() == getOwner()) && kUnitTeam.isCityNoEmbarkCost())
            {
                if(movesLeft() > (baseMoves(DOMAIN_LAND) * GC.getMOVE_DENOMINATOR()))
                {
                    setMoves(baseMoves(DOMAIN_LAND) * GC.getMOVE_DENOMINATOR());
                }
            }
            else if(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST && targetPlot.isCity() && (targetPlot.getOwner() == getOwner()) && kUnitTeam.isCityLessEmbarkCost())
            {
                changeMoves(-iMoveCost);
            }
            else if(!GET_PLAYER(getOwner()).GetPlayerTraits()->IsEmbarkedToLandFlatCost())
            {
                finishMoves();
            }
#else
            finishMoves();
#endif
        }
        else if(!isEmbarked() && targetPlot.needsEmbarkation(this))  // moving from land to the water
        {
            if (m_unitMoveLocs.size())  // If we have some queued moves, execute them now, so that the embark is done at the proper location visually
                PublishQueuedVisualizationMoves();

            embark(pOldPlot);
#if defined(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST)
            TeamTypes eUnitTeam = getTeam();
            CvTeam& kUnitTeam = GET_TEAM(eUnitTeam);
            UnitHandle pUnit = GET_PLAYER(getOwner()).getUnit(GetID());
            //If city, and player has disembark to city at reduced cost...
            if(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST && pOldPlot->isCity() && (pOldPlot->getOwner() == getOwner()) && kUnitTeam.isCityNoEmbarkCost())
            {
                if(movesLeft() > (baseMoves(DOMAIN_SEA) * GC.getMOVE_DENOMINATOR()))
                {
                    setMoves(baseMoves(DOMAIN_SEA) * GC.getMOVE_DENOMINATOR());
                }
            }
            else if(MOD_BALANCE_CORE_EMBARK_CITY_NO_COST && pOldPlot->isCity() && (pOldPlot->getOwner() == getOwner()) && kUnitTeam.isCityLessEmbarkCost())
            {
                changeMoves(-iMoveCost);
            }
            else
            {
                finishMoves();
            }
#else
            finishMoves();
#endif
            bShouldDeductCost = false;
        }
#if defined(MOD_BALANCE_EMBARKED_SHIPS)
        else if(isEmbarked() && getDomainType() == DOMAIN_SEA && targetPlot.isWater())
        {
            if (m_unitMoveLocs.size())  // If we have some queued moves, execute them now, so that the disembark is done at the proper location visually
                PublishQueuedVisualizationMoves();

            disembark(pOldPlot);
        }
#endif
    }

    if(bShouldDeductCost)
        changeMoves(-iMoveCost);
    setXY(targetPlot.getX(), targetPlot.getY(), true, true, bShow && targetPlot.isVisibleToWatchingHuman(), bShow);
}
Iamblichos commented 8 years ago

looks like we need to edit canheal too eh?

ilteroi commented 8 years ago

so is this a reproducable problem or not?

and it's only about the unit icon, right?

LoneGazebo commented 8 years ago

@ilteroi , no, it's not just cosmetic as I thought, as they can't heal either.

Iamblichos commented 8 years ago

No it's the ship graphics. Since they are built on a plot that is city, and since that passes the isFriendlyCityOrPassableImprovement check, they come out embarked. There's also a heal problem.

ilteroi commented 8 years ago

ah yeah sorry i meant the model, not the icon.

anyway quite the thread you have going here ... let me take a look too

Iamblichos commented 8 years ago

@LoneGazebo , edited CvUnit::move as above, still coming out embarked.

ilteroi commented 8 years ago

maybe it was a bad idea to re-use the embarked state here.

i could just as well make up a new flag for "docked", which inhibits canRangeStrike() and that's it

come to think of it, don't even need a flag for that. simply "if domain==sea and plot!=water: return false" should do it

Iamblichos commented 8 years ago

@ilteroi that's a much easier fix.

Gonna test out right now.

Iamblichos commented 8 years ago

Good to go just needs this somewhere in canRangeStrike().

#if defined(MOD_BALANCE_EMBARKED_SHIPS)
    if(getDomainType() == DOMAIN_SEA && !plot()->isWater())
    {
        return false;
    }
#endif

...and erase the other two (or if you edited anywhere else) instances of #if defined(MOD_BALANCE_EMBARKED_SHIPS).

LoneGazebo commented 8 years ago

Okay, so we'll cut out all the new embarked code except for what @Iamblichos just posted?

LoneGazebo commented 8 years ago

Still leaves the issue of melee boats being able to attack out of cities...

ilteroi commented 8 years ago

yeah just tried it myself. seems to work fine.

regarding melee, that's handled in IsCanAttackWithMove()

LoneGazebo commented 8 years ago

@ilteroi , and that's already handled by the existing code?

Iamblichos commented 8 years ago

Not it's a different function...just needs to be added in there:

let me try this and will report back:

bool CvUnit::IsCanAttackWithMove() const
{
    VALIDATE_OBJECT
    if(!IsCombatUnit())
    {
        return false;
    }
#if defined(MOD_BALANCE_EMBARKED_SHIPS)
    else if(getDomainType() == DOMAIN_SEA && !plot()->isWater())
    {
        return false;
    }
#endif
    else
    {
        return !isOnlyDefensive();
    }
}
Iamblichos commented 8 years ago

Okay that works. One thing to note with the above code. Melee Boats can still attack into forts and cities, but melee boats in forts or cities cannot attack back. Which may be what we want, but just something to be aware of.

LoneGazebo commented 8 years ago

Why would that be a problem?

ilteroi commented 8 years ago

in fact, i just remembered there's a convenient function for this:

bool CvUnit::isNativeDomain(CvPlot*)

but we're heading for a merge conflict here, so i won't touch this for now.

anyway, i thought about whether melee ships make sense at all. so i'd be all for disabling attacks on cities. which would stop them from capturing cities as well.

forts shouldn't be problem because they must be friendly for you to enter them.

Iamblichos commented 8 years ago

@LoneGazebo , it's not just something for users to be aware of.

@ilteroi I'd really really hate to nerf melee boats, Ranged units used to be king, and they are still tough, but I'd hate to see coastal cities not capturable by melee boats.

LoneGazebo commented 8 years ago

Yeah, melee ships have to be able to capture cities, otherwise their utility drops dramatically. I wonder if we could assign damage to naval vessels in a city (like we do with garrisoned units) if melee v. melee occurs?

LoneGazebo commented 8 years ago

We'd be back at pre-G&K utility for naval units if we dropped melee damage.

Iamblichos commented 8 years ago

One of these days we need to play a multiplayer game small map, 3 civs. mano y mano

LoneGazebo commented 8 years ago

haha

LoneGazebo commented 8 years ago

secret: I barely even play Civ these days! Too busy modding. :)

Iamblichos commented 8 years ago

That doesn't mean you aren't good. The more I code, the better I am, I'm about to hit 8000 hours of civ play though some of that is testing, leaving the game on overnight, falling asleep at the game, etc. But I think I got a lot better at the game when I started coding the game and understanding the nuts and bolts of it, which has been about a year and a half now.

LoneGazebo commented 8 years ago

Maybe! I do okay when I do play. :dancer:

Iamblichos commented 8 years ago

a quick question if you set this as not native domain plot will that affect embarked land units attacking cities from water?

LoneGazebo commented 8 years ago

Shouldn't, because land cities are land, and embarked units are native land.

G