Ticket #42518

AI: Sea caravans accumulate in unused loads

Open Date: 2021-06-12 06:38 Last Update: 2021-09-08 08:35

Reporter:
Owner:
Type:
Status:
Closed
Component:
MileStone:
Priority:
5 - Medium
Severity:
5 - Medium
Resolution:
Fixed
File:
6

Details

Merchant Ships in Aviation ruleset are units that can only enter marketplace. The trade routes are established by flying caravans and they do in autogame, but Merchants are seen just sitting in ports.

Ticket History (3/20 Histories)

2021-06-12 06:38 Updated by: ihnatus
  • New Ticket "AI: Sea caravans accumulate in unused loads" created
2021-06-21 07:35 Updated by: ihnatus
Comment

There are two problems I see in the code:

  • AI thinks caravans need a boat if the cities are on different continents, and
  • when the best caravan's target is looked for, the "windfall" one-time bonus is ignored if the caravan can't establish trade routes (that is the main case why the ships don't move).

Maybe the former problem requires a separate ticket. Also, it should be checked if caravan bonus kind (gold and/or science) is correctly accounted by the AI.

EDIT: checked it, we need to handle both problems to make them moving, or a "caravan" that found no ferry will just get stuck in a nearest city. My current solution includes checking that the target and the caravan's current position are adjacent to a same continent navigable by the unit due to its class adv.sea_move/land_move==MOVE_FULL (then no need in a ferry). Probably we need to support a) sea caravans, b) caravans of a partial move kind carried by e.g. land carriers (we have things like this in Alien World), c) caravans that won't go to another continent in a reasonable time. The current algorithm refuses to assess travel time of a caravan if there is a city on another continent to trade, I'll try to fix it for case a) at least before publishing a patch.

(Edited, 2021-06-22 06:10 Updated by: ihnatus)
2021-06-25 07:15 Updated by: ihnatus
Comment

Fixed for 3.0. The patch is splitted, the naval part goes to bug #42567 and the caravan bonus value part is here.

EDIT: oww, the patch contained wrong logic (while working in this specific case). We should keep current behaviour in 2.6 for caravans that can ever establish trade route (but hmm what if routes can work only with one city per civ... well, it's another problem). In 3.x we probably should only slightly pessimize entering marketplace since we have a formula that is intended to evaluate both. Patches a bit later.

(Edited, 2021-06-26 01:31 Updated by: ihnatus)
2021-06-25 07:17 Updated by: ihnatus
  • File 3.0-marketplace-caravans.patch (File ID: 7127) is attached
2021-06-26 01:25 Updated by: ihnatus
  • File 3.0-marketplace-caravans.patch (File ID: 7127) is deleted
2021-06-26 08:43 Updated by: ihnatus
Comment

This is an improved patch. The variant when a caravan pays for itself by one-rime bonus is so rare that I think it can even go like this into 2.6. Maybe the idea itself needs improvement but it's for future patches. Also, probably yet another ticket should be opened to allow AI select between several available caravan types (now it orients on roles and the action, age of Merchant Ships in Aviation is explained by that Zep is obsoleted by a non-caravan unit). Also, the caravan code needs at least one bugfix and other cleanup.

2021-06-27 18:36 Updated by: ihnatus
Comment

hmm... after a good consideration, we don't need to increase caravan cost due to production time when it is already produced... And for 2.6 we should probably totally keep current behaviour for normal caravans.

2021-07-21 04:27 Updated by: ihnatus
Comment

3.0-only-marketplace-caravans.patch works for all 3.x branches. Over those two patches, a patch in #42641 is developed.

2021-07-21 04:28 Updated by: ihnatus
2021-07-24 07:12 Updated by: None
Comment

Oops, newer versions require city for unit cost estimation. I supply src, seems to work.

2021-09-01 16:16 Updated by: cazfi
Comment

S3_0 version does not compile.

../../../../src/common/aicore/caravan.c: In function ‘get_discounted_reward’:

../../../../src/common/aicore/caravan.c:546:7: error: suggest parentheses around ‘&&’ within ‘||’ -Werror=parentheses

540 | || (consider_windfall
|
541 | && (!parameter->consider_trade /* can only enter marketplaces */
|
542 | /* (FIXME: test any extra restrictions for trade routes) */
| ~
543 | /* or the result is so big that we are still in plus */
| ~
544 | /* (consider having produced IF_GOLD instead) */
|
545 | || windfall >= cost))
| ~
546 | && trade + windfall >= wonder) {
|

../../../../src/common/aicore/caravan.c:546:37: error: expected ‘)’ before ‘{’ token

546 | && trade + windfall >= wonder) {
| ~
| )

../../../../src/common/aicore/caravan.c:539:6: note: to match this ‘(’

539 | if ((consider_trade
|

../../../../src/common/aicore/caravan.c:562:1: error: expected expression before ‘}’ token

562 | }
|

../../../../src/common/aicore/caravan.c:562:1: error: control reaches end of non-void function -Werror=return-type

562 | }
|
2021-09-06 05:32 Updated by: None
Comment

Oh yes, did not test after correcting *'_'* Here is the right 3.0 patch.

2021-09-06 11:09 Updated by: cazfi
  • Owner Update from (None) to cazfi
  • Resolution Update from None to Accepted
2021-09-08 08:35 Updated by: cazfi
  • Status Update from Open to Closed
  • Resolution Update from Accepted to Fixed

Edit

You are not logged in. I you are not logged in, your comment will be treated as an anonymous post. » Login