Looping to check for coasts doesn't work quite right....

Afforess

Afforess

The White Wizard
Joined
Jul 31, 2007
Messages
12,239
Location
Austin, Texas
I created (borrowed, really) a loop that checks to make sure a city is near water, and the water is not a lake. (Basically, the city needs to be on the coast.)

It works. Kinda. If the city isn't on any water at all, it gets ignored. But if a city is next to a 1 tile lake, but still inland, the loop works, when it shouldn't. I'm sure this is some stupid oversight on my part.

Code: 
void CvUnit::tradeUnit(PlayerTypes eReceivingPlayer)
{
    CvUnit* pTradeUnit;
    CvWString szBuffer;
    CvCity* pCapitalCity;
    CvArea* pWaterArea = NULL;
    PlayerTypes eOwner;
    int iLoop;
    
    eOwner = getOwnerINLINE();
    
    if (eReceivingPlayer != NO_PLAYER)
    {
        pCapitalCity = GET_PLAYER(eReceivingPlayer).getCapitalCity();
        
        pTradeUnit = GET_PLAYER(eReceivingPlayer).initUnit(getUnitType(), pCapitalCity->getX_INLINE(), pCapitalCity->getY_INLINE(), AI_getUnitAIType());
        
  [COLOR="Blue"]      if (pTradeUnit->getDomainType() == DOMAIN_SEA)
        //Find the first coastal city, and put the ship there
        {
            for (CvCity* pLoopCity = GET_PLAYER(eReceivingPlayer).firstCity(&iLoop); pLoopCity != NULL; pLoopCity = GET_PLAYER(eReceivingPlayer).nextCity(&iLoop))
            {
                if (((pWaterArea = pLoopCity->waterArea()) != NULL && !pWaterArea->isLake()))
                {
                    pTradeUnit = GET_PLAYER(eReceivingPlayer).initUnit(getUnitType(), pLoopCity->getX_INLINE(), pLoopCity->getY_INLINE(), AI_getUnitAIType());
                    break;
                }
            }
        }
[/COLOR]
        pTradeUnit->convert(this);
        
         szBuffer = gDLL->getText("TXT_KEY_MISC_TRADED_UNIT_TO_YOU", GET_PLAYER(eOwner).getNameKey(), pTradeUnit->getNameKey());
         gDLL->getInterfaceIFace()->addMessage(pTradeUnit->getOwnerINLINE(), false, GC.getEVENT_MESSAGE_TIME(), szBuffer, "AS2D_UNITGIFTED", MESSAGE_TYPE_INFO, pTradeUnit->getButton(), (ColorTypes)GC.getInfoTypeForString("COLOR_WHITE"), pTradeUnit->getX_INLINE(), pTradeUnit->getY_INLINE(), true, true);
         
     }
}
 
you can use bool CvCity::isCoastal(int iMinWaterSize) const
IIRC the maximum size for a lake is defined globally in the XML.
 
Sephi said:
you can use bool CvCity::isCoastal(int iMinWaterSize) const
IIRC the maximum size for a lake is defined globally in the XML.
Click to expand...

Hmm...

I changed this line
Code: 
 if (((pWaterArea = pLoopCity->waterArea()) != NULL && !pWaterArea->isLake()))
to this:
Code: 
if (pLoopCity->isCoastal(GC.getMIN_WATER_SIZE_FOR_OCEAN()))

But it is still having the same effect. The value in the global defines is 10, BTW. The city next to an inland lake is still being targeted.
 
how big is the lake?

edit: how about GC.getLAKE_MAX_AREA_SIZE()? (though it is the same used by the isLake() function you use, so probably won't help much either)
 
Sephi said:
how big is the lake?
Click to expand...
1 tile. It's just one inland coastal tile next to the capital. The rest of the land is plains or grass.

Screenshot:
Spoiler :
Civ4ScreenShot0123.JPG
 
in your code you create a unit in the Capital before you do any checks for lakes. is that intended?
 
Sephi said:
in your code you create a unit in the Capital before you do any checks for lakes. is that intended?
Click to expand...

By default, the unit should go to the capital, but if the capital isn't on the coast, and the unit is a ship, it should go to the first coastal city the game finds.
 
But you don't remove the first unit after you've found a coast (okay, that's not the reason for the problem).

But have you checked, if pWaterArea = pLoopCity->waterArea() is really NULL here?
Because ships can enter cities, so maybe the city square is a water area itself, when it is connected to water somehow, and the lake besides the city is another water area.
(just a wild guess)
 
That screenshot is of the capital. If your loop doesn't find a coastal city what happens? Doesn't it put the unit in the capital? Sooo, what's the problem here? Are there coastal cities you are expecting it to find? Given what The_J said, are you sure it's not putting a unit in the capital and in some coastal city, and you're just not noticing the second unit?

An easy way around this is to have a local variable to hold the "best city" for the unit to be placed into. Init it to the capital and set it in the loop when you find a better city:

Code: 
CvCity* pBestCity = pPlayer->getCapital();

for (each city)
    if (pLoopCity is coastal)
        pBestCity = pLoopCity
        break

init unit in pLoopCity
 
EmperorFool said:
That screenshot is of the capital. If your loop doesn't find a coastal city what happens? Doesn't it put the unit in the capital? Sooo, what's the problem here? Are there coastal cities you are expecting it to find? Given what The_J said, are you sure it's not putting a unit in the capital and in some coastal city, and you're just not noticing the second unit?

An easy way around this is to have a local variable to hold the "best city" for the unit to be placed into. Init it to the capital and set it in the loop when you find a better city:

Code: 
CvCity* pBestCity = pPlayer->getCapital();

for (each city)
    if (pLoopCity is coastal)
        pBestCity = pLoopCity
        break

init unit in pLoopCity
Click to expand...

Hmmm... That is possible. I'll try it out, and report back.
 
Wait, I think my trouble here stems from the fact that I need to initialize pTradeUnit to something before I can go checking it's domain.
 
No, you're creating a new unit in the capital based on the traded unit. pTradeUnit is initialized so you can call getDomain() on it. However, you may instead prefer to just use the CvUnitInfo for it's type to determine the domain and initUnit() it only once you've picked the city to receive it.
 
EmperorFool said:
No, you're creating a new unit in the capital based on the traded unit. pTradeUnit is initialized so you can call getDomain() on it. However, you may instead prefer to just use the CvUnitInfo for it's type to determine the domain and initUnit() it only once you've picked the city to receive it.
Click to expand...

I'm not sure what you mean. Regardless, the code doesn't work at all like this, I just get a CTD when I try to trade a unit.:

Code: 
void CvUnit::tradeUnit(PlayerTypes eReceivingPlayer)
{
    CvUnit* pTradeUnit;
    CvWString szBuffer;
    CvCity* pBestCity;
    //CvArea* pWaterArea = NULL;
    PlayerTypes eOwner;
    int iLoop;
    
    eOwner = getOwnerINLINE();
    
    if (eReceivingPlayer != NO_PLAYER)
    {
        pBestCity = GET_PLAYER(eReceivingPlayer).getCapitalCity();
        
        if (pTradeUnit->getDomainType() == DOMAIN_SEA)
        //Find the first coastal city, and put the ship there
        {
            for (CvCity* pLoopCity = GET_PLAYER(eReceivingPlayer).firstCity(&iLoop); pLoopCity != NULL; pLoopCity = GET_PLAYER(eReceivingPlayer).nextCity(&iLoop))
            {
                if (pLoopCity->isCoastal(GC.getMIN_WATER_SIZE_FOR_OCEAN()))
                {
                    pBestCity = pLoopCity;
                    break;
                }
            }
        }

        pTradeUnit->convert(this);
        
        pTradeUnit = GET_PLAYER(eReceivingPlayer).initUnit(getUnitType(), pBestCity->getX_INLINE(), pBestCity->getY_INLINE(), AI_getUnitAIType());
        
         szBuffer = gDLL->getText("TXT_KEY_MISC_TRADED_UNIT_TO_YOU", GET_PLAYER(eOwner).getNameKey(), pTradeUnit->getNameKey());
         gDLL->getInterfaceIFace()->addMessage(pTradeUnit->getOwnerINLINE(), false, GC.getEVENT_MESSAGE_TIME(), szBuffer, "AS2D_UNITGIFTED", MESSAGE_TYPE_INFO, pTradeUnit->getButton(), (ColorTypes)GC.getInfoTypeForString("COLOR_WHITE"), pTradeUnit->getX_INLINE(), pTradeUnit->getY_INLINE(), true, true);
         
     }
}
 
Isn't this function being called on the unit that's about to be traded? Just call getDomainType() directly:

Code: 
if (getDomainType() == DOMAIN_SEA)

Then you need to initUnit() a new unit for pTradeUnit in the correct city (after finding the correct city) and convert it to this unit.
 
You must log in or register to reply here.

Similar threads

M
AI giving a city; the inner mechanism.
Replies
4
Views
1K
Manco Capac
M
OrionVeteran
Kick Naval Unit Out of City
Replies
1
Views
853
OrionVeteran
OrionVeteran
S
Missionary XP's
Replies
14
Views
2K
stolenrays
S
Afforess
Troubles with AI diplomacy code
Replies
0
Views
1K
Afforess
Afforess
G
Liberating, trading and gifting cities
2
Replies
22
Views
2K
Padjur
P
Top Bottom