[Freeciv-Dev] Re: Freeciv commit: ali: Rectangular area selection of cities with righ...

[Arnstein Lindgard <a-l@xxxxxxx>]

On Fri, 14 Nov 2003 14:13:25 +0100
Raimar Falke <i-freeciv-lists@xxxxxxxxxxxxx> wrote:

> > +  if (clipboard_is_unit)  {
> > +    cid += B_LAST;
> > +  }
> 
> This is strongly discouraged. It violates the abstraction of the cid
> data type.
> 
> It should be written as
> 
>   cid cid = cid_encode(clipboard_is_unit, clipboard);

Fixed.

> > +  if ((pcity->currently_building == clipboard
> > +    && pcity->is_building_unit == clipboard_is_unit)
> > +      || !city_can_build_impr_or_unit(pcity, cid))  {
> > +    return;
> > +  }
> 
> This may also be rewritten as
> 
>   if (cid == cid_encode_from_city(pcity)
>       || !city_can_build_impr_or_unit(pcity, cid)) {
>     return;
>   }

Advice taken :)

> A general question is if we shouldn't (in the client) introduce cid
> everywhere.

The cid concept was a bit unfamiliar to me after looking mostly at
the client, and not so much in the city-dialog code. I ended up using
B_LAST after seeing many comments about the relationship between
B_LAST and cid.

> A seperate struct should be introduced to mark it clearly that this
> field is client-only. See struct connection as an example.

Fixed.


Arnstein

rectangle_correction.diff
Description: Text document