[Freeciv-Dev] Re: (PR#13005) latest CVS (assertion failed, effects.c)

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=13005 >

Vasco Alexandre da Silva Costa wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=13005 >
> 
> 
> These functions are just a convenient simplified wrapper glue around the
> generic get_target_bonus_effects function.
> If you passed all get_city_tile_output_bonus arguments as NULL, it would
> end up being the same thing as calling get_world_bonus.

Which would obviously be a bug.

> Calling get_city_tile_output_bonus with a NULL tile argument is equivalent
> to calling get_city_output_bonus (if it existed that is). So in that
> respect, I suppose the NULL assertion is fine, it could even suggest you
> which function you should be using.

Unfortunately there is already a get_city_output_bonus function,
otherwise I would have added one.

> I suppose we could be using a different API though. The way this has
> evolved, I think a single varargs function would be better. e.g.:
> 
> int get_effect_bonus(enum effect_type effect_type, const char *fmt, ...)
> 
> 
> get_effect_bonus(EFT_ENABLE_NUKE, "")
>       -> get_world_bonus
> 
> get_effect_bonus(EFT_HAVE_EMBASSIES, "p", pplayer)
>       -> get_player_bonus
> 
> get_effect_bonus(EFT_MAKE_HAPPY, "c", pcity)
>       -> get_city_bonus
> 
> get_effect_bonus(EFT_OUTPUT_ADD_TILE, "cto", pcity, ptile, poutput)
>       -> get_city_tile_output_bonus
> 
> etc.

Please no.  You give away all typesafety and any hope at catching errors
at compile time.

-jason