Settlers - irrigation maked on the sea

[iegik]

Reproduce:

• press "I" for the Settlers
• pet Settlers to the Thereme
• on the same turn go with Thereme somewhere

In result we have irrigation on the sea :)

[fire-eggs]

Comparing SWY1985's last versus my current, I believe all the Settler "irrigation" bugs are due to these code changes. They can be found in src\Units\Settlers.cs, function NewTurn(), lines 187-215.

Original code:

    else if (BuildingIrrigation > 0)
    {
        BuildingIrrigation--;
        if (BuildingIrrigation > 0)
        {
            MovesLeft = 0;
            PartMoves = 0;
        }
        else if (Map[X, Y] is Forest)
        {
            Map[X, Y].Irrigation = false;
            Map[X, Y].Mine = false;
            Map.ChangeTileType(X, Y, Terrain.Plains);
        }
        else if ((Map[X, Y] is Jungle) || (Map[X, Y] is Swamp))
        {
            Map[X, Y].Irrigation = false;
            Map[X, Y].Mine = false;
            Map.ChangeTileType(X, Y, Terrain.Grassland1);
        }
        else
        {
            Map[X, Y].Irrigation = true;
            Map[X, Y].Mine = false;
        }
    }

My current:

    else if (BuildingIrrigation > 0)
    {
        BuildingIrrigation--;
        //if (BuildingIrrigation > 0)
        //{
        //  MovesLeft = 0;
        //  PartMoves = 0;
        //}
        //else 
              if (Map[X, Y] is Forest)
        {
            Map[X, Y].Irrigation = false;
            Map[X, Y].Mine = false;
            Map.ChangeTileType(X, Y, Terrain.Plains);
        }
        else if ((Map[X, Y] is Jungle) || (Map[X, Y] is Swamp))
        {
            Map[X, Y].Irrigation = false;
            Map[X, Y].Mine = false;
            Map.ChangeTileType(X, Y, Terrain.Grassland1);
        }
        else
        {
            Map[X, Y].Irrigation = true;
            Map[X, Y].Mine = false;
        }
              MovesLeft = 1;
              PartMoves = 0;
          }

By setting MovesLeft to 1, the unit is allowed to move. The original BuildingIrrigation check prevents the unit from moving until irrigation is complete.

To support both the original behavior and my "cheat" behavior, these changes should have a test for the Settings.Instance.AutoSettlers flag.

An initial test might be to restore the original code, and run the game with the 'AutoSettlers' cheat disabled.

If the fix is made and verified for both the "original" and "cheat" settings, then the same fix can be applied for the BuildinMine, BuildingRoad, and BuildingFortress blocks in the same routine.

[iegik]

@fire-eggs Oh, great! I wondered - where this part, where Settlers should miss the turn. Thanks!

[iegik]

@fire-eggs Please, describe the Settings.Instance.AutoSettlers - are the cheat gives for irrigation, road, mine and Fortress to build in only one turn?

[iegik]

Maybe this cheat should override whole NewTurn method?

[iegik]

I think, since irrigation, road, mine and Fortress cannot build at once by one unit - maybe will be better to replace counters with one?

[fire-eggs]

@fire-eggs Please, describe the Settings.Instance.AutoSettlers - are the cheat gives for irrigation, road, mine and Fortress to build in only one turn?

Correct.

[fire-eggs]

I think, since irrigation, road, mine and Fortress cannot build at once by one unit - maybe will be better to replace counters with one?

Please give it a try!

[iegik]

Please give it a try! I've investigated possibility of that change. Seems to be we don't have type of what we try to build https://github.com/fire-eggs/CivOne/blob/dcbb3a2bd43216c34d87331f87f9f8b2a77e28e9/src/Units/UnitExtensions.cs#L28

So, do we want to add some build types?

[iegik]

For example, in BaseUnit we have MovesLeft, so we can add also MovesSkip in the likeness, to count skipped moves

[fire-eggs]

Take 2, because my first attempt to reply was completely wrong.

For example, in BaseUnit we have MovesLeft, so we can add also MovesSkip in the likeness, to count skipped moves

I'd have to ponder this a bit but my gut reaction is this isn't a good idea. E.g. would this mean having to add checks against MovesSkip in every location where MovesLeft is checked? Also, this is Settlers-specific, does it need to go in BaseUnit?

Maybe the Busy property is what handles your MovesSkip logic?

So, do we want to add some build types?

Yes, this is what would need to happen. Some sort of "what is this unit building" property. Some indication that covers None, Road, Irrigation, Mine, Fortress, and Cleaning-Pollution.

[Note that Cleaning-Pollution is missing from the UnitExtensions code you referenced, probably because the current codebase doesn't generate pollution. So that's a bug.]

Having a single counter and some "building" marker is closer to the CIV savefile format than the existing multitude of counters.

You need to look at Settlers.cs : SetStatus and GetStatus which translates the savefile format to/from the existing "Building" properties.

NOTE: I believe you've made me aware of a bug, which is the fact the code does not accurately load/save the "building" state to the savefile. I.e. if you save the game, all current progress is lost (on reload, the progress count is reset). There is a "Special Moves" field in the savefile which is meant to record this [offset 5, according to the format topic in CivFanatics] but I don't see that being used... Needs to be investigated.

[iegik]

@fire-eggs Thanks, already done like that. I have added _order (moved Order enum from Orders to Enums/Order.cs). Also fixed https://github.com/fire-eggs/CivOne/issues/131

[iegik]

https://github.com/fire-eggs/CivOne/pull/141

[fire-eggs]

Fixes integrated from pull request #141