freeciv21 icon indicating copy to clipboard operation
freeciv21 copied to clipboard

Unit upkeep calculation code does not account for un-homed units

Open jwrober opened this issue 2 years ago • 1 comments

Describe the bug While working on #1808 and posting screen shots to my team mates in the LT76Teams game, it was noticed that the new units view was showing me paying for upkeep for my explorers and tribal workers.

The existing code base takes into account calculating upkeep for units that are "homed", but does not account for units that are "unhomed", such as the units you get at game start. What ends up happening is when ever upkeep is calculated in the client, you have to do checks like this:

unit_list_iterate(client.conn.playing->units, punit)
{
  if (unit_type_get(punit) == unittype) {
    count++;
    upgradable =
        client_has_player()
        && nullptr != can_upgrade_unittype(client_player(), unittype);

    // Only units with a home city have upkeep
    if (punit->homecity != 0) {
      gold_cost += punit->upkeep[O_GOLD];
      food_cost += punit->upkeep[O_FOOD];
      shield_cost += punit->upkeep[O_SHIELD];
    }
  }
}
unit_list_iterate_end;

or like this

unit_list_iterate(pcity->units_supported, punit)
{
  if (unit_type_get(punit) == unittype) {
    count++;
    partial_cost += punit->upkeep[O_GOLD];
  }
}
unit_list_iterate_end;

In both cases the client code is purposely only asking to get upkeep for units. However in the first example, if you get rid of if (punit->homecity != 0), the client will show upkeep for unhomed units taking the value from the uk_* value for the unit in units.ruleset.

Expected behavior This check should be completely unnecessary. The base code that gets the upkeep at the server level (or what ever low level area that does all the checks for government, etc.) should do the work so the client doesn't have to do the check all the time.

Screenshots N/A

Platform and version (please complete the following information):

  • OS: All supported
  • Freeciv21 version: been around FOREVER!
  • Ruleset/Longturn game (if applicable): N/A

Additional context Add any other context about the problem here.

jwrober avatar Feb 07 '23 22:02 jwrober

Retriaging as refactoring since in the end the code is working, it's just doing things in a strange way.

lmoureaux avatar Dec 27 '23 02:12 lmoureaux