[Freeciv-Dev] Re: PATCH: cityview unification (PR#1085)

[Vasco Alexandre Da Silva Costa <vasc@xxxxxxxxxxxxxx>]

On Mon, 3 Dec 2001 jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:

> The attached patch unifies city_get_canvas_xy/city_get_map_xy from all 
> GUI's (except xaw) into one set of common functions.  I did this just 
> because I've grown tired of seeing the identical code in so many places.
> 
> Some issues:
> 
> - These functions should rightfully go into a file cityview_common.[ch]. 
>   However, no such file currently exists so I've placed them into 
> mapview_common.[ch].  I'll fix this if desired (creating the new file).

That would be a better idea, yes.

> - I don't clean up the functions any; I just copy them as-is.  I would 
> like to clean them up, though (at the same time that I clean up the 
> corresponding mapview functions).
> 
> - I've changed the names from city_get_[canvas|map]_xy to 
> canvas_pos_to_city_pos/city_pos_to_canvas_pos.  This results in a lot of 
> extra bloat to the diff file, since these name changes were made in many 
> places.  If desired, I'll undo this - but the new name seems much more 
> logical and matches the corresponding mapview functions.

Hmm. What is it with you people and long function names? :-)
I would name it canvas_to_city_pos() and city_to_canvas_pos() but i don't
really care that much about that.

> Aside from general code cleanup, this isn't a crucial change.  It does 
> make the codebase smaller and IMO more maintainable.  It will make it 
> easier to supply isometric-view to the xaw client (since the xaw client 
> can now use these functions).  And it will make it easier for me to 
> clean up these functions to match the mapview ones (it will be necessary 
> to clean up the mapview ones for the general-topologies change, and 
> would be very bad IMO to not clean up this code at the same time).

Excellent! Please proceed!

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