Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Pepeto _ wrote: > http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > >> [EMAIL PROTECTED] - Lun. Déc. 03 18:45:11 2007]: >> >> William Allen Simpson wrote: >> >>> (2) One of them has not worked for about two years, and certainly > was not >>> tested at the time it was committed. >> I'm quite sure it worked when it was committed. > > The code you committed was not clean. It worked for one selected unit. > For more, it crashed. But I don't blame you. In a so big patch, This is > 'human' to make at least one mistake. Multi unit selection feature > couldn't be fixed in only one commit, that's normal... The original patch was provided by a warclient developer and worked. I broke it when battlegroups were introduced, clearly a rather foolish programming error. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > William Allen Simpson wrote: > http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > > Jason Dorje Short wrote: >> William Allen Simpson wrote: >>> (2) One of them has not worked for about two years, and certainly was not >>> tested at the time it was committed. >> I'm quite sure it worked when it was committed. >> > Here's the patch from your message of Wed, 16 Nov 2005 18:35:19 -0800, > and I don't see any difference from what was there last week: > > @@ -2324,10 +2623,9 @@ > **/ > void key_quickselect(enum quickselect_type qtype) > { > - struct unit *punit; > + unit_list_iterate(get_units_in_focus(), punit) { > +struct unit *punit2 = quickselect(punit->tile, qtype); > > - if(punit_focus) { > -punit = quickselect(punit_focus->tile, qtype); > -set_unit_focus_and_select(punit); > - } > +set_unit_focus_and_select(punit2); > + } unit_list_iterate_end; > } Yes, the previously working behavior was broken in that commit, and nobody noticed until it was released since all the good multiplayer players use the stable branch. We know this already. > Have you tested LMB and RMB? I don't have a useful test scenario. They work but I'm not sure if it's as the behavior is intended. Obviously this code should be examined. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > [EMAIL PROTECTED] - Lun. Déc. 03 18:45:11 2007]: > > William Allen Simpson wrote: > > > (2) One of them has not worked for about two years, and certainly was not > > tested at the time it was committed. > > I'm quite sure it worked when it was committed. The code you committed was not clean. It worked for one selected unit. For more, it crashed. But I don't blame you. In a so big patch, This is 'human' to make at least one mistake. Multi unit selection feature couldn't be fixed in only one commit, that's normal... ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Jason Dorje Short wrote: > William Allen Simpson wrote: >> (2) One of them has not worked for about two years, and certainly was not >> tested at the time it was committed. > > I'm quite sure it worked when it was committed. > Here's the patch from your message of Wed, 16 Nov 2005 18:35:19 -0800, and I don't see any difference from what was there last week: @@ -2324,10 +2623,9 @@ **/ void key_quickselect(enum quickselect_type qtype) { - struct unit *punit; + unit_list_iterate(get_units_in_focus(), punit) { +struct unit *punit2 = quickselect(punit->tile, qtype); - if(punit_focus) { -punit = quickselect(punit_focus->tile, qtype); -set_unit_focus_and_select(punit); - } +set_unit_focus_and_select(punit2); + } unit_list_iterate_end; } It was a bad idea then, a bad idea today, and will be a bad idea tomorrow. Let's stick to keeping working code working, and stop obsessing about code that hasn't worked in years Have you tested LMB and RMB? I don't have a useful test scenario. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > William Allen Simpson wrote: > (1) You have two ways of doing the same thing. Should be fixed. > (2) One of them has not worked for about two years, and certainly was not > tested at the time it was committed. I'm quite sure it worked when it was committed. > (3) It crashes. We're not talking about failing an assert, or some other > known problem, we're talking hard crashes! Quite. Fortunately it's a 2-line fix. > (4) It is not easily used by laptops. That IS a problem. As discussed on irc and requested many times throughout the life of the progject, gtk menu keys really should be configurable. > (5) It was not important enough to be implemented by all clients -- nor > *any* other client! At the time it was added gui-gtk was pretty much the only client. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Daniel Markstedt wrote: > I agree that this was done in too hasty a fashion. William, if you > wanted this code gone so badly, you could at least have presented some > kind of roadmap for reimplementation. > There is no *need* for reimplementation. There is *already* another implementation using the mouse. This is an example of one of the serious problems with this project: careless and untested implementation. I came to this project to help add new features, but instead have spent nearly all my time trying to cleanup old trash. (1) You have two ways of doing the same thing. (2) One of them has not worked for about two years, and certainly was not tested at the time it was committed. (3) It crashes. We're not talking about failing an assert, or some other known problem, we're talking hard crashes! (4) It is not easily used by laptops. (5) It was not important enough to be implemented by all clients -- nor *any* other client! (6) It was never documented. There is absolutely no excuse for a "stable" branch having this kind of crap in it at all. It's not stable. And there's no good reason for doing it *again* in development If you want to spend time on something, make sure the remaining method always works and is well documented. BTW, your poll is terribly inaccurate and incomplete. It forgot to mention the mouse quickselect that does the same things. And anybody that says they've used the feature (even seldom) is probably lying, since it crashed immediately in 2.1, and I'm not sure it worked in earlier versions. They may have used it in a warclient, but that's irrelevant to this project. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > William Allen Simpson wrote: > http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > > Jason Dorje Short wrote: >> Also, it makes no technical sense to delete an existing keyboard command >> that duplicates a mouse command. We still have exactly the same >> problem we had before but without the keyboard shortcut for it. Instead >> we should fix the commands so that they don't have two separate backend >> implementations, and fix them to work properly. >> > Presumably, the mouse command actually works (at least the code is much > longer). I didn't find a bug report for it. So, no, we don't have "exactly > the same problem". Please re-read the code. The crash problem having been easily solved, the remaining problem becomes what to do when multiple units are selected. > And tell us where the mouse command is documented?!?!? Excellent point. I suspect a thorough review of all commands and their documentation would be in order. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > On 12/3/07, Jason Dorje Short <[EMAIL PROTECTED]> wrote: > > http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > > William Allen Simpson wrote: > > > Since this was broken (by you in PR#14365) circa Dec 2005, and has had only > > one report (Aug 2007) in two (2) years, it is clearly not a commonly used > > command. It has never worked. It was never documented. It is not in > > either xaw or sdl. Going, going, gone > > Um, no. > > > First of all, deleting features from the stable branch is not something > that should be done lightly. Nor should you, in general, delete things > from any branch without the chance for discussion. > [...] I agree that this was done in too hasty a fashion. William, if you wanted this code gone so badly, you could at least have presented some kind of roadmap for reimplementation. To hear the opinion of the gaming commuity, I started a poll at: http://forum.freeciv.org/viewtopic.php?t=4218 ~Daniel ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Jason Dorje Short wrote: > Also, it makes no technical sense to delete an existing keyboard command > that duplicates a mouse command. We still have exactly the same > problem we had before but without the keyboard shortcut for it. Instead > we should fix the commands so that they don't have two separate backend > implementations, and fix them to work properly. > Presumably, the mouse command actually works (at least the code is much longer). I didn't find a bug report for it. So, no, we don't have "exactly the same problem". Please re-read the code. And tell us where the mouse command is documented?!?!? ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > William Allen Simpson wrote: > Since this was broken (by you in PR#14365) circa Dec 2005, and has had only > one report (Aug 2007) in two (2) years, it is clearly not a commonly used > command. It has never worked. It was never documented. It is not in > either xaw or sdl. Going, going, gone Um, no. First of all, deleting features from the stable branch is not something that should be done lightly. Nor should you, in general, delete things from any branch without the chance for discussion. Secondly, there was discussion, which you ignored. If we were to continue the discussion, it would go something like this: the bug was reported shortly after 2.1 was released and has since been a continual complaint on the forums (I'm told); therefore it clearly IS a command that is used. Also, it makes no technical sense to delete an existing keyboard command that duplicates a mouse command. We still have exactly the same problem we had before but without the keyboard shortcut for it. Instead we should fix the commands so that they don't have two separate backend implementations, and fix them to work properly. Please rethink your unreasonable position. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > [jdorje - Dim. Déc. 02 01:59:36 2007]: > > Your last patch looks like it does what it says it does. And I agree > the current code is broken (modifying a unit list inside the list > iterator is a no-no, that's what iterate_safe is for, but even with > iterate_safe the behavior is broken as it just selected different units > repeatedly). > > What I question is why you'd want the behavior to be that way when there > are multiple units selected. Say I have 5 units on 5 different tiles > selected...what use would be a quickselect that gives me 5 different > land units, one per tile? I thought the purpose of quickselect was to > give just ONE unit for quick action in multiplayer games. > > In that context I would think the key_quickselect action should just > pick one unit on one tile to bring into focus. The question then > becomes (when the battlegroup spans multiple tiles), which tile to > choose. I'm no expect on multiplayer freeciv but I'd think the tile > nearest the center of the screen would be the most likely to be the one > the player wants. > > -jason > > I thought about it a long time. I think the last issue I found was the more practical/nearer than the current way... ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Committed S2_1 revision 14112. Committed S2_2 revision 14113. Committed trunk revision 14114. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Jason Dorje Short wrote: > No need to read the code, just ask anyone who plays multiplayer. > The code itself is canonical. > The quickselect is intended to pick a quick unit from the current stack > to do a quick action. > Wrong. The code comments explicitly say "Focus unit is excluded." The code matches the comments [client/control.c line 2028]: if (game.player_ptr != unit_owner(punit) || unit_is_in_focus(punit)) { continue; } Perhaps that careless misunderstanding is how the code was so badly done (but doesn't explain how it was egregiously never tested before commit). > The / and * keys duplicate the behavior of the RMB and LMB. This should > be documented and those key commands added to the menu. And while we're > at it we should pick keys that aren't so random. > We should eliminate duplicate functions. They are confusing and cause extra work to document and maintain. Since this was broken (by you in PR#14365) circa Dec 2005, and has had only one report (Aug 2007) in two (2) years, it is clearly not a commonly used command. It has never worked. It was never documented. It is not in either xaw or sdl. Going, going, gone ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > William Allen Simpson wrote: > Reading the code to try to divine the original intent, it makes no sense to > have a special purpose key to filter land or sea units from an existing > selected group. The only time that could possibly be used would be in a > port, and only when both ships and land units are present. > > This especially make no sense with battlegroups that can be on more than one > tile. More than one port? Battlegroups with both land and sea units? No need to read the code, just ask anyone who plays multiplayer. The quickselect is intended to pick a quick unit from the current stack to do a quick action. Since the most important action is around ports which almost always contain both land and sea units, the case you dismiss is actually one of the more common ones. The / and * keys duplicate the behavior of the RMB and LMB. This should be documented and those key commands added to the menu. And while we're at it we should pick keys that aren't so random. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Yet another of the myriad bugs introduced by PR#14365. I'm so sick and tired of cleaning up this battlegroup mess! These keys are undocumented intercepts (just as ESC) that appear only in the gui-gtk2 client. They are not in any menu. Apparently, their only real purpose is to crash. Only warclient folks would even notice them (probably by furiously trying every key). http://www.youtube.com/watch?v=TTwgNhX4BSo Looking at the proposed patches, they are inconsistent. None performs a semantically useful function. They fix the symptom without addressing the underlying problem. Reading the code to try to divine the original intent, it makes no sense to have a special purpose key to filter land or sea units from an existing selected group. The only time that could possibly be used would be in a port, and only when both ships and land units are present. This especially make no sense with battlegroups that can be on more than one tile. More than one port? Battlegroups with both land and sea units? (Before PR#14365, it would try to find a land or sea unit that wasn't currently selected. That's duplicated by LMB/RMB already in mapview for both gtk2 and xaw. But not documented!) It's a waste of time to maintain such undocumented and infrequently useful keys I'm deleting the code. Index: client/control.c === --- client/control.c(revision 14109) +++ client/control.c(working copy) @@ -1990,8 +1990,8 @@ /** Quickselecting a unit is normally done with left, right click, - or keypad / * for the current tile. Bypassing the stack popup is quite - convenient, and can be tactically important in furious multiplayer games. + for the current tile. Bypassing the stack popup is quite convenient, + and can be tactically important in furious multiplayer games. **/ static struct unit *quickselect(struct tile *ptile, enum quickselect_type qtype) @@ -2770,15 +2770,3 @@ { request_toggle_fog_of_war(); } - -/** -... -**/ -void key_quickselect(enum quickselect_type qtype) -{ - unit_list_iterate(get_units_in_focus(), punit) { -struct unit *punit2 = quickselect(punit->tile, qtype); - -set_unit_focus_and_select(punit2); - } unit_list_iterate_end; -} Index: client/gui-gtk-2.0/gui_main.c === --- client/gui-gtk-2.0/gui_main.c (revision 14109) +++ client/gui-gtk-2.0/gui_main.c (working copy) @@ -595,14 +595,6 @@ key_city_workers(w, ev); break; - case GDK_KP_Divide: -key_quickselect(SELECT_SEA); -break; - - case GDK_KP_Multiply: -key_quickselect(SELECT_LAND); -break; - default: return FALSE; } Index: client/control.h === --- client/control.h(revision 14109) +++ client/control.h(working copy) @@ -169,7 +169,6 @@ void key_city_outlines_toggle(void); void key_map_grid_toggle(void); void key_map_borders_toggle(void); -void key_quickselect(enum quickselect_type qtype); void key_recall_previous_focus_unit(void); void key_unit_move(enum direction8 gui_dir); void key_unit_airbase(void); ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > Your last patch looks like it does what it says it does. And I agree the current code is broken (modifying a unit list inside the list iterator is a no-no, that's what iterate_safe is for, but even with iterate_safe the behavior is broken as it just selected different units repeatedly). What I question is why you'd want the behavior to be that way when there are multiple units selected. Say I have 5 units on 5 different tiles selected...what use would be a quickselect that gives me 5 different land units, one per tile? I thought the purpose of quickselect was to give just ONE unit for quick action in multiplayer games. In that context I would think the key_quickselect action should just pick one unit on one tile to bring into focus. The question then becomes (when the battlegroup spans multiple tiles), which tile to choose. I'm no expect on multiplayer freeciv but I'd think the tile nearest the center of the screen would be the most likely to be the one the player wants. -jason ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > > [pepeto - Tue Aug 21 21:40:57 2007]: > > I got a crash. I selected many units with 'y', and then I typed on a > quickselect key '*' or '/': > Can reproduce like this on 2.1.1 win32-gtk2: select at least three of the same kind of unit and press '*' - client crash! ~Daniel ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > New patch: It selects new units on the tiles of the current units in focus. It selects only 1 unit by tile, to avoid unit selection duplication . Index: client/control.c === --- client/control.c(révision 13752) +++ client/control.c(copie de travail) @@ -2726,13 +2726,41 @@ } /** -... + Selects new units on the same tiles than the previous selected ones. + Allows only one unit by tile. **/ void key_quickselect(enum quickselect_type qtype) { + struct unit_list *punits = unit_list_new(); + struct unit *punit2; + bool tiles[MAP_INDEX_SIZE], first; + + memset(tiles, 0, sizeof(tiles)); + /* Collect the units */ unit_list_iterate(get_units_in_focus(), punit) { -struct unit *punit2 = quickselect(punit->tile, qtype); +if (tiles[punit->tile->index]) { + /* One unit by tile only */ + continue; +} -set_unit_focus_and_select(punit2); +punit2 = quickselect(punit->tile, qtype); +if (punit2) { + unit_list_append(punits, punit2); +} +tiles[punit->tile->index] = TRUE; } unit_list_iterate_end; + + /* Set them in the focus */ + first = TRUE; + unit_list_iterate(punits, punit) { +if (first) { + set_unit_focus_and_select(punit); + first = FALSE; +} else { + add_unit_focus(punit); +} + } unit_list_iterate_end; + + unit_list_unlink_all(punits); + unit_list_free(punits); } Index: client/control.c === --- client/control.c (révision 13752) +++ client/control.c (copie de travail) @@ -2726,13 +2726,41 @@ } /** -... + Selects new units on the same tiles than the previous selected ones. + Allows only one unit by tile. **/ void key_quickselect(enum quickselect_type qtype) { + struct unit_list *punits = unit_list_new(); + struct unit *punit2; + bool tiles[MAP_INDEX_SIZE], first; + + memset(tiles, 0, sizeof(tiles)); + /* Collect the units */ unit_list_iterate(get_units_in_focus(), punit) { -struct unit *punit2 = quickselect(punit->tile, qtype); +if (tiles[punit->tile->index]) { + /* One unit by tile only */ + continue; +} -set_unit_focus_and_select(punit2); +punit2 = quickselect(punit->tile, qtype); +if (punit2) { + unit_list_append(punits, punit2); +} +tiles[punit->tile->index] = TRUE; } unit_list_iterate_end; + + /* Set them in the focus */ + first = TRUE; + unit_list_iterate(punits, punit) { +if (first) { + set_unit_focus_and_select(punit); + first = FALSE; +} else { + add_unit_focus(punit); +} + } unit_list_iterate_end; + + unit_list_unlink_all(punits); + unit_list_free(punits); } ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39602) quickselect doesn't like the selection of many units
http://bugs.freeciv.org/Ticket/Display.html?id=39602 > I got a crash. I selected many units with 'y', and then I typed on a quickselect key '*' or '/': #0 quickselect (ptile=0x101, qtype=SELECT_LAND) at ../utility/speclist.h:110 listsize = panytransporter = panymovesea = panysea = panymoveland = panyland = panymoveunit = panyunit = __PRETTY_FUNCTION__ = "quickselect" #1 0x0041a79e in key_quickselect (qtype=SELECT_LAND) at control.c:2704 punit2 = myiter = (struct genlist_link *) 0x1 #2 0x004b86ae in keyboard_handler (w=0x9010a0, ev=0x1ee2b10, data=) at gui_main.c:587 No locals. #3 0x2afa41d1f68d in _gtk_marshal_BOOLEAN__BOXED () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #4 0x2afa444187da in g_closure_invoke () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #5 0x2afa44428408 in ?? () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #6 0x2afa44429617 in g_signal_emit_valist () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #7 0x2afa44429a13 in g_signal_emit () from /usr/lib/libgobject-2.0.so.0 No symbol table info available. #8 0x2afa41e1d13e in ?? () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #9 0x2afa41d18d45 in gtk_propagate_event () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #10 0x2afa41d19c91 in gtk_main_do_event () from /usr/lib/libgtk-x11-2.0.so.0 No symbol table info available. #11 0x2afa421bf45c in ?? () from /usr/lib/libgdk-x11-2.0.so.0 No symbol table info available. #12 0x2afa44a85a14 in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0 No symbol table info available. #13 0x2afa44a8885d in ?? () from /usr/lib/libglib-2.0.so.0 No symbol table info available. #14 0x2afa44a88b6a in g_main_loop_run () from /usr/lib/libglib-2.0.so.0 No symbol table info available. #15 0x2afa41d1a023 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0 #16 0x004ba44a in ui_main (argc=1, argv=0x7fff690ed898) at gui_main.c:1424 home = sig = style = #17 0x00413f35 in main (argc=, argv=0x7fff690ed898) at civclient.c:357 i = 1 loglevel = 2 ui_options = ui_separator = option = user_tileset = false /** ... **/ void key_quickselect(enum quickselect_type qtype) { unit_list_iterate(get_units_in_focus(), punit) { struct unit *punit2 = quickselect(punit->tile, qtype); set_unit_focus_and_select(punit2); } unit_list_iterate_end; } This function is all wrong. If you set a unit in focus whereas you are iterating the same list, there will be in this function some free links. set_unit_focus_and_select() will call unit_list_unlink_all(), then the links which are always in the key_quickselect() function will be wrong. Anyway I don't think it's a good idea to allow quickselect with many selected units. I propose 2 patches... --- client/control.c.old 2007-08-21 23:59:24.0 +0200 +++ client/control.c 2007-08-21 23:59:31.0 +0200 @@ -2709,9 +2709,13 @@ **/ void key_quickselect(enum quickselect_type qtype) { - unit_list_iterate(get_units_in_focus(), punit) { -struct unit *punit2 = quickselect(punit->tile, qtype); + struct unit_list *punits = get_units_in_focus(); + struct unit *punit; -set_unit_focus_and_select(punit2); - } unit_list_iterate_end; + if (unit_list_size(punits) == 0) { +return; + } + + punit = unit_list_get(punits, 0); + set_unit_focus_and_select(quickselect(punit->tile, qtype)); } --- client/control.c.old 2007-08-21 23:59:24.0 +0200 +++ client/control.c 2007-08-22 00:04:00.0 +0200 @@ -2709,9 +2709,15 @@ **/ void key_quickselect(enum quickselect_type qtype) { - unit_list_iterate(get_units_in_focus(), punit) { -struct unit *punit2 = quickselect(punit->tile, qtype); + struct unit_list *punits = get_units_in_focus(); + struct unit *punit; + int size = unit_list_size(punits); -set_unit_focus_and_select(punit2); - } unit_list_iterate_end; + if (size == 1) { +punit = unit_list_get(punits, 0); +set_unit_focus_and_select(quickselect(punit->tile, qtype)); + } else if (size > 1) { +append_output_window(_("You can use 'quick select' " + "when many units are selected.")); + } } ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev