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

["Vasco Alexandre da Silva Costa" <vasc@xxxxxxxxxxxxxx>]

<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.

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.

The wrappers were useful because they allowed us to change the effects API
core code more than once with minimum (zero) impact on existing effects API
user code.

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.

Pros:
  * only one function to remember as an effects user instead of a zillion.
  * only one function to code as an effects developer instead of a zillion.
  * at least as compact as the old versions.

Cons:
  * No static argument checking.


I never thought there would be an explosion in target types like this,
that's why I sticked with the simpler wrappers. But now, I think a varargs
like function would be best.

---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa