bugFreeciv - Bugs: bug #22187, Client allows attempted violation...

 
 
Show feedback again

bug #22187: Client allows attempted violation of embarks/disembarks restrictions

Submitted by:  Jacob Nevins <jtn>
Submitted on:  Sat 14 Jun 2014 11:57:30 AM UTC  
 
Category: generalSeverity: 3 - Normal
Priority: 5 - NormalStatus: Fixed
Assigned to: pepeto <pepeto>Open/Closed: Closed
Release: S2_5 r25141Operating System: None
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 06 Oct 2014 10:38:31 PM UTC, comment #15:

> Yes, it is unfixed in S2_5. That's the reason I have kept it
> open.

...but from comment #10 - comment #12 it sounds like it's hard to backport.

I think this remains a relatively cosmetic issue; on S2_5, the client [G]oto line indicates that you can load riflemen onto a helicopter over ocean, or that riflemen can unload from a helicopter en route; but when you try it you get "Orders for Riflemen aborted because of failed move". (Tested with S2_5 r26716 + pending pathfinding patches; and tested that trunk gets this right.)

The really bad bug noted in comment #5 has been dealt with on S2_5 under bug #22299; and comment #4 indicates that the AI is unlikely to be confused by this (at least with civ2civ3).

So I think we can live without fixing this on S2_5 if it's too hard/risky. I leave you to remove the release target and close the ticket if you agree.

Jacob Nevins <jtn>
Project Administrator
Sun 05 Oct 2014 08:33:28 PM UTC, comment #14:

> What's the status of this?
> Is it fixed on trunk by commits under this and related tickets,
> or is there still more to do?


It is fixed on trunk (at least I hope).

> Does it remain basically unfixed on S2_5?


Yes, it is unfixed in S2_5. That's the reason I have kept it open.

pepeto <pepeto>
Project MemberIn charge of this item.
Sun 05 Oct 2014 01:39:26 PM UTC, comment #13:

What's the status of this?
Is it fixed on trunk by commits under this and related tickets, or is there still more to do?
Does it remain basically unfixed on S2_5?

Jacob Nevins <jtn>
Project Administrator
Thu 24 Jul 2014 05:07:16 PM UTC, comment #12:

I won't do that for S2_5.

pepeto <pepeto>
Project MemberIn charge of this item.
Tue 22 Jul 2014 12:03:40 PM UTC, comment #11:

Would also need patch #3900 (and follow-up bugfixes there), and possibly more (depending on dependencies found following that stack). I think it's probably relatively safe to leave this unimplemented for S2_5. The AI may try to disembark impossibly once in a while, but except when the beachhead is invalid, this should not cause issues. Player GoTo may appear allowed, and the player get a message as if they had used a keyboard move command, but that's just a minor UI point.

Note that it may be worth short-circuiting this in the client, so that if a unit can't disembark, it cannot be given GoTo orders and the keyboard commands provide feedback directly in the client, rather than a round-trip to the server, but that's a different sort of fix than described by the patch series in question.

Emmet Hikory <persia>
Project Member
Tue 22 Jul 2014 10:41:19 AM UTC, comment #10:

It seems very too complicate to be able to handle disembark restrictions in S2_5. Maybe should we just let this as incomplete feature? Or, maybe should we apply the patch series applied to trunk (notably patch #3901, patch #4889 and patch #4910)?

What do you think?

pepeto <pepeto>
Project MemberIn charge of this item.
Sat 12 Jul 2014 02:56:11 PM UTC, comment #9:

I only fixed for trunk, it needs to be backported to S2_5.

pepeto <pepeto>
Project MemberIn charge of this item.
Wed 09 Jul 2014 01:16:53 PM UTC, comment #8:

I raised bug #22299 in order to fix bug describe at comment #5.

pepeto <pepeto>
Project MemberIn charge of this item.
Wed 09 Jul 2014 01:15:13 PM UTC, SVN revision 25469:

Respect of embarking and disembarking restrictions in pathfinding.

Reported by Jacobs Nevins (jtn@gna)

See gna bug #22187

(Browse SVN revision 25469)

pepeto <pepeto>
Project MemberIn charge of this item.
Sun 06 Jul 2014 06:34:17 PM UTC, comment #6:

Fix attached for the pathfinding part. Applies over patch #4889 and patch #4910.

(file #21305)

pepeto <pepeto>
Project MemberIn charge of this item.
Sun 06 Jul 2014 03:58:18 PM UTC, comment #5:

Worse than pathfinding violation, the server allows Riflemen to load to Helicopter into Ocean. But as it cannot be loaded, the Riflemen stands alone in Ocean.

pepeto <pepeto>
Project MemberIn charge of this item.
Sat 14 Jun 2014 08:25:23 PM UTC, comment #4:

The specific example doesn't affect AI pathfinding, but only because the AI can't deal with that class of transport at all, so won't choose it. When the AI learns how to deal with more complex transport choices, it will need to understand about embarks/debarks, but that's better handled in an overhaul of the ferry model.

Emmet Hikory <persia>
Project Member
Sat 14 Jun 2014 05:34:42 PM UTC, comment #3:

> In the meanwhile, do you think civ2civ3 should allow all units
> to disembark from helicopters everywhere?

I don't think there's any need for the ruleset to change. I think this is a minor UI issue.

Jacob Nevins <jtn>
Project Administrator
Sat 14 Jun 2014 05:20:56 PM UTC, comment #2:

In the meanwhile, do you think civ2civ3 should allow all units to disembark from helicopters everywhere?

David Fernandez <bardo>
Sat 14 Jun 2014 12:31:39 PM UTC, comment #1:

I guess pf_tools.c:normal_move_unit() and friends need to be taught about the possibility of units' transport status restricting their ability to move; currently I think they only check tile nativity, not transport status explicitly.

> client goto and direction keys do not know ...

Actually it's normal for direction keys not to check this sort of thing.

Jacob Nevins <jtn>
Project Administrator
Sat 14 Jun 2014 11:57:30 AM UTC, original submission:

In civ2civ3, with Riflemen on a Helicopter (who can't unload except in a city/airbase), client goto and direction keys do not know about the restriction, so will allow attempted moves which the server will reject with "Riflemen cannot disembark outside of a city or a native base for Helicopter."

(Maybe this affects AI pathfinding too?)

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

 

Digest:
   patch dependencies.

Digest:
   bug dependencies.

 

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

     

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

     

     

    Follow 21 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 06 Oct 2014 11:29:02 PM UTCjtnPlanned Release2.5.0, 2.6.0=>2.6.0
    Mon 06 Oct 2014 11:10:21 PM UTCpepetoStatusNeed Info=>Fixed
      Assigned toNone=>pepeto
      Open/ClosedOpen=>Closed
    Thu 24 Jul 2014 05:07:16 PM UTCpepetoAssigned topepeto=>None
    Tue 22 Jul 2014 10:41:18 AM UTCpepetoStatusIn Progress=>Need Info
    Sat 19 Jul 2014 09:19:11 AM UTCpepetoDependencies-=>bugs #22317 is dependent
    Sat 19 Jul 2014 09:18:18 AM UTCpepetoAssigned toNone=>pepeto
    Sat 12 Jul 2014 03:01:01 PM UTCpepetoDependencies-=>bugs #22050 is dependent
    Sat 12 Jul 2014 02:56:11 PM UTCpepetoStatusFixed=>In Progress
      Assigned topepeto=>None
      Open/ClosedClosed=>Open
    Wed 09 Jul 2014 01:16:53 PM UTCpepetoStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Sun 06 Jul 2014 06:34:34 PM UTCpepetoDependencies-=>Depends on patch #4910
    Sun 06 Jul 2014 06:34:17 PM UTCpepetoAttached File-=>Added pf_embarking_disembarking.patch, #21305
      CategoryNone=>general
      StatusIn Progress=>Ready For Test
    Sun 06 Jul 2014 03:58:18 PM UTCpepetoStatusNone=>In Progress
    Mon 16 Jun 2014 08:51:31 PM UTCpepetoAssigned toNone=>pepeto
    Sat 14 Jun 2014 11:58:01 AM UTCjtnRelease=>S2_5 r25141
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup