Single Player bugs and crashes v38 plus (SVN) - After the 20th of February 2018

[raxo2222]

Could you try changing FREE_CITY_ADJACENT_CULTURE from 1 to 0 in the global defines see if that helps.
The code for that also looks quite relevant.

It looks like it was fix for NPC civs, as they could have problems with one tile start.

bool bFound = false;
    if (GC.getGameINLINE().isOption(GAMEOPTION_PERSONALIZED_MAP) && GET_PLAYER(getOwnerINLINE()).isModderOption(MODDEROPTION_USE_LANDMARK_NAMES))
    {
        for (int iI = 0; iI < NUM_CITY_PLOTS_2; iI++)
        {
            CvPlot* pLoopPlot = getCityIndexPlot(iI);
            if (!pLoopPlot->getLandmarkName().empty() && pLoopPlot->getLandmarkType() != NO_LANDMARK)
            {
                setName(pLoopPlot->getLandmarkName());
                if (!getName().empty())
                {
                    bFound = true;
                    break;
                }
            }
        }
    }
    if (!bFound)
/************************************************************************************************/
/* Afforess                         END                                                            */
/************************************************************************************************/
------------------------------------------------------------------------------------------------------------------
    setName(GET_PLAYER(getOwnerINLINE()).getNewCityName());

    setEverOwned(getOwnerINLINE(), true);

    pPlot->setImprovementType(NO_IMPROVEMENT);
    pPlot->setOwner(getOwnerINLINE(), bBumpUnits, false);
    pPlot->setPlotCity(this);

    updateCultureLevel(false);
----------------------------------------------------------------------------------------------------------
Should be this ^ in {} brackets at least for multiline entry?

    if (pPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_CULTURE"))
    {
        pPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_CULTURE"), bBumpUnits, false);
    }

    for (iI = 0; iI < NUM_DIRECTION_TYPES; iI++)
    {
        pAdjacentPlot = plotDirection(getX_INLINE(), getY_INLINE(), ((DirectionTypes)iI));

        if (pAdjacentPlot != NULL)
        {
            if (pAdjacentPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"))
            {
                pAdjacentPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"), bBumpUnits, false);
            }
            pAdjacentPlot->updateCulture(bBumpUnits, false);
        }
    }

This thing starts on 319th line in CvCity.cpp

 

[Usul13]

Ok, I didn't know c++ could have this syntax:

if A
B = A; // inside if
D = B; // not inside if

Kinda confusing that there are no indents or any other markers to show that "D = B" is not inside the "if" statement.

It's actually

if (A) B=A;

After the if statement there can be any number of whitespace characters. The new line is just a whitespace character like a space or a tab for example. Also the rounded brackets are mandatory.

 

[Toffer90]

It looks like it was fix for NPC civs, as they could have problems with one tile start.

This thing starts on 319th line in CvCity.cpp

That's the code that made me think it had something to do with the issue. And how is it meant to help the NPC, the free culture is added to all plots surrounding a city when its founded regardless of what team that owns the city.

 

[raxo2222]

That's the code that made me think it had something to do with the issue. And how is it meant to help the NPC, the free culture is added to all plots surrounding a city when its founded regardless of what team that owns the city.

I was guessing before I looked into code - probably that was meant to be added and it never was added.

 

[JosEPh_II]

I can assure you that no civic XML could possibly cause this phenomena.

Yeah I just got done verifying that.

Could you try changing FREE_CITY_ADJACENT_CULTURE from 1 to 0 in the global defines see if that helps.
The code for that also looks quite relevant.

I will do so.

 

[JosEPh_II]

That's the code that made me think it had something to do with the issue. And how is it meant to help the NPC, the free culture is added to all plots surrounding a city when its founded regardless of what team that owns the city.

Is this a recent change?

 

[Thunderbrd]

So what options are messing with One Tile City start?
Realistic Culture Spread?
Influence-Driven War (BUG option - can be changed in game trough BUG)?
Fixed Borders?

Or combination of all four means One Tile start can't work as it was intended (city having only one tile on beginning)?
I always play with those three options and One Tile start wasn't working properly for subsequent cities.

I suppose conquered cities on RCS and Fixed Borders would probably work a bit differently for One Tile City start as well based on what I saw. Again, if you turn off fixed borders in that case you'd probably see things work as you'd think they should. Though I wasn't researching anything about One Tile City Start when I was looking into this either.

Promotion - City Raider 3 isn't available for mounted type units. City raider 2 and 4, 5 are available. I think it's a mistake. Either CR3 should be available for mounted, or CR4-5 unavailable.

4-5 are unavailable if 3 is unavailable. I suppose the pedia doesn't understand this naturaly inherited rule huh? That might be something to look into.

Also, a request please - could button for completing trade mission (for trade merchants, caravans, carrack merchants etc. also for Rogue type units (for contraband)) be implemented in a way that when multiple units are selected it would apply for all selected when pressed. Currently when i move my stacks of caravans i have to manually press this button for every unit. Also if you could add hotkey to this button it would be really great.

Don't really know how to do that. Missions are tremendously complex and I've never been able to decipher the code that's allowing this for some of them. As for hotkeys, I THINK there's a way to define a hotkey in the XML but I don't have a map of what hotkeys are what so I don't ever add any in case there's some conflict.

Probably not as this is happening even though no food or extra population are added to the city.

I know. It's a longshot. But if something is strange afoot, I was mostly trying to come up with a theory as to how because it wouldn't be in what you had there.

It's very useful to me to know about this, thanks! I wrote it down so I won't forget.

There's always a chance I'm wrong about what I'm reading in the code as I only did read so far into it but from what I gathered in only a few hours of research was what I explained. Some verification of this would be cool if you find it does behave differently as suspected.

The bold "if" statement has no content, is that even a legal line in dll?

It's saying in natural speak: if the variable bFound is defined as false then consider everything in the brackets that follow. The () is the variable insertion of the if being evaluated. ! means 'not'. So if is not bFound. Or, if bFound is false. If there are no brackets then only the first next line is assumed to be the statement under the if switch, thus setName(GET_PLAYER(getOwnerINLINE()).getNewCityName()); is all that's under that if statement.

It looks like the whole thing is designed to interrupt the normal automatic naming of the city since in the red portion it names the city after the landmark instead.

I don't personally like this form of expression - much prefer it always adhere to using the brackets, even if you don't have to, just to be very clear about it.

Ok, Usul already answered you - so why he's not helping us with bugs in the code I'm not sure...

 

[JosEPh_II]

Could you try changing FREE_CITY_ADJACENT_CULTURE from 1 to 0 in the global defines see if that helps.
The code for that also looks quite relevant.

That did the trick. The cities sre now founding properly with only 1 tile.

EDIT: Same game as before too, with cache cleared and GlobalDefine setting changed. Settings screenshot added with pertinent Options shown.

 

[Usul13]

There's always a chance I'm wrong about what I'm reading in the code as I only did read so far into it but from what I gathered in only a few hours of research was what I explained. Some verification of this would be cool if you find it does behave differently as suspected.

I'll test it and I'll get back to you.

Ok, Usul already answered you - so why he's not helping us with bugs in the code I'm not sure...

Actually I'm already thinking about it, but first I'll have to learn some things about this Civ IV modding business. And I never did any serious coding I just dabble. I think the most serious piece of program I ever wrote was a Barnes-Hut simulation under Unity.

Also my attention span can be relatively short which is not a good thing in a big project like this. I can get bored with things after a couple of weeks or something else can grab my attention so I can't make any promises. We'll see.

 

[JosEPh_II]

Questions: All players that want to use the I City Tile Start Option must now change the GlobalDefine for the line FREE_CITY_ADJACENT_CULTURE from the setting of 1 to "0" correct?
Players that use this Option should also expect for the Barb cities to stay at 1 Tile as well?
Will the NPC player Neanderthal also have it's Cities stay at 1 city tile if the Option Neanderthal Cities is On or Off?

 

[Thunderbrd]

All players that want to use the I City Tile Start Option must now change the GlobalDefine for the line FREE_CITY_ADJACENT_CULTURE from the setting of 1 to "0" correct?

I don't think it's necessarily a bad thing to change it for all games but I'd test that to make sure it doesn't make the same effect if it's not on 1CTS.

Players that use this Option should also expect for the Barb cities to stay at 1 Tile as well?

Have we still not given them some means to pump some culture simply for being barbarians?

Will the NPC player Neanderthal also have it's Cities stay at 1 city tile if the Option Neanderthal Cities is On or Off?

I would think it's fine for both to operate the same way. I'd expect them to start with 1 tile just like any other city but as neanders and barbs they should be getting some bonus culture, even just a little.

 

[Toffer90]

Is this a recent change?

Not at all, the global define and its dll code was identical in vanilla.

Have we still not given them some means to pump some culture simply for being barbarians?

We have.

I would think it's fine for both to operate the same way. I'd expect them to start with 1 tile just like any other city but as neanders and barbs they should be getting some bonus culture, even just a little.

They do.

I don't think it's necessarily a bad thing to change it for all games but I'd test that to make sure it doesn't make the same effect if it's not on 1CTS.

It would have to be tested a bit to ensure we don't introduce any glitches, there may very well be a good technical reason for why vanilla Civ added that free culture around a city when it's founded.

Other than any technical reasons, I think the original intention was that if you settle a city at the edge of another nations border there would be a better chance of getting ownership of the other nations land that was adjacent to the newly founded city.

I don't really like this, why should the colonizer be favoured over those who have owned the land for possibly much longer? I say, if you want to grab anothers land you must fight for it or out-culture your neighbours in the long run.

I'm thinking we should just remove this vanilla code completly:

    if (pPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_CULTURE"))
    {
        pPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_CULTURE"), bBumpUnits, false);
    }

    for (iI = 0; iI < NUM_DIRECTION_TYPES; iI++)
    {
        pAdjacentPlot = plotDirection(getX_INLINE(), getY_INLINE(), ((DirectionTypes)iI));

        if (pAdjacentPlot != NULL)
        {
            if (pAdjacentPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"))
            {
                pAdjacentPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"), bBumpUnits, false);
            }
            pAdjacentPlot->updateCulture(bBumpUnits, false);
        }
    }

If it doesn't cause any bugs that is.

Before that code there is this line: updateCultureLevel(false);
I'm pretty sure that handles all the culture related stuff when a city is founded.

 

[Thunderbrd]

Not at all, the global define and its dll code was identical in vanilla.
We have.
They do.
It would have to be tested a bit to ensure we don't introduce any glitches, there may very well be a good technical reason for why vanilla Civ added that free culture around a city when it's founded.

Other than any technical reasons, I think the original intention was that if you settle a city at the edge of another nations border there would be a better chance of getting ownership of the other nations land that was adjacent to the newly founded city.

I don't really like this, why should the colonizer be favoured over those who have owned the land for possibly much longer? I say, if you want to grab anothers land you must fight for it or out-culture your neighbours in the long run.

I'm thinking we should just remove this vanilla code completly:

    if (pPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_CULTURE"))
    {
        pPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_CULTURE"), bBumpUnits, false);
    }

    for (iI = 0; iI < NUM_DIRECTION_TYPES; iI++)
    {
        pAdjacentPlot = plotDirection(getX_INLINE(), getY_INLINE(), ((DirectionTypes)iI));

        if (pAdjacentPlot != NULL)
        {
            if (pAdjacentPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"))
            {
                pAdjacentPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_ADJACENT_CULTURE"), bBumpUnits, false);
            }
            pAdjacentPlot->updateCulture(bBumpUnits, false);
        }
    }

If it doesn't cause any bugs that is.

Before that code there is this line: updateCultureLevel(false);
I'm pretty sure that handles all the culture related stuff when a city is founded.

I don't think it is designed to boot another civ from those tiles so much as it is to establish a minimum amount of claiming culture there on founding. The first segment appears to be for the plot where the city is founded. It might be best, actually, if I go in and make the Free City Adjacent Culture call here depend on whether the 1CTS option is on or not rather than working with any global define that isn't really necessary given how unlikely it is to need further definition than 0 or 1. I suspect new cities without 1CTS may be founding without their first rung of tiles - at least for one round - if this global is turned to 0.

 

[makotech222]

@Thunderbrd Sorry I've been away for a while, just noticed my tags. It looks like you got everything working again? Would probably be good to document what you did in case anyone else has similar issues?

 

[Thunderbrd]

@Thunderbrd Sorry I've been away for a while, just noticed my tags. It looks like you got everything working again? Would probably be good to document what you did in case anyone else has similar issues?

There have been a few things - some help files I found helped me sort out some other stuff too. But I'm still having some annoying errors with it that I haven't been able to get rid of. I figured I should write a guide on this yes and should do it soon if I can remember the steps.

 

[Toffer90]

I suspect new cities without 1CTS may be founding without their first rung of tiles - at least for one round - if this global is turned to 0.

We could just add a gameoption switch there as you say, but...

I did an in game test without the 1CTS option after setting both those global defines to zero.
FREE_CITY_CULTURE = 0
FREE_CITY_ADJACENT_CULTURE = 0

I founded a city and it only got the center tile, it would go from poor to fledgling in 4 turns (Ultrafast GS).
The next turn it got the first rung at the start of the turn, still the same culturelevel as it was last turn.

I actually think it's how it should be when playing without the 1CTS, that the city is extra vulnerable the same turn it is founded and only gets that first rung after the other players have had their turn.
I still vote that we remove the two globals and the dll code for them, what it does does not justify the extra globals and codelines imo.

 

[Morlark]

I suppose conquered cities on RCS and Fixed Borders would probably work a bit differently for One Tile City start as well based on what I saw. Again, if you turn off fixed borders in that case you'd probably see things work as you'd think they should. Though I wasn't researching anything about One Tile City Start when I was looking into this either.

Well that explains the behaviour I just saw. I conquered a few cities (with RCS on) and they're just expanding borders in rings, as if RCS is off. Came here to report it, and it looks like it's already the big topic of discussion this week.

 

[Thunderbrd]

I still vote that we remove the two globals and the dll code for them, what it does does not justify the extra globals and codelines imo.

I would be fine with the global for the first rung being set to 0 but I think it would be problematic if the city didn't demand itself to at least be claimed. And I see no cause to remove code over it since opinions may change later. I'd say starvation would be an issue but it shouldn't be now with the added food if you have more pop.

 

[Toffer90]

I would be fine with the global for the first rung being set to 0 but I think it would be problematic if the city didn't demand itself to at least be claimed..

It already does demand that the center tile is claimed without the global define helping.

This line comes before any of that code.
pPlot->setOwner(getOwnerINLINE(), bBumpUnits, false);

If we use the switch as you want then I would still argue that we remove the global define.

if (!GC.getGameINLINE().isOption(GAMEOPTION_1_CITY_TILE_FOUNDING))
{
    for (iI = 0; iI < NUM_DIRECTION_TYPES; iI++)
    {
        pAdjacentPlot = plotDirection(getX_INLINE(), getY_INLINE(), ((DirectionTypes)iI));

        if (pAdjacentPlot != NULL && pAdjacentPlot->getOwnerINLINE() == NO_PLAYER)
        {
            pAdjacentPlot->setOwner(getOwnerINLINE(), bBumpUnits, false);
        }
    }
}

I have no theory as to what the point of the below code is: Edit: I still don't.

    if (pPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_CULTURE"))
    {
        pPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_CULTURE"), bBumpUnits, false);
    }

 

[Thunderbrd]

It already does demand that the center tile is claimed without the global define helping.

This line comes before any of that code.
pPlot->setOwner(getOwnerINLINE(), bBumpUnits, false);

If we use the switch as you want then I would still argue that we remove the global define.

if (!GC.getGameINLINE().isOption(GAMEOPTION_1_CITY_TILE_FOUNDING))
{
    for (iI = 0; iI < NUM_DIRECTION_TYPES; iI++)
    {
        pAdjacentPlot = plotDirection(getX_INLINE(), getY_INLINE(), ((DirectionTypes)iI));

        if (pAdjacentPlot != NULL && pAdjacentPlot->getOwnerINLINE() == NO_PLAYER)
        {
            pAdjacentPlot->setCulture(getOwnerINLINE(), 1, bBumpUnits, false);
            pAdjacentPlot->updateCulture(bBumpUnits, false);
        }
    }
}

I see no reason why we would want a big culture bonus given to the first rung for a freshly founded city. The minimum of 1 is only used to get it claimed right away if it is neutral land.

I have no theory as to what the point of the below code is:

    if (pPlot->getCulture(getOwnerINLINE()) < GC.getDefineINT("FREE_CITY_CULTURE"))
    {
        pPlot->setCulture(getOwnerINLINE(), GC.getDefineINT("FREE_CITY_CULTURE"), bBumpUnits, false);
    }

You can claim a plot for a city but not own it. It just means the city has dibs on it for when it has some culture and the plot is within its ownership range. (A claim is particularly more pertinent between competing cities of the same player or the same team.) The last bit of code is the part that gives it a minimum of 1 culture to that plot where the city was founded. That's the global I wouldn't mess with. I considered code along the lines you suggested, yes, which would then remove the need for that global at all and if you read back, I made that very suggestion. However, I think we could safely turn the FREE_CITY_ADJACENT_CULTURE global to 0 and not have to worry about it further in the slightest. No need to adjust the code at all but if we did want to reverse that then we could more easily react since it's still there to react with, or tell people to manipulate it locally, or whatever. Only if this becomes a bigger issue is it worth a recoding effort.