Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Bug #923677

Fuel pathfinder: bug of always waiting in departure airdrome

Added by Alexandro Ignatiev 6 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Category:
General
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

The effect of this bug harms both AI (main cause of no ability to use fighters) and players' goto paths (just when aircraft has full moves "wait" order is skipped). The origin:

1. pf_fuel_map_new initializes the starting node in NS_PROCESSED status with no segment.
2. the iterator never assigns segments to nodes in NS_PROCESSED status.
3. refueling nodes with no segment are considered waited.

I think the algorithm may worth a capital rethinking but this severe bug should be dealed with even before.

fix-fueled-wait.patch (3.33 KB) fix-fueled-wait.patch Alexandro Ignatiev, 2021-03-24 08:12 PM
2.6-fix-fueled-wait.patch (3.18 KB) 2.6-fix-fueled-wait.patch Alexandro Ignatiev, 2021-03-24 08:13 PM
fuel-dont-wait.patch (4.18 KB) fuel-dont-wait.patch Alexandro Ignatiev, 2021-03-28 01:32 PM
fuel-dont-wait.patch (4.92 KB) fuel-dont-wait.patch Alexandro Ignatiev, 2021-04-03 11:40 PM

Related issues

Has duplicate Freeciv - Bug #714400: Goto postpones move for fighters with reduced move pointsClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocks Freeciv - Feature #658896: Restore AI ability to build Fighters (fuel = 1 units)Closed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Alexandro Ignatiev 6 months ago

  • Blocks Feature #658896: Restore AI ability to build Fighters (fuel = 1 units) added

#2 Updated by Alexandro Ignatiev 6 months ago

Hmm, just creating init_node->segment=pf_pos_ref(init_node->pos=pf_pos_replace(NULL, init_node)) does not do the thing...

#3 Updated by Alexandro Ignatiev 6 months ago

Resolved. Other problems were:
  • Waiting was applied to a node as soon as we saw that we might ever need it here, not when we have processed all path shorter than the waiting and start to look at paths with waiting. Thus, the paths passing by the node without waiting use it.
  • pf_fuel_pos.dir_to_here and pf_fuel_node.dir_to_here had different signed treatment. So, when the segment tracer encountered a refuel node with dir_to_here = direction8_invalid(), it compared 8 to -8 and concluded that the pos of the node is not actual, and marked for the path tracer that waiting was arranged here.
    The patches are basically the same as is the code, just applying master patch to 2.6 fails for unknown reason.

#4 Updated by Alexandro Ignatiev 6 months ago

Hmmmm... But if we iterate the map behind the waiting cost (that air AI likely would do) and then extract a shorter path, seems to me like it will still use the waiting... Need some more fixture here...

#5 Updated by Alexandro Ignatiev 6 months ago

Ah, understood how it works, it does not happen (segment of fuel nodes is written only once without waiting and is equal to backtraced positions for paths not including waiting there). Just added an assertion, it did not fire in autogames.

#6 Updated by Marko Lindqvist 6 months ago

  • Sprint/Milestone changed from 2.6.4 to 2.6.5

#7 Updated by Marko Lindqvist 6 months ago

About making dir_to_here unsigned: Why you need to do that, and isn't direction8_invalid() a negative value?

I'd prefer that since you need 'waited' outside the if-block anyway, you wouldn't do assignment in if() condition:

if ((waited = (NS_WAITING node->status))) {

->

waited = (NS_WAITING node->status);
if (waited) {

That "#ifdef PF_DEBUG ... #endif" thing in advgoto.c will never be compiled as PF_DEBUG is defined inside path_finding.c only.

#8 Updated by Alexandro Ignatiev 6 months ago

Marko Lindqvist wrote:

About making dir_to_here unsigned: Why you need to do that, and isn't direction8_invalid() a negative value?

We could as well make it positive, it should just be done the same way in both structs (-8 into unsigned 4 bits gives 8 without warning), just I needed to edit the header file for it.

That "#ifdef PF_DEBUG ... #endif" thing in advgoto.c will never be compiled as PF_DEBUG is defined inside path_finding.c only.

Well, yes. I'll run some autogames with it unwrapped. If we'll write pathfinder for triremes and helicopters, we will need such waits though, so maybe I'll wrap them into another ifdef defined in path_finding.h.

#9 Updated by Alexandro Ignatiev 6 months ago

Improved patch.

#10 Updated by Marko Lindqvist 6 months ago

  • Status changed from New to Resolved
  • Assignee set to Marko Lindqvist

Thanks.

To make future submissions even better, you could start adding ticket reference to the commit messages. ("See hrm #????" / "See osdn #????")

#11 Updated by Marko Lindqvist 6 months ago

Does this fix the problem with AI units with "Coast" flag that never leave their home port?

#12 Updated by Alexandro Ignatiev 6 months ago

Marko Lindqvist wrote:

Does this fix the problem with AI units with "Coast" flag that never leave their home port?

Yes, I've seen triremes on seas in civ2 when ran this patch without fuel=1 unit blocking.

To make future submissions even better, you could start adding ticket reference to the commit messages. ("See hrm #????" / "See osdn #????")

Thanks, I'll do it in the future.

#13 Updated by Marko Lindqvist 6 months ago

  • Status changed from Resolved to Closed

#14 Updated by Marko Lindqvist 6 months ago

Soon after this went in, there's fuel(?) related autogame assert failure: https://osdn.net/projects/freeciv/ticket/41940

Can you have a look (there's no reproducible case yet, but I'll attach my autogame to the ticket if it reproduces the problem) ?

#15 Updated by Alexandro Ignatiev 6 months ago

Hmm not yet launched it but... There is some code inteded to protect autosettlers from approaching enemy units... Maybe it sometimes fires this assertion.

#16 Updated by Marko Lindqvist 3 months ago

  • Has duplicate Bug #714400: Goto postpones move for fighters with reduced move points added

Also available in: Atom PDF