<URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 >
> [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]: > > I only read sources, not yet tested. > > > - Please don't add new empty function headers. New code should have > function headers already when committed. What do you mean by "empty function header"? If you mean the /***************************** ... ******************************/ Then I only fill that in if the function name is not descriptive enough or there are some special conditions or side-effects that programmers need to be aware of (e.g. "you must free the return value yourself"). Do you think I should describe the functions more in the header? > - You add function client_player(), but don't always use it in new > code instead of client.conn.playing This I will fix. It is because I added the client_* functions after already writing some code using client.conn.playing and forgot to update the old code. :( > > What really happens to these files: > > --- a/client/gui-gtk-2.0/Makefile.am > +++ b/client/gui-gtk-2.0/Makefile.am > @@ -42,8 +42,10 @@ libguiclient_a_SOURCES = \ > dialogs.h \ > diplodlg.c \ > diplodlg.h \ > - editdlg.c \ > - editdlg.h \ > + editgui.c \ > + editgui.h \ > + editprop.c \ > + editprop.h \ > > Are editdlg.[ch] renamed, split or completely replaced? They are completely replaced. Similarly for client/include /editgui_g.h. The point was that the code in editgui.[ch] is more that just an "edit dialog" so the name editdlg did not really apply anymore (cf. function editgui_tileset_changed and editgui_refresh which affect more than one widget). I added editprop.[ch] because editgui.[ch] was getting very long. It will contain the properties dialog (see the wikia editor page for an overview) and its helper functions. > - There's some new direct S_FORTRESS and S_AIRBASE references. That > goes against work for generic military bases. See common/base.[ch] to > see if better implementation is already possible. That I did not know about. I'll look into using that code. Most of the time I had problems trying to get a "generic" sprite for various objects (like roads or fortresses) so the added code in client/tilespec.c might not be the best possible approach. > - Add FIXME about the fact that following code is not generic enough > (gen-movement) > + coastal = is_sailing_unittype(punittype); > + homecity = find_closest_owned_city(pplayer, ptile, coastal, NULL); This is a direct conversion of the old can_unit_exist_at_tile to work on unit_type's (needed for example to check if a unit type can be created at a certain tile). I'll add the FIXME here, but generalizing this code really belongs in another ticket. > - Why city creation now fails without telling reason to client? > (modifications to handle_edit_city_create() / > handle_edit_create_city() ) I thought a long time about how and when to do error messages in server edithand.c. I decided the server should for the most part remain silent for bad input or "illegal" operations. This is because the user (who is editing) is assumed to know the rules (e.g. where cities may be placed) and becase operations can be batched (which would result in a lot of duplicate messages). Do you think the server should notify the user more in such circumstances? > This is huge patch. Let's make only agreed fixes against this patch > before commit (It's impossible for others to track what other changes > appear if you do other development). It's not hard for me to stash local changes and variously merge between branches, but I will address now the issues you have raised before extending the functionality further. ---------------------------------------------------------------------- 何も欲しくない人がこの世界のどこかにいるか _______________________________________________ Freeciv-dev mailing list [email protected] https://mail.gna.org/listinfo/freeciv-dev
