[Freeciv-Dev] Re: (PR#6871) Replace int with Impr_Type_id

["Raimar Falke" <i-freeciv-lists@xxxxxxxxxxxxx>]

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

On Sat, Nov 15, 2003 at 09:43:17AM -0800, Jason Short wrote:
> 
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=6871 >
> 
> Raimar Falke wrote:
> > <URL: http://rt.freeciv.org/Ticket/Display.html?id=6871 >
> > 
> > 
> > This patch changes the definition of Impr_Type_id from int to enum
> > improvement_type_id. This is coherent with the usage of Impr_Type_id.
> > 
> > There is a problem of a circular include dependency between
> > improvement.h and unittype.h. This could be resolved if we move struct
> > impr_effect to effects.h. Is this acceptable?
> 
> >    append_impr_or_unit_to_menu(menu, _("Improvements in City"), FALSE, 
> > TRUE, 
> > -                              FALSE, TRUE, city_got_building, FALSE);
> > +                              FALSE, TRUE, 
> > (TestCityFunc)city_got_building, FALSE);
> 
> I don't like casts of values being passed to functions; it's a hack to 
> make the compilers/checkers happy but doesn't add correctness.

In this case it makes the compiler happy.

> In this particular case, why is the cast needed?  Is city_got_building 
> not already a TestCityFunc?  If not, it should be made into one.

It was. The patch changed this by changing the prototype of
city_got_building to the correct one. The gui code should be adapted
here. Adding a

bool city_got_building_wrapper(struct city *pcity,  gint id){
  return city_got_building(pcity,(Impr_Type_id)id);
}

would help.

> > -  dio_put_uint8_vec8(&dout, packet->equiv_dupl, B_LAST);
> > -  dio_put_uint8_vec8(&dout, packet->equiv_repl, B_LAST);
> > +  dio_put_uint8_vec8(&dout, (int *)packet->equiv_dupl, B_LAST);
> > +  dio_put_uint8_vec8(&dout, (int *)packet->equiv_repl, B_LAST);
> 
> Here again.  If the enumeration isn't equivalent to an integer value, 
> this will give very wrong results.

If sizeof(enum ...)!=sizeof(int) a lot will break. We have similar
casts in other parts of the network code.

> I'd suggest that this needs to be thought out before changing
> Impr_Type_id to an enumeration.

Changing Impr_Type_id to an enumeration isn't my goal. My goal is to
change Impr_Type_id to how it is currently used in the code. Note that
the enumaration will be removed when we get gen_improvements. But
nobody knows when this will happen.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
  "Debugging is twice as hard as writing the code in the first place.
   Therefore, if you write the code as cleverly as possible, you are,
   by definition, not smart enough to debug it."
    -- Brian W. Kernighan