[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Hopefully committed correctly to S2_2 (r14846) and trunk (r14847). There were some files added/moved and lots of changes to makefiles, so be sure to re-run autogen.sh. -- ほとんどできなかった。 ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 2008/6/19 Madeline Book: When I get the OK, I'll commit this editor work so far so that I can continue implementing other editor features. Looks good to me. Meanwhile, I am having a problem with my method of composi- ting sprites to make icons (functions create_terrain_pixbuf, create_military_base_pixbuf and create_tile_pixbuf). For some reason the sprite.offset_{x,y} fields seem to mean something different for terrains than they do for the military base sprites. For example, some base sprites have negative offsets! Sorry, can't help you there. When I changed tilespec code to handle military bases a bit differently from other specials, offset stuff was just recreated as it was before. Are unit sprite offsets handled same way as terrain sprite offsets, or do they use (yet) another system? - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [book - Wed Jun 11 20:43:45 2008]: [EMAIL PROTECTED] - Wed Jun 11 19:15:25 2008]: - packhand.c:2368: handle_tile_info: Assertion `unit_list_size(ptile-units) == 0' failed. I were removing vision from tile with somebody else's unit. This looks like a bug due to a condition I failed to forsee. It should be easy enough to fix (i.e. don't allow removing vision of a player on a tile with that players' units/city in it). I'll get right on fixing it. This actually turned out to be caused by something different than what I expected. It seems the client expects tiles changing seen state from known seen to have no units on it: client/packhand.c +2357 if (TILE_KNOWN_SEEN == old_known TILE_KNOWN_SEEN != new_known) { /* This is an error. So first we log the error, then make assertion. But for NDEBUG clients we fix the error. */ unit_list_iterate(ptile-units, punit) { freelog(LOG_ERROR, %p %d %s at (%d,%d) %s, punit, punit-id, unit_rule_name(punit), TILE_XY(punit-tile), player_name(unit_owner(punit))); } unit_list_iterate_end; assert(unit_list_size(ptile-units) == 0); unit_list_unlink_all(ptile-units); } So my idea is to send a packet_unit_remove to the client for every unit on the tile if none of the units give vision to the client's player (i.e. neither owned by the client's player nor by any player that really shares vision with the player). Is there anything wrong with this approach? Or would it be better to just have the client remove the units itself in the above quoted section? -- 早く返事して下さいね ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Madeline Book wrote: I actually like the whole 3 line ... header as a visual separator between functions (thanks in large part due to the nice color difference in syntax hilighting). Anyway, that is just my preference; are you suggesting that empty headers should just be: / / Or perhaps no header at all? On that I'm not sure. I actually never found a function that couldn't make use of a 1-line descriptive comment yet. I'd probably say the empty header should just have the comments with no line in between though. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [book - Wed Jun 11 19:56:11 2008]: [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]: - 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. Alright after examining the new generic military-base code I have reached some conclusions regarding the use of S_FORTRESS and S_AIRBASE in the editor code (off-topic rant about the approach in general posted in PR#37356, which you can feel free to ignore if you've heard before or just have not had time to address the issues raised there). Please correct me if I am wrong but the references to S_FORTRESS and S_AIRBASE that are being objected to are in the function get_basic_special_sprite in client/tilespec.c. My argument is that as long as S_FORTRESS and S_AIRBASE are a part of enum tile_special_type this reference is simply unavoidable. Furthermore if I were to say add some manner of conversion function (e.g. base_by_special, special_is_base) I would have to check for this and add annoying special-case code every time I iterate over the special types (which is very common considering the requirements of the editor). In any case, changes will have to be made in the future when military-base and special handling changes; I'll be sure to update the editor code as that is finalized. I will however look into using the {fore,middle,back}ground sprites more intelligently (I suppose they should be combined into one sprite to actually make the full representative sprite of an airbase or fortress). -- 誰があの余計なことに興味を持っているか ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Thanks for the thorough reply. My ideas for specific issues follow below. [EMAIL PROTECTED] - Thu Jun 12 21:21:14 2008]: Please don't assume that bases are stored as specials. Iterating specials should already skip S_FORTRESS and S_AIRBASE (at least it did when I last worked on this) The macro tile_special_type_iterate doesn't skip over them, so I suppose I will make the change and post that as a new ticket. I realize that editor code may require some new accessor functions. I can write those in a proper manner, if you tell me what you need. Note that tile.c has functions for checking, adding and removing bases from tile. I think the existing accessors are sufficient. If I am missing something I'll add it myself or ask for ideas if I am unsure of the implementation. I would have to check for this and add annoying special-case code every time I iterate over the special types (which is very common considering the requirements of the editor). Isn't common iterator macro no longer doing this? As I said above, tile_special_type_iterate does not, but I will fix it so that it does. Then in the other parts of the editor code, after iterating specials, I'll add code that iterates over bases (using base_type_iterate). I will however look into using the {fore,middle,back}ground sprites more intelligently (I suppose they should be combined into one sprite to actually make the full representative sprite of an airbase or fortress). No, those separate parts are for a reason. Depending on tileset, units in fortress may appear in front of fortress background, but behind foreground. There certainly is a reason for middleground too, even though I cannot remember it now. I assumed that drawing the foreground over the middleground over the background would make an acceptable icon. ;) But I'll check this some more. -- 今おいしいラーメンを食べる ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
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 Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [EMAIL PROTECTED] - Wed Jun 11 19:15:25 2008]: After some testing: - Only functionality missing from this new model, but present in old version seems to be modifying city internals I don't recall there being any GUI for doing that and the packets, handlers, etc., were variously #if 0'd out along with some FIXMEs. Anyway the property editor (in editprop.[ch]) that I would be working on now and will be working on soon will be able to do that (also for tiles, units, and possibly players). - packhand.c:2368: handle_tile_info: Assertion `unit_list_size(ptile-units) == 0' failed. I were removing vision from tile with somebody else's unit. This looks like a bug due to a condition I failed to forsee. It should be easy enough to fix (i.e. don't allow removing vision of a player on a tile with that players' units/city in it). I'll get right on fixing it. -- 皆さん頑張ろうね! ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Madeline Book wrote: - 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 an issue for a lot of regular-game city report operations as well. There is no established standard afaik (there should be), but most places seem to have client checking as well as server spamming of error messages IIRC. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Madeline Book wrote: 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? Previous discussions have agreed that empty comments are not helpful. It's possible that the function name really says everything about the function; in such a case maybe ... isn't needed. But I really don't see any use to having ... as a comment. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [EMAIL PROTECTED] - Thu Jun 12 04:43:02 2008]: Madeline Book wrote: 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? Previous discussions have agreed that empty comments are not helpful. It's possible that the function name really says everything about the function; in such a case maybe ... isn't needed. But I really don't see any use to having ... as a comment. I actually like the whole 3 line ... header as a visual separator between functions (thanks in large part due to the nice color difference in syntax hilighting). Anyway, that is just my preference; are you suggesting that empty headers should just be: / / Or perhaps no header at all? - 勘弁してくれ。 ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [EMAIL PROTECTED] - Tue Jun 10 12:37:35 2008]: 2008/6/9 Madeline Book With your blessing I will being to merge this new editor gui code into S2_2 and trunk. That way afterwards individual commits can stay relatively small and examinable. Currently I concentrate to 2.1.5 release, so I'm not looking in to your patch just yet. I'm not asking you to postpone commit if doing so will make further development harder for you (you have waited the required 24h for comments already, and there's no point in waiting forever just because others have no time) but if you can keep on developing even when this is not committed, I would appreciate postponing this to next week. No problem, it doesn't inconvenience me at all. I'll just keep working on my local repository and post diff snapshots every week or two (and perhaps expand the internal summary on the wikia page). -- 喜んで。問題ないよ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 On Tue, Jun 10, 2008 at 11:32 AM, Madeline Book [EMAIL PROTECTED] wrote: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [EMAIL PROTECTED] - Tue Jun 10 12:37:35 2008]: 2008/6/9 Madeline Book With your blessing I will being to merge this new editor gui code into S2_2 and trunk. That way afterwards individual commits can stay relatively small and examinable. Currently I concentrate to 2.1.5 release, so I'm not looking in to your patch just yet. I'm not asking you to postpone commit if doing so will make further development harder for you (you have waited the required 24h for comments already, and there's no point in waiting forever just because others have no time) but if you can keep on developing even when this is not committed, I would appreciate postponing this to next week. No problem, it doesn't inconvenience me at all. I'll just keep working on my local repository and post diff snapshots every week or two (and perhaps expand the internal summary on the wikia page). Personally I'd rather it be committed (once caz gets time to look at it) so if I get some time to take a look I can go straight into fixing things. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 2008/4/28 Madeline Book [EMAIL PROTECTED]: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 The usual status update, gentlemen. I will have more time to work on this this week since the stuff I wanted to do for longturn is done now. ;) You've made the Global observer work again, I noticed! Would it work in a networked game too? I haven't looked into the details in the changes. If so, could we break out that part and commit it as soon as possible? AFAIK Marko would be happy to have the global observer back. And I would too. ulrik ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [engla - Mon Apr 28 16:15:54 2008]: 2008/4/28 Madeline Book [EMAIL PROTECTED]: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 The usual status update, gentlemen. I will have more time to work on this this week since the stuff I wanted to do for longturn is done now. ;) You've made the Global observer work again, I noticed! Would it work in a networked game too? I haven't looked into the details in the changes. If so, could we break out that part and commit it as soon as possible? AFAIK Marko would be happy to have the global observer back. And I would too. See PR#40139. Since I needed global observer to work for testing editor features, I had need to apply the patch posted there to my local editor branch. Similarly with PR#40124. -- 料理が全然上手じゃないという噂を聞いた。 ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 On 04/04/2008, Madeline Book wrote: A small preview of the editor toolbar in action, as described in the editor wikia page. I have also attached a screenshot if you do not have the time (or the means) to apply a git binary diff and compile the result (if you do, don't forget to run autogen.sh). Care to send editor.png separately so I can take just the code changes from patch, and not to struggle with binary part? Looks like good development. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 [EMAIL PROTECTED] - Sun Apr 06 16:20:31 2008]: On 04/04/2008, Madeline Book wrote: A small preview of the editor toolbar in action, as described in the editor wikia page. I have also attached a screenshot if you do not have the time (or the means) to apply a git binary diff and compile the result (if you do, don't forget to run autogen.sh). Care to send editor.png separately so I can take just the code changes from patch, and not to struggle with binary part? Can do. -- そして地獄に迷い込んだ女と会うこと。 inline: editor.png___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
On Fri, Apr 4, 2008 at 10:45 AM, Daniel Markstedt [EMAIL PROTECTED] wrote: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Very nice, Madeleine! I'm thinking of starting a forum thread to attract the attention of Freeciv's creative minds. :) Yes, really awesome. I am happy to see that someone is working on the editor again. - Per ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work
URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 Very nice, Madeleine! I'm thinking of starting a forum thread to attract the attention of Freeciv's creative minds. :) On 4/4/08, Madeline Book [EMAIL PROTECTED] wrote: URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 A small preview of the editor toolbar in action, as described in the editor wikia page. Internals are still mostly stubs, which I am now working on. ;) I have also attached a screenshot if you do not have the time (or the means) to apply a git binary diff and compile the result (if you do, don't forget to run autogen.sh). I would welcome perhaps better icons, if any of the freeciv artists would care to make some; the ones I threw together are mostly just cut-and-pasted from random places and could probably be improved. -- じゃー、それは?いい星ですね。どのぐらい? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev