[Freeciv-Dev] Re: (PR#15877) Editor: place units and cities

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

I'm not sure if this patch is going down the wrong path but I do think 
the existing structure in the editor (client) code is rather 
problematic.  The selected_unit/selected_city/new_unit/new_city set of 
variables are not good because they do not give a good way to control 
the data in an easily understandable way.

For instance, you can call editor_get_new_city and then fiddle with the 
city values, then call do_city() to place a city.  But it is not at all 
obvious to the coder what parts of the city can be changed and will take 
effect, or in what way.  The city structure is giant, and filled with 
fields that will break things or be ignored if you edit them.  So what 
is the point of filling out such a structure only to have it be 
converted 1:1 into an EDITOR_CREATE_CITY packet?

As an example of the problem, the patch you provide is broken in several 
ways.  The rapture and anarchy values may be edited in the dialog but 
are ignored in the packet.  Other values that are in the packet may not 
be edited.

I'm still not sold on the tools dialog method of editing (rather than 
editing via the citydlg), since I don't see how it will work for editing 
existing units; it would be nice to see a completed demo or just an 
explanation of how this works. If we're going to do it that way we need 
a better interface for the editor code that exposes only the values that 
can be edited.  One possibility is to replace the new_city/new_unit 
pointers with a simple packet_editor_city structure.  Allowing the GUI 
code to access and edit this structure ensures that only editable parts 
may be edited, and will shorten the code by cutting out the 1:1 
conversion from city to edit-city-packet structure.  This isn't the only 
possibility, however.

-jason