bugFreeciv - Bugs: bug #21938, ORDER_FULL_MP waits if unit's move...

Show feedback again

bug #21938: ORDER_FULL_MP waits if unit's move rate is reduced after orders laid in

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sat Apr 19 16:32:58 2014  
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: Operating System: Any
Planned Release: 2.4.4, 2.5.0, 2.6.0Contains string changes: None

Add a New Comment (Rich MarkupRich Markup):

You are not logged in

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


(Jump to the original submission Jump to the original submission)

Fri Sep 5 08:14:33 2014, SVN revision 26227:

Make ORDER_FULL_MP waiting only when the moves left of a unit is inferior to
its move rate.

Report and method by Jacob Nevins

See gna bug #21938

(Browse SVN revision 26227)

pepeto <pepeto>
Project MemberIn charge of this item.
Fri Sep 5 08:14:27 2014, SVN revision 26226:

Make ORDER_FULL_MP waiting only when the moves left of a unit is inferior to
its move rate.

Report and method by Jacob Nevins

See gna bug #21938

(Browse SVN revision 26226)

pepeto <pepeto>
Project MemberIn charge of this item.
Fri Sep 5 08:14:21 2014, SVN revision 26225:

Make ORDER_FULL_MP waiting only when the moves left of a unit is inferior to
its move rate.

Report and method by Jacob Nevins

See gna bug #21938

(Browse SVN revision 26225)

pepeto <pepeto>
Project MemberIn charge of this item.
Wed Sep 3 14:34:16 2014, comment #3:

Patch attached.

(file #22010)

pepeto <pepeto>
Project MemberIn charge of this item.
Mon May 19 08:40:26 2014, comment #2:

'<' looks the correct thing.

I don't think we should cancel path or recalculate path if the move rate was changed. Or at least not at server side (anyway the AI recalculates path every turns). And I think this is a separated issue.

Not that computed paths have many other ways to become obsolete (terrain changes, infrastructure built/removed, non-allied unit moves etc.).

pepeto <pepeto>
Project MemberIn charge of this item.
Sat Apr 19 16:57:59 2014, comment #1:

To my mind, changing this to be >= rather than != would be a safe and harmless change. Note that pathfinding is going to ignore any potentially available extra moves (as it considers the path for the unit class, with some type attributes, but not the path for the unit itself), so the unit will only benefit from unit_move_rate() moves for any calculated paths.

For the second issue, units should probably recalculate paths for GoTo and Connect on any turn change and on any change in move rate (one can easily gain a technology mid-turn, which has examples in the default rules both increasing and decreasing move_rate (decrease through wonder obsolescence)). While critical for fueled units who might otherwise be destroyed due to inability to complete given orders, such reconsideration is likely to cause other units to take advantage of changing conditions (settler passing through a city decides to use newly created road, rather than climbing the mountains, etc.). Dunno what the right UI for this is though: the current UI shows the planned path, and folk who really care about details might get annoyed if that wasn't the followed path.

My personal preference would be to not cancel orders just because the move rate changed, if the unit can still complete the orders, or otherwise reach the final destination within approximately the same number of turns. When fighting an air-supported war on multiple fronts, I generally assign aircraft to fronts at build-time, expecting them to arrive to battle in perhaps 4-6 turns. If the path is interrupted, I have no context to know where that aircraft wanted to go (and while I'd rather be able to recover the unit than have it destroyed, I don't want to have to remanage all my units just because I learned a new tech or finally completed a wonder, or whatever).

Emmet Hikory <persia>
Project Member
Sat Apr 19 16:32:58 2014, original submission:

While investigating bug #21932 I came across this in server-side execute_orders():

The != condition looks like it's intended for the common case where moves_left < move_rate, but it looks suspicious to me in the case where a unit's moves_left is greater than its current move_rate (due to the loss this turn of some Move_Bonus effect from wonder or tech loss). The unit will waste its current excess of movement points sitting around doing nothing.

On the other hand, surely the client-side pathfinding algorithm wouldn't have inserted ORDER_FULL_MP in any such case for the current turn, so it seems harmless. (Indeed, this check should never fail for input from the pathfinding algorithm, I'd have thought.)

And if a unit's move rate reduces, any pre-calculated orders are potentially dangerously wrong, for fueled units. So it's not clear to me what would be better to put here. (Arguably we should try to cancel orders of units whose move rate reduces.)

Raising the bug to get others' opinion; it might well get closed Wont Fix or Invalid.

Jacob Nevins <jtn>
Project Administrator


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

Attach File(s):

Attached Files


Depends on the following items: None found

Items that depend on this one: None found


Carbon-Copy List
  • -unavailable- added by pepeto (Posted a comment)
  • -unavailable- added by persia (Posted a comment)
  • -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.


    Error: not logged in



    Follow 8 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Fri Sep 5 08:14:45 2014pepetoStatusReady For Test=>Fixed
    Wed Sep 3 14:34:16 2014pepetoAttached File-=>Added execute_ORDER_FULL_MP.patch, #22010
      StatusIn Progress=>Ready For Test
      Planned Release=>2.4.4, 2.5.0, 2.6.0
    Tue Sep 2 14:06:45 2014pepetoAssigned toNone=>pepeto
    Tue Sep 2 14:06:44 2014pepetoStatusNeed Info=>In Progress
    Show feedback again

    Back to the top

    Powered by Savane 3.1-cleanup