/********************************************************************** + Calculates our need for diplomats as defensive units. + May replace values in choice. +***********************************************************************/ +void ai_choose_diplomat_defensive(struct player *pplayer, struct city *pcity, + struct ai_choice *choice, int def) +{ + Unit_Type_id u = best_role_unit(pcity, F_DIPLOMAT); [GB] The above line should move below + + /* Build a diplomat if our city is threatened by enemy diplomats, and + we have other defensive troops, and we don't already have a diplomat + to protect us. If we see an enemy diplomat and we don't have diplomat + tech... race it! */ + if (def != 0 && pcity->ai.diplomat_threat && !pcity->ai.has_diplomat) { [GB] Here! + if (uname); + choice->want = 16000; /* diplomat more important than soldiers */ + pcity->ai.urgency = 1; + choice->type = CT_DEFENDER; + choice->choice = u; + } else if (num_role_units(F_DIPLOMAT) > 0) { + /* We don't know diplomats yet... */ + freelog(LOG_VERBOSE, "A defensive diplomat is wanted badly in city %s.", + pcity->name); + u = get_role_unit(F_DIPLOMAT, 0); + /* 3000 is a just a large number, but not hillariously large as the + previously used one. This is important for diplomacy later - Per */ + pplayer->ai.tech_want[get_unit_type(u)->tech_requirement] += 3000; + } + } +} + +/********************************************************************** + Calculates our need for diplomats as offensive units. + May replace values in choice. [GB] Should mention (maybe as a TODO) that we don't build diplomats to incite. +***********************************************************************/ +void ai_choose_diplomat_offensive(struct player *pplayer, struct city *pcity, + struct ai_choice *choice) +{ + Unit_Type_id u = best_role_unit(pcity, F_DIPLOMAT); + + if (u >= U_LAST) { + /* We don't know diplomats yet! */ + return; + } + + if (ai_handicap(pplayer, H_DIPLOMAT)) { + /* Diplomats are too tough on newbies */ + return; + } + + if ((choice->want >= 100 && choice->type != CT_ATTACKER) + || (pcity->ai.urgency > 0) ) { + /* military_advisor_choose_build does something idiotic, + * this function should not be called if there is danger... */ + return; + } [GB] Above: (1) align the comment please (2) Just for the record. This is what I meant in the comment: even if the house is in danger and CT_DEFENDER is really needed, military_advisor_choose_build seems to proceed happily to choose an attacker to attack some faraway countries. This should change when we clean the m_a_c_b up. + /* Do we have a good reason for building diplomats? */ + { + struct unit_type *ut = get_unit_type(u); + struct city *acity = find_city_to_diplomat(pplayer, pcity->x, + pcity->y, FALSE); + int want, gain, loss, p_success, p_failure, time_to_dest; + + if (acity == NULL || acity->ai.already_considered_for_diplomat) { + /* Found no target or city already considered */ + return; + } + + /* Assuming we are going to steal tech */ + if (city_owner(acity)->research.techs_researched < + pplayer->research.techs_researched) { + gain = total_bulbs_required(pplayer) * TRADE_WEIGHTING; + } else { + gain = 1; /* we don't know how to use calculate incite yet */ + } [GB] I would change the above condition to: if (!has_embassy(pplayer, city_owner(acity)) || (city_owner(acity)->research.techs_researched < pplayer->research.techs_researched)) // If we haven't got an embassy with them, the embassy is at least as // needed, as the technologies we will later steal. Otherwise you won't get an embassy with a retarded civ: you'd exit with negative want, see below. + loss = ut->build_cost * SHIELD_WEIGHTING; + + /* Probability to succeed, assuming no defending diplomat */ + p_success = game.diplchance; + /* Probability to lose our unit */ + p_failure = (unit_type_flag(u, F_SPY) ? 100 - p_success : 100); + + /* FIXME: Use warmap and unit_move_turns */ + time_to_dest = real_map_distance(pcity->x, pcity->y, acity->x, acity->y) + * SINGLE_MOVE / ut->move_rate; + time_to_dest *= time_to_dest; /* No long treks, please */ + + /* Almost kill_desire */ + want = (p_success * gain - p_failure * loss) / 100 + - SHIELD_WEIGHTING * time_to_dest; + + if (want <= 0) { + return; + } Here! + want = military_amortize(want, time_to_dest, ut->build_cost); + + if (!player_has_embassy(pplayer, city_owner(acity))) { + freelog(LOG_VERBOSE, "A diplomat desired in %s to establish an " + "embassy with %s in %s", pcity->name, + city_owner(acity)->name, acity->name); + want = MAX(want, 99); + } + if (want > choice->want) { + freelog(LOG_DEBUG, "%s,%s: %s is desired with want %d (was %d) to spy in %s", + pplayer->name, pcity->name, ut->name, want, choice->want, acity->name); + choice->want = want; + choice->type = CT_ATTACKER; + choice->choice = u; + acity->ai.already_considered_for_diplomat = TRUE; + } + } +} [...] +/************************************************************************** + Find a city to send diplomats against, or NULL if none available on + this continent. x,y are coordinates of diplomat or city which wishes + to build diplomats, and foul is TRUE if diplomat has done something bad + before. +**************************************************************************/ +struct city *find_city_to_diplomat(struct player *pplayer, int x, int y, + bool foul) +{ + bool has_emb; + int oic = 0; /* incite cost */ + int rmd = 0; /* real map distance */ + int dist=MAX(map.xsize, map.ysize); + int continent=map_get_continent(x, y); + bool dipldef; /* whether target is protected by diplomats */ + bool handicap = ai_handicap(pplayer, H_TARGETS); + struct city *ctarget = NULL; + + players_iterate(aplayer) { + /* sneaky way of avoiding foul diplomat capture -AJS */ + has_emb=player_has_embassy(pplayer, aplayer) || foul; Spacing around "=" above (3 counts) + + /* Note: it is possible to lose an embassy to an allied player */ + if (aplayer == pplayer || (pplayers_allied(pplayer,aplayer) && + has_emb)) { [GB] I'll probably put myself in grave_danger of being crucified by Petr if I say that I prefer && to be on the new line... + continue; + } + + city_list_iterate(aplayer->cities, acity) { + struct city *capital = find_palace(city_owner(acity)); + if (handicap && !map_get_known(acity->x, acity->y, pplayer)) { + /* Target is not visible */ + continue; + } + if (continent != map_get_continent(acity->x, acity->y)) { + continue; + } + /* Figure out incite cost */ + city_incite_cost(acity); /* FIXME: remove need for this */ + oic = acity->incite_revolt_cost; + if (pplayer->player_no == acity->original) oic = oic / 2; + rmd = real_map_distance(x, y, acity->x, acity->y); + dipldef = (count_diplomats_on_tile(acity->x, acity->y) > 0); + if (!ctarget || (dist > rmd)) { + if (!has_emb + || (acity->steal == 0 && pplayer->research.techs_researched < + city_owner(acity)->research.techs_researched && !dipldef) + || (oic < (pplayer->economic.gold - pplayer->ai.est_upkeep) + && !government_has_flag(get_gov_pplayer(city_owner(acity)), + G_UNBRIBABLE) + && acity != capital && !dipldef)) { + /* We have the closest enemy city so far on the same continent */ + ctarget = acity; + dist = rmd; + } + } + } city_list_iterate_end; + } players_iterate_end; + return ctarget; +} + +/************************************************************************** + Attempt to find a city that needs reinforcent on this continent. + Currently only used by diplomats. Not tested with other units. - Per +**************************************************************************/ +struct city *find_city_to_defend(struct player *pplayer, int x, int y, + Unit_Type_id utype) [GB] It can be made static, right? +{ + int dist, urgency; + int best_dist = 30; /* any city closer than this is better than none */ + int best_urgency = 0; + struct city *ctarget = NULL; + int continent = map_get_continent(x, y); + + city_list_iterate(pplayer->cities, acy) { [GB] acy is weird. While there are no rules, I'd generally prefer pcity to denote our city and acity an enemy city. + if (continent != map_get_continent(acy->x, acy->y)) continue; + urgency = acy->ai.urgency + 1; + if (unit_type_flag(utype, F_DIPLOMAT)) { + if (!acy->ai.has_diplomat && acy->ai.diplomat_threat) { + urgency *= 5; /* we can help */ + } else if (acy->ai.has_diplomat) { + urgency /= 3; /* we are not really needed there */ + } + } + dist = real_map_distance(x, y, acy->x, acy->y); + /* I don't know if this formula is optimal, but it works. */ + if (dist > best_dist) { + /* punish city for being so far away */ + urgency /= (float)(dist/best_dist); + } + if (urgency > best_urgency) { + ctarget = acy; + best_urgency = urgency; + best_dist = MAX(dist,1); /* squelch divide-by-zero */ + } + } city_list_iterate_end; + return ctarget; +} [...] +/************************************************************************** + If we are the only diplomat in a threatened city, defend against enemy + actions. The passive defense is set by game.diplchance. The active + defense is to bribe units which end their move nearby. Our next trick is + to look for enemy cities on our continent and do our diplomat things. + FIXME: It is important to establish contact with all civilizations, so + we should send diplomats by boat eventually. I just don't know how that + part of the code works, yet - Per +**************************************************************************/ +static void ai_manage_diplomat(struct player *pplayer, struct unit *pdiplomat) +{ + struct packet_diplomat_action dact; + struct city *ctarget = map_get_city(pdiplomat->x, pdiplomat->y); [GB] ctarget?? it confused me for a while. I don't think you should reuse variables, leave it to the compiler. + Unit_Type_id sanity = pdiplomat->id; + bool broken_goto = FALSE; + + assert((pplayer != NULL) && (pdiplomat != NULL)); + + /* Look for someone to bribe */ + broken_goto = ai_diplomat_bribe_adjacent(pplayer, pdiplomat, ctarget); + + /* Sanity check */ + if (find_unit_by_id(sanity)==NULL) { return; } [GB] spacing around "==" And I think we should have a special amendment to the style guide about oneliners like if (blah) continue; + + /* If we are the only diplomat in a threatened city, then stay to defend it. */ + if ((ctarget != NULL) && (count_diplomats_on_tile(pdiplomat->x, pdiplomat->y) == 1) + && ((ctarget->ai.urgency > 0) || (ctarget->ai.diplomat_threat))) { + return; + } + + /* Check if we can do the nasty diplomat stuff now. */ It's rather "Check if we are next to the enemy target and can do nasty stuff _right_ now". + ctarget = map_get_city(pdiplomat->goto_dest_x, pdiplomat->goto_dest_y); +#ifdef DEBUG [GB] ifdef?? why?? + freelog(LOG_DEBUG, "%s's diplomat %d aims for (%d,%d), and is at " + "(%d,%d) with distance %d to %s (owned by %s)", pplayer->name, pdiplomat->id, + pdiplomat->goto_dest_x, pdiplomat->goto_dest_y, pdiplomat->x, + pdiplomat->y, real_map_distance(pdiplomat->x, pdiplomat->y, + pdiplomat->goto_dest_x, pdiplomat->goto_dest_y), ctarget ? + ctarget->name : "(none)", ctarget ? city_owner(ctarget)->name : "n/a"); +#endif + if (ctarget && (real_map_distance(pdiplomat->x, pdiplomat->y, + pdiplomat->goto_dest_x, pdiplomat->goto_dest_y) == 1) [GB] Can use is_tiles_adjacent. Exactly the same thing but a better name. + && (pplayers_at_war(pplayer, city_owner(ctarget)) + || !player_has_embassy(pplayer, city_owner(ctarget)))) { [GB] align the line above + dact.diplomat_id=pdiplomat->id; + dact.target_id=ctarget->id; + if (!pdiplomat->foul && diplomat_can_do_action(pdiplomat, + DIPLOMAT_EMBASSY, ctarget->x, ctarget->y)) { + dact.action_type=DIPLOMAT_EMBASSY; + freelog(LOG_VERBOSE, "AI player %s's diplomat made embassy in %s", + pplayer->name, ctarget->name); [GB] Don't we get some message by default from handle_diplomat_action? + handle_diplomat_action(pplayer, &dact); + } else if (ctarget->steal == 0 && diplomat_can_do_action(pdiplomat, + DIPLOMAT_STEAL, ctarget->x, ctarget->y)) { + dact.action_type=DIPLOMAT_STEAL; + freelog(LOG_VERBOSE, "AI player %s's diplomat steals tech from %s", + pplayer->name, ctarget->name); + handle_diplomat_action(pplayer, &dact); + } else if (diplomat_can_do_action(pdiplomat, DIPLOMAT_INCITE, + ctarget->x, ctarget->y)) { + dact.action_type=DIPLOMAT_INCITE; + freelog(LOG_VERBOSE, "AI player %s's diplomat incites %s", + pplayer->name, ctarget->name); + handle_diplomat_action(pplayer, &dact); + } else if (diplomat_can_do_action(pdiplomat, DIPLOMAT_SABOTAGE, + ctarget->x, ctarget->y)) { + /* mission could be accomplished! do something else instead */ + dact.action_type=DIPLOMAT_SABOTAGE; + dact.value = B_LAST+1; /* no idea why +1 */ + freelog(LOG_VERBOSE, "AI player %s's diplomat sabotages %s", + pplayer->name, ctarget->name); + handle_diplomat_action(pplayer, &dact); + } else { + /* This can happen for a number of reasons, stop GOTO, retarget */ + freelog(LOG_NORMAL, "AI player %s's diplomat %d decides to stand " + "idle outside enemy city %s!", pplayer->name, pdiplomat->id, + ctarget->name); + handle_unit_activity_request(pdiplomat, ACTIVITY_IDLE); } + /* FIXME: What if we are a spy with more moves left? We could do more! + I don't know if the additional complexity is worth it... but at + least we should call some kind of escape_to_safety() function - Per */ [GB] Long comment style: /* blah * blah blah * more blah */ Here and below. + return; + } else if (pdiplomat->pgr && !broken_goto) { + /* We are on our way somewhere, so don't interrupt! This code we call + is not so good for diplomats, since it does not scoop up adjancent + enemy units and bangs into city we wish to diplomat instead of + stopping in safe distance of it, but will have to do for now. At + least it stops for cities and units now. Ideally we should have + a goto that stops for all adjancent enemy units and X tiles before + our destination. FIXME. - Per */ + enum goto_result cause = goto_route_execute(pdiplomat); + if (cause == GR_DIED || cause == GR_OUT_OF_MOVEPOINTS) { + freelog(LOG_DEBUG, "%s's diplomat %d %s", pplayer->name, + pdiplomat->id, cause == GR_DIED ? "died" : "ran out of moves"); + return; + } else { + freelog(LOG_DEBUG, "%s's diplomat %d noticed something happening " + "and stopped", pplayer->name, pdiplomat->id); + handle_unit_activity_request(pdiplomat, ACTIVITY_IDLE); /* deloop */ + ai_manage_diplomat(pplayer, pdiplomat); /* recurse */ + return; /* gone */ + } + } + + /* We are idle. Acquire a target */ + if ((ctarget = find_city_to_diplomat(pplayer, pdiplomat->x, pdiplomat->y, + pdiplomat->foul)) == NULL) { + if ((ctarget = find_city_to_defend(pplayer, pdiplomat->x, pdiplomat->y, + pdiplomat->type)) == NULL) { + /* This should only happen if the entire continent was suddenly + conquered. So we head for closest coastal city and wait for someone + to code ferrying for diplomats, or hostile attacks from the sea. */ + ctarget = find_closest_owned_city(pplayer, pdiplomat->x, pdiplomat->y, + TRUE, NULL); } } + + /* GOTO */ + if (ctarget) { + pdiplomat->goto_dest_x=ctarget->x; + pdiplomat->goto_dest_y=ctarget->y; + set_unit_activity(pdiplomat, ACTIVITY_GOTO); + freelog(LOG_DEBUG, "%s's diplomat %d is starting goto towards %s", + pplayer->name, pdiplomat->id, ctarget->name); + do_unit_goto(pdiplomat, GOTO_MOVE_ANY, FALSE); + } else { + /* Very unlikely but remotely possible */ + freelog(LOG_NORMAL, "%s's diplomat %d could not find a target, " + "a city to defend or even a coastal city!", pplayer->name, + pdiplomat->id); [GB] I guess you never saw it during testing ;) But it should be LOG_DEBUG. And what's this "coastal"? Is it "closest"? + } } /************************************************************************* diff -u3NrX /home/perrin/freeciv/diff_ignore freeciv/client/control.c freeciv-phased/client/control.c --- freeciv/client/control.c Mon Mar 18 19:51:25 2002 +++ freeciv-phased/client/control.c Wed Mar 27 13:18:49 2002 @@ -450,8 +450,10 @@ pcity = find_city_by_id(victim_id); punit = find_unit_by_id(victim_id); - if (!pdiplomat || !unit_flag(pdiplomat, F_DIPLOMAT)) + if (!pdiplomat || !unit_flag(pdiplomat, F_DIPLOMAT)) { + assert(1); /* this is always an error */ [GB] What does assert(1) do? continue; + } if (punit && is_diplomat_action_available(pdiplomat, DIPLOMAT_ANY_ACTION, [...] @@ -210,6 +210,10 @@ /* so we can contemplate with warmap fresh and decide later */ int settler_want, founder_want; /* for builder (F_SETTLERS) and founder (F_CITIES) */ int a, f, invasion; /* who's coming to kill us, for attack co-ordination */ + + /* Hackery, used by hostile AIs. This illustrated the desperate need for + a more intelligent build system - Per */ + bool already_considered_for_diplomat; }; [GB] Don't be too critical, the above isn't too bad for a hack. In fact I think think that this is _the_ right way. struct city { diff -u3NrX /home/perrin/freeciv/diff_ignore freeciv/common/player.c freeciv-phased/common/player.c --- freeciv/common/player.c Wed Mar 27 11:11:50 2002 +++ freeciv-phased/common/player.c Mon Mar 25 21:56:54 2002 @@ -35,15 +35,15 @@ #include "player.h" - /*************************************************************** -... + ... ***************************************************************/ [GB] Nice reformatting! Now how about a comment? bool player_has_embassy(struct player *pplayer, struct player *pplayer2) { + return (TEST_BIT(pplayer->embassy, pplayer2->player_no) + || (pplayer == pplayer2) + || (player_owns_active_wonder(pplayer, B_MARCO) && + pplayer != pplayer2 && !is_barbarian(pplayer2))); ^^^^^^^^^^^^^^^^^^^ [GB] This is a tautology, no? }