openscope / openscope

openScope Air Traffic Control Simulator
http://www.openscope.io
Other
664 stars 181 forks source link

Direct removes procedural hold #1276

Open Marcel510 opened 5 years ago

Marcel510 commented 5 years ago

Issue Description

Some STARs have holds built hin for example: EGKK TIMBA4B

    "TIMBA4B": {     
        "icao": "TIMBA4B",  
        "name": "Timba Four Bravo",  
        "entryPoints": {  
            "KUNAV": []  
        },  
        "body": ["KUNAV", ["AMDUT", "A160"], ["KKE64", "S250-"], ["@TIMBA", "A70|S250-"]],  
        "rwy": {  
            "EGKK08R": [],  
            "EGKK26L": []  
        },  
        "draw": [  
            ["KUNAV", "AMDUT"]  
        ]  
    },  

which works perfectly: screenshot_197 but as soon as you give the aircraft a direct to the holding fix the holding is omitted: screenshot_198

Steps to reproduce

cake-pie commented 5 years ago

Analysis:

~Currently~ In 6.10, the exithold method is basically "if currently in a holding pattern, exit it":

https://github.com/openscope/openscope/blob/847bb91801b7865be3769d68ce120a48b2c1a81c/src/assets/scripts/client/aircraft/Pilot/Pilot.js#L532-L542

However, absent an explicit command to cancel holding, exithold is instead implied by various other commands. The intended behavior is:

Here is proceedDirect:

https://github.com/openscope/openscope/blob/847bb91801b7865be3769d68ce120a48b2c1a81c/src/assets/scripts/client/aircraft/Pilot/Pilot.js#L765-L775

L770 and L771 are in wrong order, so dct TIMBA results in:

~Before I PR this -- should I make it a bugfix or hotfix?~

From 6.11 onwards changes in cancel hold code behavior slightly complicates things (see #1333)

sam-irl commented 5 years ago

bugfix imo. obviously defer to someone more experienced but this has been a known issue for a while, it's important but not absolutely critical

cake-pie commented 5 years ago

Well, it's either a hotfix for 6.10.x, or subsumed by #1333 in 6.11 due to changes by #1160.

felixscheffer commented 5 years ago

bugfix for 6.11 (which we ideally release in 6.12)

erikquinn commented 5 years ago

When an aircraft has holding instructions (though in this case, its implied by the STAR rather than being explicitly issued), and they are cleared direct to the holding fix, we would have to reiterate the hold instructions. So the behavior is a pretty standard one. But I can see your point. More thought is needed to see what we should do here... 🤔