patchFreeciv - Patches: patch #3887, Use UCF_MISSILE to identify...

 
 
Show feedback again

patch #3887: Use UCF_MISSILE to identify missiles for pathfinding

Submitted by:  Emmet Hikory <persia>
Submitted on:  Mon 29 Apr 2013 10:29:48 AM UTC  
 
Category: aiPriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: pepeto <pepeto>Open/Closed: Closed
Planned Release: 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.

 

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

Mon 20 Jan 2014 11:04:59 PM UTC, SVN revision 24176:

Path-finding : Use UCF_MISSILE to determine whether fueled unit can do
suicidal attacks.

Patch by Emmet Hikory

See gna patch #3887

(Browse SVN revision 24176)

pepeto <pepeto>
Project MemberIn charge of this item.
Tue 14 Jan 2014 10:25:25 PM UTC, comment #5:

> For the pf_fuel_map_iterate() hunk, I'm not entirely sure why
> the "|| 0 >= moves_left" check exists, so I've left it, but I
> suspect it of causing AI missiles not to attack as much as they
> might. Explanations or suggestions to drop welcomed.


I think this test is not right, it should check only "|| 0 > moves_left" to ensure to don't have negative moves left due to many targets in the same path.

Else, I have kept your entire work.

(file #19749)

pepeto <pepeto>
Project MemberIn charge of this item.
Thu 23 May 2013 05:34:06 PM UTC, comment #4:

I believe the apparent issue with bomber handling is related to the current get_MC() callbacks not providing the correct value for the cost of an attack for UTYF_ONEATTACK units, rather than being an issue in pf_fuel_map_attack_is_possible(). If airmove() is modified to charge the remainder of that turn's possible moves for these units, I don't believe the bomber logic needs further changes in pf_fuel_map_attack_is_possible().

One possible way to handle this is that from patch #3901 (in the attack_move_cost() function, although that depends on pf_fuel_map_adjust_cost() from patch #3897). Another would be to perform similar conditionals and modulo math inside airmove(), preserving the current nativity issues.

With correct attack cost charges, pf_fuel_map_iterate() should only allow attack in cases where there would be sufficient fuel to return to base after the attack.

Emmet Hikory <persia>
Project Member
Wed 22 May 2013 11:53:44 AM UTC, comment #3:

When I wrote this part of the code, there weren't a UCF_MISSILE unit class flag.

I agree that bomber hanlding may be incorrect in pf_fuel_map_attack_is_possible(), I will investigate more.

I can't remember why had I added the line "|| 0 >= moves_left" for fixing bug #15244, I will investigate.

pepeto <pepeto>
Project MemberIn charge of this item.
Thu 02 May 2013 11:32:14 PM UTC, comment #2:

In pf_fuel_map_iterate(), the function exits early in the event that there aren't enough moves to return to a refueling point unless either the unit in question is a missle or it has no moves anyway (as mentioned, I don't really understand the purpose of this OR condition: I would think it just wastes cycles processing the rest of the function for units that can't move anyway). If the unit is a missile, we can perform suicidal attacks (we don't care if we will later be able to return to base because we wouldn't survive anyway). We later exit early in the event that we're targeting an enemy tile and wouldn't be safe to attack (with pf_fuel_map_attack_is_possible()).

In pf_fuel_map_attack_is_possible(), we check to see if it makes sense for the unit to attack: for missiles, we always attack (the unit would die anyway, so we don't care about fuel), for bombers, we only attack if we have more moves left than our move_rate (moves_left is initialised with pf_move_rate(), so fueled units are treated as having many more moves per turn than move_rate to simplify the pathfinding logic), which means that we are not on the last turn of fuel. If we are on the last turn of fuel, we don't attack, otherwise it is safe to attack.

I agree there may be remaining issues with bombers (the logic doesn't check to make sure that there will be enough moves_left to return to base after the attack has been performed), but this patch doesn't change the conditionals for bombers except in the case where fuel == 1, in which case it is a bit more conservative (and won't actually attack: committing suicide is only permitted for UCF_MISSILE units).

Emmet Hikory <persia>
Project Member
Thu 02 May 2013 08:47:35 PM UTC, comment #1:

Reading the patch file only (not checking larger context) I suspect that Bomber handling is not quite right. You are not checking fuel (=how many turns flight time left) at all, but only how much movement for this particular turn is left, when you deduct that this is last turn (running out of fuel) and unit cannot both attack and return to base.

Marko Lindqvist <cazfi>
Project Administrator
Mon 29 Apr 2013 10:29:48 AM UTC, original submission:

The current code uses the combination of UTYF_ONEATTACK and fuel==1 to determine that a given fueled unit is a missile. This is suboptimal, as it causes units that don't need to die in the attack consider suicide an acceptable option, which ought be something done purposefully, rather than as a side-effect of pathfinding being confusing.

This patch changes the two places that are checks for missiles to use UCF_MISSILE instead. For the pf_fuel_map_iterate() hunk, I'm not entirely sure why the "|| 0 >= moves_left" check exists, so I've left it, but I suspect it of causing AI missiles not to attack as much as they might. Explanations or suggestions to drop welcomed.

One side effect of this patch is that units that are UTYF_ONEATTACK, have fuel == 1, and are not UCF_MISSILE will only attack from pathfinding when they are currently on a refueling point (which I think appropriate, but some ruleset author might consider unexpected).

Emmet Hikory <persia>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #19749:  pf_UCF_MISSILE.diff added by pepeto (2kB - text/x-diff)
file #17855:  use-UCF_MISSILE-for-missile-ID.patch added by persia (2kB - application/octet-stream)

 

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 cazfi (Posted a comment)
  • -unavailable- added by persia (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
    Mon 20 Jan 2014 11:05:16 PM UTCpepetoStatusReady For Test=>Done
      Open/ClosedOpen=>Closed
    Tue 14 Jan 2014 10:25:25 PM UTCpepetoAttached File-=>Added pf_UCF_MISSILE.diff, #19749
      StatusIn Progress=>Ready For Test
      Planned Release=>2.6.0
    Wed 22 May 2013 11:53:44 AM UTCpepetoStatusNone=>In Progress
      Assigned toNone=>pepeto
    Mon 29 Apr 2013 10:29:48 AM UTCpersiaAttached File-=>Added use-UCF_MISSILE-for-missile-ID.patch, #17855
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup