[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Update of bug #20842 (project freeciv): Status: Ready For Test = Fixed Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #10, bug #20842 (project freeciv): Anyway, I started testing your patch on 2.4-beta-2, and it seems to work without problem. I encountered some problems to reproduce with version = 2.4, I didn't find the reason. But this bug is still reproducible when an enemy unit moves near your unit stack (you also can simulate it with adding a unit near your owns with editor). So I am attaching the version for these branches. I think I am still missing something with ACTIVITY_IDLE. Is the comment in packand.c you mentioned still up to date? I doubt about it, because for normal actions such as you described, units should be put into the urgent focus queue. I have tried to investigate. As far as I saw, since revision 8870 for PR#10695, the client doesn't set ACTIVITY_IDLE directly anymore. I propose to remove this obsolete comment (patch attached). (file #19707, file #19708) ___ Additional Item Attachment: File name: 0001-Remove-units-which-aren-t-IDLE-from-the-urgent-focus.patch Size:3 KB File name: 0002-Removed-obsolete-comment-r8870.patch Size:1 KB ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Update of bug #20842 (project freeciv): Planned Release: 2.3.5, 2.4.0, 2.5.0, 2.6.0 = 2.3.5, 2.4.1, 2.5.0, 2.6.0 ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #9, bug #20842 (project freeciv): I think you are correct. Your patch looks much better than mine: it does the test at a better location, avoiding code duplication, and handle the order list as well. Very good! As for why I thought only sentry activity orders were concerned, I misunderstood the purpose of the code which I pointed to as a justification for the bug only happening for sentry order. That code is the part which inserts units in the urgent list. It does not relate to the category of orders affected by the bug after a unit is woken up. Oddly though, I never experienced it in any other situation than the sentry order when playing the game. Anyway, I started testing your patch on 2.4-beta-2, and it seems to work without problem. I'll continue testing and keep you posted if anything comes up. ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #8, bug #20842 (project freeciv): Your patch fix only when you apply sentry to units. But if you do fortify, the bug still exists. I have first thought to add your line in request_new_unit_activityXXX(), but it may be still broken if order lists have been requested. This would make the code hard to maintain. I have opted for another way : modifying how units are selected from the _urgent_focus_queue_. Could you test my patch on S2_3? I think I am still missing something with _ACTIVITY_IDLE_. Is the comment in packand.c you mentioned still up to date? I doubt about it, because for normal actions such as you described, units should be put into the urgent focus queue. (file #18059) ___ Additional Item Attachment: File name: S2_3_urgent_focus_queue.patch Size:2 KB ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Update of bug #20842 (project freeciv): Status: In Progress = Ready For Test Planned Release: = 2.3.5, 2.4.0, 2.5.0, 2.6.0 ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #7, bug #20842 (project freeciv): The modification seems effective on the latest beta (I see the same behaviour change). ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #4, bug #20842 (project freeciv): One last detail which I forgot to add: I think you could not reproduce the bug simply because it's triggered by the pace at which the server will process the packets it receives; for a relatively low number of units, the server will reply quickly enough to let the client updates the units status before it needs to select the next unit to focus on. My computer being fairly old, I can see the issue happening very early. Perhaps if you raise the number of units in my example by a significant amount (a hundred or so might do it), you would experience it as well. If you believe that such scenario is unlikely to happen, or you still cannot reproduce it, just disregard and close the bug report. Thanks. ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Update of bug #20842 (project freeciv): Status:None = In Progress Assigned to:None = pepeto ___ Follow-up Comment #5: Good analysis. This patch looks much better. Before preparing commit, could you affirm that only sentry order is affected by this bug? ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #6, bug #20842 (project freeciv): As far as I understand, the `urgent_focus_queue` is only updated in the case of a unit which was woken up by a server message out of sentry state to idle state (look at the test conditions around the use of `urgent_unit_focus` in `packhand.c`). My patch should be enough to handle the issue in the 2.3.4 version. I will checkout trunk to see how it may be relevant there. ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #3, bug #20842 (project freeciv): Your helpful answer, and further testing, led me to investigate again the issue which did not actually disappear after my first patch. As you rightfully indicated, the client should not expect the server to accept requests unilaterally, and thus should not update the state of an unit before receiving the server update. There seems to be one exception to that rule (see comments of function `handle_unit_packet_common` in client/packhand.c), but it doesn't concern the issue at hand. Basically, whenever a group of units is being awaken, the server will send a batch of packets to indicate that change to the client. In certain situations, the client will then put these units in the `client/control.c:urgent_focus_queue` list, so that these units get the player attention. However, when these units are put in sentry mode, they are not removed from that queue. Later, when the client looks for a unit to be put in focus, it will check `urgent_focus_queue` (see the `client/control.c:advance_unit_focus` function). My original patch was not far off it seems, and the new patch I hereby propose simply removes any unit put in sentry mode from the urgent queue. I provide also a minimal map to test the issue on the client before and after the patch. Let me restate the steps to reproduce the bug: - load up the map, - click on the sole unit stack there, - click on the 'ready all' button of the select dialog, - press 'shift-v' to select the whole unit stack, - press 's' to put them all in sentry again The above steps consistently produce the issue on my computer. Also, a small log of the packets exchanged between the client and the server is provided, with light annotations (and a few more debugging messages which I added to my own build) of the packet stream. It shows the packets which made units to be added to the urgent list (see packets '10' to '20', which correspond to the idle activity change notification from the server after pressing 'shift-v'), and the selection of the next focused unit (after packet '29'). (file #18034, file #18035) ___ Additional Item Attachment: File name: 0001-fix-focus-unit-list-when-unit-put-in-sentry.patch.gz Size:0 KB File name: test_case.tgz Size:10 KB ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
URL: http://gna.org/bugs/?20842 Summary: unit still in focus after put in sentry mode in batch Project: Freeciv Submitted by: didc Submitted on: ven. 24 mai 2013 13:42:08 GMT Category: client-gtk-2.0 Severity: 2 - Minor Priority: 5 - Normal Status: None Assigned to: None Originator Email: Open/Closed: Open Release: 2.3.4 Discussion Lock: Any Operating System: None Planned Release: ___ Details: When selecting a group of units (using Ctrl-V for instance), and putting them in sentry, the whole unit set is indeed put in sentry (one can see that their state as been updated, except for the one in focus), but focus still goes through all of them in turn. This is not happening every time, but it occurs under certain conditions which remain to be clearly identified. Step to reproduce: - start a new game - select all the units on the start tile (using V shortcut or the menu) - put them in sentry mode once (shortcut S) - select all the units again - put them in sentry mode Now you have to cycle through each unit even though they are supposed to be in sentry. This problem happens for the gtk2 client, but might also be there in other clients. You'll find a patch attached which solves the issue (potentially for any client). I am not sure if the solution offered is the most appropriate one: the issue does not happen for other orders (I just tried with irrigate) though it seemingly uses the same algorithm to update the units state. I must be missing something somewhere. ___ File Attachments: --- Date: ven. 24 mai 2013 13:42:08 GMT Name: fix-focus-unit-list-when-unit-put-in-sentry.tgz Size: 583 o By: didc http://gna.org/bugs/download.php?file_id=18002 ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #1, bug #20842 (project freeciv): Units should lose focus when the server changes their activity. I don't think it's a good solution to assume the server will accept the client request. PS: I failed to reproduce this bug. ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20842] unit still in focus after put in sentry mode in batch
Follow-up Comment #2, bug #20842 (project freeciv): Could you explain in which situations the server would refuse a request of activity change to sentry? I think this could help me better understand the issue I am experiencing (which may not be an issue but just normal behaviour). Regarding the bug reproduction, my explanations were perhaps not detailed enough or misleading in some way; let me know if certain steps seem ambiguous or inconsistent. The server seems to return an answer: once being put in sentry client side, each unit in the group appears as being in sentry mode, except for the one which ends up being focused on (I suppose that it also was in sentry, but got immediately woken up because of focus). Somehow, even though the order is accepted, the units are still iterated over in the current focus group. Btw, I am using stock version 2.3.4 compiled from source on debian squeeze. The packaged version is not installed anymore, but did expose the same issue. ___ Reply to this item at: http://gna.org/bugs/?20842 ___ Message posté via/par Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
