AI Caravan unit tries to move to its own (not home) city

[iegik]

Relates to #98

https://github.com/fire-eggs/CivOne/blob/0b6e2ded039f8122f0e7119988ad2df64e40e8d2/src/Units/Caravan.cs#L91

Fail in infinite loop

[iegik]

Please confirm, that logic should be following:

        // AI: Build wonder if city building it now or establish trade route when it is a destination target.
        // Otherwise move forward.

[axx0]

There are a bunch of if statements and a return true; at the end of internal override bool Confront(int relX, int relY) method. Does it reach this and keep looping? Can you print the status of city during this loop?

    // AI: Build wonder if city building it now or establish trade route when it is a destination target.
    // Otherwise move forward.

By AI do you mean enemy city? You should only help building wonder in your own city.

[iegik]

@axx0 In this particular case, AI Caravan (enemy's Caravan) reaches its own city (not caravans's Home city), where distance is 10 tiles longer from home city (can establish the route).

But I didn't see such case in the current code. Neither establish the trade route, neither help build the wonder (if it exists in the city) or pass by to the final destination.

https://github.com/fire-eggs/CivOne/blob/19eef7d8e9250afcb46fc0be629fcf2dd107c51f/src/Units/Caravan.cs#L103

The code reaches return true; every time and every time it exits from 0..1000 loop instantly, so on next move same pattern is repeating.

https://github.com/fire-eggs/CivOne/blob/19eef7d8e9250afcb46fc0be629fcf2dd107c51f/src/AI.cs#L122

This happens, because on the turn game searches first available unit who has free moves and it again - same Caravan.

This referrers to the #159

TL;DR; https://github.com/fire-eggs/CivOne/blob/19eef7d8e9250afcb46fc0be629fcf2dd107c51f/src/Game.cs#L507

[axx0]

@axx0 In this particular case, AI Caravan (enemy's Caravan) reaches its own city (not caravans's Home city), where distance is 10 tiles longer from home city (can establish the route).

But I didn't see such case in the current code. Neither establish the trade route, neither help build the wonder (if it exists in the city) or pass by to the final destination.

That's because a caravan can establish trade route & help building wonder only if it's human. Notice Game.Human == Owner in https://github.com/fire-eggs/CivOne/blob/19eef7d8e9250afcb46fc0be629fcf2dd107c51f/src/Units/Caravan.cs#L79

AI caravan may only establish a trade route and cannot help building a wonder.

The code reaches return true; every time and every time it exits from 0..1000 loop instantly, so on next move same pattern is repeating.

The 0...1000 for loop is fishy and seems wasteful. We should probably make a List of available AI units and go on from there.

Also I don't ever remember seeing enemy caravan units in the original. According to this: "When the AI builds a caravan it doesn't move it on the map to establish a trade route like a human player, but the caravan gets immediately teleported to the current best trade city in the world (that isn't human controlled). Even if the AI that owns the caravan has no contact with that city yet. But the AI does indeed build caravans and you can see that in game replays: 'somebody builds first caravan', and that somebody isn't you. "

So should we stay faithful to the original and implement this or go the path where AI caravans actually move through terrain?

[iegik]

That's because a caravan can establish trade route & help building wonder only if it's human. Notice Game.Human == Owner in

Yes, that I wanna try to say and figure out how to do that. Only want to check some cases.

The 0...1000 for loop is fishy and seems wasteful

agree.

So should we stay faithful to the original and implement this or go the path where AI caravans actually move through terrain?

Personally, I would like to see caravans in the desert, and have ability to rob them and take money form enemies (and spoil politic relations). But as the main goal of this project is to be more similar to original I assume that the best choice - stay faithful to the original and implement this.

Custom solution we can implement as optional plugin.

[axx0]

Personally, I would like to see caravans in the desert, and have ability to rob them and take money form enemies (and spoil politic relations). But as the main goal of this project is to be more similar to original I assume that the best choice - stay faithful to the original and implement this.

Custom solution we can implement as optional plugin.

I also agree, since that is the way the project was laid out. So in this case the current AI caravan logic should be commented out and instead replaced by:

1. Enemy building a caravan
2. Immediately establishing trade route (With which city? I'm not sure if this was ever disassembled, but for start there should be some randomness to this.)