[Freeciv-Dev] Re: (PR#8455) Bombardment (aka ranged attack)

["Per I. Mathisen" <per@xxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8455 >

On Mon, 5 Apr 2004, James Canete wrote:
> Here's a version where I removed all that stuff regarding range, client
> activation of bombardment, and special options.
...
> No, I don't like the way it is now, but I guess you can't please
> everyone. :)

At least it is a start, and you can always add more options later, right?
If they are necessary...

+++ freeciv-mod/common/combat.h Mon Apr  5 03:27:22 2004
@@ -54,6 +54,9 @@
 int get_virtual_defense_power(Unit_Type_id att_type, Unit_Type_id def_type,
                              int x, int y, bool fortified, int veteran);
 int get_total_attack_power(struct unit *attacker, struct unit *defender);
+int base_get_bombard_power(Unit_Type_id type, int veteran);
+int get_bombard_power(struct unit *punit);
+int get_total_bombard_power(struct unit *attacker, struct unit *defender);

Are all these three used anywhere else?

+++ freeciv-mod/common/packets.def      Mon Apr  5 03:36:32 2004
@@ -946,6 +946,8 @@
   FLOAT power_fact[MAX_VET_LEVELS]; add-cap(veteran)
   UINT8 move_bonus[MAX_VET_LEVELS]; add-cap(veteran)

+  UINT8 bombard_rate;

You should add a capability here. Also correspondingly in
common/capability.c.

+++ freeciv-mod/common/unit.c   Mon Apr  5 03:36:58 2004

 /**************************************************************************
+Return whether the unit can bombard.
+Basically if it is a bombarder, isn't being transported, and hasn't
+moved this turn.
+**************************************************************************/

Nitpick: Two spaces indentation before text inside a function comment.

+  if (!unit_flag(punit, F_BOMBARDER))
+    return FALSE;

Style: You forgot some braces. Should be:

+ if (!unit_flag(punit, F_BOMBARDER)) {
+    return FALSE;
+ }

instead. I noticed this several places.

+++ freeciv-mod/server/ruleset.c        Mon Apr  5 03:39:10 2004
@@ -905,6 +905,8 @@
     BV_CLR_ALL(u->flags);
     assert(!unit_type_flag(i, F_LAST-1));

+    u->bombard_rate = 0;
+
     slist = secfile_lookup_str_vec(file, &nval, "%s.flags", sec[i]);
     for(j=0; j<nval; j++) {
       sval = slist[j];
@@ -931,6 +933,11 @@
         u->paratroopers_mr_req = 0;
         u->paratroopers_mr_sub = 0;
       }
+
+      if (ival == F_BOMBARDER) {
+       u->bombard_rate = secfile_lookup_int(file,
+           "%s.bombard_rate", sec[i]);
+      }

You should use secfile_lookup_int_default() using zero as default instead
here. Then you don't need the above u->bombard_rate = 0 nor the if test.

+++ freeciv-mod/server/unithand.c       Mon Apr  5 03:56:30 2004

+static bool handle_unit_bombard_request(struct unit *punit, int x, int y);

This isn't a packet anymore, is it? So it should not be named _request.

+    if (map_is_known_and_seen(pattacker->x, pattacker->y, other_player) ||
+       map_is_known_and_seen(pdefender->x, pdefender->y, other_player)) {
+      if (!can_player_see_unit(other_player, pattacker)) {
+       assert(other_player->player_no != pattacker->owner);
+       lsend_packet_unit_short_info(&other_player->connections,
+                                    &unit_att_short_packet);
+      }

Conditionals || && etc should preface new lines. As in

if (cond1
    && cond2)

not

if (cond1 ||
    cond2)

+  combat.attacker_unit_id=pattacker->id;

Missing space around '='.

+  /*combat.bombard=bombard*/

Delete this line.

+static bool handle_unit_bombard_request(struct unit *punit, int x, int y)

+  for (i=0; i<rate; i++) {

Missing spaces. Several other places too.

+    notify_player_ex(unit_owner(punit), punit->x, punit->y,
+                    E_UNIT_WIN_ATT,
+                    _("Game: Your bombarding %s%s became more experienced!"),
+                    unit_name(punit->type),
+                    get_location_str_at(unit_owner(punit),
+                    punit->x, punit->y));

This format string does not look correct.

+  /*combat.bombard = FALSE;*/

Delete.

+++ freeciv-mod/server/unittools.c      Mon Apr  5 03:43:54 2004
+void unit_bombard_unit(struct unit *attacker, struct unit *defender)
+{
+  int attackpower = get_total_bombard_power(attacker,defender);
+  int defensepower = get_total_defense_power(attacker,defender);
+
+  int attack_firepower, defense_firepower;
+  get_modified_firepower(attacker, defender,
+                        &attack_firepower, &defense_firepower);
+
+  freelog(LOG_VERBOSE, "bombard:%d, defense:%d, bombard firepower:%d",
+         attackpower, defensepower, attack_firepower);
+
+  if (myrand(attackpower+defensepower) >= defensepower) {
+    defender->hp -= attack_firepower;
+  }
+
+  /* Don't kill the target. */
+  if (defender->hp <= 0) {
+    defender->hp = 1;
+  }
+}

You don't actually use bombard_rate anywhere?? Can this be removed?

  - Per