Summary: What makes a building useless/replaced/redundant?
Submitted by: jtn
Submitted on: Mon Feb 14 00:28:43 2011
Severity: 3 - Normal
Priority: 5 - Normal
Status: Need Info
Assigned to: None
Discussion Lock: Any
Operating System: None
Currently, different bits of the code take slightly different views of what
constitutes a "useless" building.
In particular, most parts of the code (including AI) don't take into account
whether a building is enabling production of a certain kind of unit, via the
unit's impr_req property (which isn't set in any of the default rulesets). (I
happened upon this while looking at bug #17730; the one place which *does*
check impr_provides_buildable_units() is the Gtk client's worklist dialog in
Most places use is_building_replaced(), which looks at the effects system to
see if any of the building's effects are currently effective, and has a
special case for Coinage (IF_GOLD). Callers on trunk are:
* ai_sell_obsolete_buildings(): I think the AI can sell of buildings even if
they are the only thing enabling it to create certain units.
* advbuilding.c:adjust_wants_by_effects(): will never consider buildings
which only enable units.
* advbuilding.c:building_advisor() ("find our Wonder City"): probably
* Gtk client wldlg.c: as above
* sell_all_improvements() (on client): will consider improvements only
providing units as obsolete (this is used by the "sell all" buttons on the
* city_improvement_name_translation (on client): marks buildings with "(*)"
if all they do is allow units (I've tested this)
Another factor that could be taken into account when deciding "uselessness"
So, the definition of "uselessness" used in the above places could usefully
be expanded to include impr_provides_buildable_units(). However, that could be
argued to subtly change the semantics. Currently, if a building returns TRUE
from is_building_replaced() now, it will probably continue to do so, except
for "unplanned" events such as losing the city containing the "masking"
However, impr_provides_buildable_units() may return FALSE now but TRUE after
a "planned" change -- perhaps the units require both the particular building
and a particular government to be built. In this case, a "planned" change of
government will stop the building from being redundant, but in the meantime it
might be considered for sale.
Perhaps I'm over-analysing this :) Clearly such a function can never be
perfect in taking into account the might-have-beens.
So, I've a mind to:
0 repurpose is_building_replaced() for purely effects-based redundancy
(removing the IF_GOLD exception);
0 invent a new is_building_useless() which checks is_building_replaced(),
IF_GOLD, impr_provides_buildable_units(), and perhaps improvement_obsolete();
0 call is_building_useless() from as many of the call sites above as makes
Second opinions welcomed (especially on the effect on AI/advisors, which I'm
not that familiar with).
(Note for future improvement: the impr_provides_buildable_units() check will
not behave ideally if more than one building enables a given type of unit; if
impr_req was replaced by the effects system it would work better, since one
building would be clearly "better" than the other (although the autogenerated
help would probably get worse). But that's a project for another day.)
Reply to this item at:
Message sent via/by Gna!
Freeciv-dev mailing list