bugFreeciv - Bugs: bug #22136, Units working in teams building...

 
 
Show feedback again

bug #22136: Units working in teams building roads can get left behind

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Tue 03 Jun 2014 09:37:52 PM UTC  
 
Category: NoneSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: S2_5 r25043Operating System: GNU/Linux
Planned Release: 2.5.0, 2.6.0

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Sun 22 Jun 2014 10:03:42 AM UTC, comment #5:

Thank you for this general cleanup. I've had a look wrt things like bug #16188 and I think it's correct.

Jacob Nevins <jtn>
Project Administrator
Wed 18 Jun 2014 09:49:54 AM UTC, SVN revision 25196:

update_unit_activity() fixes:

  • Do not use clears silently the orders of the units ;
  • Really stop units doing illegal activities.

From a report by Jacob Nevins (jtn@gna)

See gna bug #22136

(Browse SVN revision 25196)

pepeto <pepeto>
Project MemberIn charge of this item.
Wed 18 Jun 2014 09:49:46 AM UTC, SVN revision 25195:

update_unit_activity() fixes:

  • Do not use clears silently the orders of the units ;
  • Really stop units doing illegal activities ;
  • Do not check for adjacent units if no activity has been completed.

From a report by Jacob Nevins (jtn@gna)

See gna bug #22136

(Browse SVN revision 25195)

pepeto <pepeto>
Project MemberIn charge of this item.
Sun 15 Jun 2014 08:53:31 PM UTC, comment #2:

I found a bunch of bugs investigating for this one:

  • At the end of update_unit_activity(), there are some very old lines (I think they are obsolete):
    • There is no reason to have these lines, as illegal activities are already supposed to be caught above...
    • Using unit_activity_handling() clears silently the orders of the units.
  • They are some erroneous piece of codes which looks to:

But 'punit' can be iterated as 'punit2'. So units which will be iterated after 'punit' can still do the base activity because 'activity_target' has been overwritten. But thanks to the previous item, this case was handled nearly corrected.

  • There is no reason to check adjacent units if no activity was done.

The attached patches should fix a part of the problem. But to be completed, we should also apply patch #4807.


> I wondered whether not setting done_moving here in
> execute_orders(), like we do everywhere else, might be
> significant [...] but it made no difference. Perhaps we should
> fix it anyway, although I think it's probably harmless at the
> moment (unless I've missed some reason why it's deliberate).


I don't think this could change anything because unit already has no moves left...

(file #21043, file #21044)

pepeto <pepeto>
Project MemberIn charge of this item.
Tue 03 Jun 2014 11:16:04 PM UTC, comment #1:

Random thoughts. (I haven't tried debugging this properly.)

I wondered if this could be caused by the bit in update_unit_activity() on work completion where all units working on the same tile (including the one who finished the work) have their activity set to IDLE. But I don't see why all units wouldn't just carry on with execute_orders() in this case.

I wondered whether not setting done_moving here in execute_orders(), like we do everywhere else, might be significant:

but it made no difference. Perhaps we should fix it anyway, although I think it's probably harmless at the moment (unless I've missed some reason why it's deliberate).

Jacob Nevins <jtn>
Project Administrator
Tue 03 Jun 2014 09:37:52 PM UTC, original submission:

While testing patch #2206, I found that if I set a pair of units building the same road, at some point before the road was completed one of the units would forget its orders and sit there in the middle of the road, while the other went on alone.

See attached savegame; keep hitting "turn done" and at T4, one of the workers stops doing anything despite having previously had orders to build a road to some distance away.

Jacob Nevins <jtn>
Project Administrator

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #20920:  roadtest.sav.bz2 added by jtn (15kB - application/x-bzip - S2_5 savegame)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by pepeto (Updated the item)
  • -unavailable- added by jtn (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 18 Jun 2014 09:50:11 AM UTCpepetoStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Sun 15 Jun 2014 08:53:31 PM UTCpepetoAttached File-=>Added trunk_update_unit_activity.patch, #21043
      Attached File-=>Added S2_5_update_unit_activity.patch, #21044
      StatusNone=>Ready For Test
      Planned Release=>2.5.0, 2.6.0
    Wed 04 Jun 2014 06:48:30 AM UTCpepetoAssigned toNone=>pepeto
    Tue 03 Jun 2014 09:37:52 PM UTCjtnAttached File-=>Added roadtest.sav.bz2, #20920
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup