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

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<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 particular case, why is the cast needed?  Is city_got_building 
not already a TestCityFunc?  If not, it should be made into one.

> -  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.  I'd suggest that this needs to be 
thought out before changing Impr_Type_id to an enumeration.

jason