MechJeb should not control ship when no RemoteTech/CommNet signal or RP-0 avionics (toggleable)

[MOARdV]

The current development of RemoteTech allows "Sanctioned Pilots" to be added to the RT control queue. The implementation of this ability requires the caller to register an OnFlyByWire method with RT. Essentially, to disable flight controls, RT has a post-OnFlyByWire method that neutralizes the FlightCtrlState changes when the vessel in question is uncontrolled. After nulling out the state, RT calls sanctioned autopilots.

It's trivial to write a bridge DLL to add MechJeb to the sanctioned pilots list, but it requires MechJebCore.OnFlyByWire to be made public (it's private right now). It does mean that MJ's OnFlyByWire would be called twice per update, although the first instance's effects will be cancelled out - so, a better solution would also be to allow an external caller to ask MechJebCore not to inject its OnFlyByWire method into the vessel.OnFlyByWire stream. I don't know how feasible the latter actually is, or if the MJ team is willing to accept that sort of change. I can implement the private -> public change easily, and I can research the latter if it's acceptable, but I wanted to get a discussion on them before I generate pull requests.

[erendrake]

One thing to consider is that According to the way remotetech normally works MechJeb should not always be available as a sanctioned pilot. A few examples,

• user has no RT connection and starts clicking SmartASS buttons : No Sanctioned Pilot
• user has a RT connection and starts clicking SmartASS buttons : Sanctioned Pilot
• user has initiated a transfer burn when they had a connection and then lost the connection : Sanctioned Pilot

If the user always has instant access to the controls, why are they playing with remotetech at all?

[MOARdV]

These are issues I intended to handle in a bridge DLL. If the user clicks on SmartASS buttons (to take your example), the MJ/RT bridge asks RT if a connection exists. If so, it adds MJ to the sanctioned list. If not, it doesn't. With a little bit of additional effort in the bridge, the RT signal delay could be accounted for, as well. As for starting an action while RT is connected, but losing it mid-action, I'd say that the MJ remains sanctioned until the action has completed - MJ is acting as a flight computer, after all. Yes, one can come up with plenty of examples where a player can abuse the design, but hashing that out would make sense in a development thread for that bridge DLL, not in a MechJeb issue. I'm not asking the MJ team to implement the bridge functionality, all I'm asking for is a couple of changes that would facilitate someone else doing it without adding it and a dependency to RemoteTech in MJ.

As an alternative, after digging through the MJ code some more, it may be able to work around the issue without a code change. If that's the case, I'll come back and close this request.

[sarbian]

@erendrake don't presume you know how everone plays. I use RT but I disable the timelag. Building the sat network is what I like.

@MOARdV Calling OnFlyByWire twice will mess up the PID controllers in some of the modules. And switching OnFlyByWire to a public may makes other mods call it and they'll mess up the PID controler too. This will need more thinking

[erendrake]

@sarbian im not primarily talking about the time lag, and integration should of course respect whatever setting the user has set in the config for RemoteTech. I presumed nothing about the user's playstyle.

@MOARdV cool, i just wanted to bring those issues up because they are issues we have had to deal with in kOS :) Good Luck!

[Starstrider42]

@MOARdV, thanks for volunteering to handle MechJeb/RemoteTech integration. It's a shame GitHub doesn't have rep, but you definitely deserve some.

[leftler]

I just wanted to come in and also voice my interest in this. I did a fork earlier today and began looking in to doing pretty much the exact same project. I ran in to similar problems as MOARdV.

The approach I was looking in to is seeing if there was anything in the various ComputerModule classes that I could intercept and could in turn generate a RT2 ICommand which would, once the delay passes, call back to invoke whatever function would of been called.

I see two ways of implementing this, the first way is if things like MechJebModuleSmartASS.Engage() and MechJebModuleLandingAutopilot.LandAtPositionTarget(object controller) where made virtual it would not be too hard to implement a set of proxy modules that do the interception functionality. This approach has less of a opportunity to mess up other components than overriding the OnFlyByWire, but it does cause more work for the MechJeb plugin creator as they will need to create a proxy class for every module they want to delay and any new modules or 3rd party plugins would need custom proxy classes also.

The other solution, which would be more work for the MechJeb team, would be to create a new virtual function on ComputerModule with a signature similar to void PreviewAction<T>(Action<T> actionToPerform, T arguments) that would be called inline for any user "action". This allows for a more generic approach on the plugin creator side and would allow the plugin to act on modules without needing to know exactly what the module does. In Remote Tech's case it could defer the delegate call in the event of a delay or just never call the action if the connection is not available.

[TheStonedRaider]

MJ and RT2 seem to get on fine far as I can tell.

[Falkvinge]

Allow me to also add my interest in this. Since MechJeb is running locally on the vessel, there should reasonably be no RemoteTech (RT) signal delay from MechJeb to the vessel controls, BUT there should be an RT signal delay from the control center to the vessel in order to execute MechJeb commands or set MechJeb states (such as "land somewhere", "vector-surface-up", "begin docking", et cetera). Is there any hope of this happening?

Today, there is no RT signal delay when setting new MechJeb states, and no RT signal delay for MechJeb execution, which effectively eliminates the difficulty of a scenario like landing remotely with signal delay. One of those signal paths needs a delay, and the logical place to have the delay would be on the set-state command as the execution of MechJeb code happens at the remote location on the vessel itself.

(oh, and there's also the control conflict between RT's Flight Computer and MechJeb...)

[sarbian]

With the current way MJ is coded that would be quite complex. I might have a look at doing it when redoing the whole UI code for KSP 1.1. I guess some kind of message bus would do the trick and if done right I could also interface it with the 1.1 "light RT"

But not before the 1.1 UI integration since it will require a large code rewrite anyway.

[Falkvinge]

It may be worth asking the RemoteTech authors for such a message bus - sort of a "return this cookie to me after the signal delay, if and only if a connection exists"? That would enable solving the problem in the general case. Also, and more importantly, you could do it statelessly (or perhaps just indicating state in the UI).

[TheBlackTower]

here's an idea... since mechjeb interface comes from an object when attached to the ship, you could block its interface by blocking the original mechjeb part and replacing it with some other part, since its your part then you could put your interface and all you would have to do its unban mechjeb but imput a delay in commands and then send the cookie to mech

[leftler]

@MOARdV one thing you could try instead of working on the expectation of OnFlyByWire being made public you could override Drive on the relevant controller classes. I have made a similar workaround that overrides Drive on MechJebModuleThrustController and MechJebModuleAttitudeController over at https://github.com/leftler/RemoteTechMechJebBridge

    public RemoteTechMechJebBridgeModuleMechJebModuleThrustController(MechJebCore core) : base(core)
    {
        priority = 800;
        driveDelegate = DriveFromRemoteTech;

        //We must remove the stock MechJebModuleAttitudeController if it already has been added and add it to the blacklist.
        var originalModules = core.GetComputerModules<MechJebModuleThrustController>();
        foreach (var originalModule in originalModules)
        {
            if (originalModule.GetType() == typeof(MechJebModuleThrustController))
            {
                core.RemoveComputerModule(originalModule);
            }
        }
        if (!core.blacklist.Contains(typeof (MechJebModuleThrustController).Name))
        {
            core.blacklist += typeof (MechJebModuleThrustController).Name;
        }
    }

    public override void OnLoad(ConfigNode local, ConfigNode type, ConfigNode global)
    {
        base.OnLoad(local, type, global);
        AddPilot();
    }

    private void AddPilot()
    {
        if (vessel != null)
        {
            //This method is safe to call multiple times, it the same delegate is added it does not add it a 2nd time.
            RemoteTech.API.API.AddSanctionedPilot(vessel.id, driveDelegate);
        }
    }

    public override void Drive(FlightCtrlState s)
    {
        //Do noting
    }

    private void DriveFromRemoteTech(FlightCtrlState s)
    {
        if(!enabled || core != vessel.GetMasterMechJeb() || core.DeactivateControl)
            return;

        base.Drive(s);

        core.CheckFlightCtrlState(s);
    }

Now, this is just my first pokes at this, and I doubt I will ever release a finished version of this but if you want to take the idea and run with it I would be more than happy to let you.

[sarbian]

You sure love complex solution. Sorry but anything that plays with the modules list like that is a no go for me.

[leftler]

Like I said, it is a hack and I have no intention of turning it in to a formal mod. But I did get some ideas last night while I was trying to sleep that may provide a better solution.

Out of curiosity, would you be against making MechJebCore.OnFlyByWire or MechJebCore.Drive be protected virtual? If you do this we could make a derived RTMechJebCore class and use MM to replace it on all parts.

If not that, then what about being open to moving the vessel.FlyByWire subscription and unsubscription logic out in to its own module or out in to virtual protected methods that could be overriden?

[leftler]

Slightly off topic, but related to what I was looking in to, is there a specific reason GetComputerModule<T>() uses the unordered list and not a ordered one? If it used the ordered one it would make the calls deterministic (as long as the priority is different) in the event two of the same module are registered.

If it used the ordered list I could get rid of everything in the constructor of my hack dealing with the module manipulation, the only thing I would need to do is set priority = 799; and it would guarantee that attitude = GetComputerModule<MechJebModuleAttitudeController>(); would get my controller instead of the stock controller.

[sarbian]

Because having 2 of the same module is not something I want to see or deal with...

[MOARdV]

I forgot I opened this issue. Anyway, I am not working on this. If someone else wants to pick it up, cool. Otherwise, I can close it.

[lamont-granquist]

Constraining this issue to just disabling MJ when out of range and/or the ship has no avionics in RP-0. Those should be independently toggleable (I would only care about making certain I have enough avionics to control the craft). Signal delay would be vastly harder and I think I'll just assert that will never happen and is out of scope (speaking personally that would be months of work and as I never play with signal delay and find it pointlessly frustrating, I have zero interest in spending any time working on it -- sounds like sarbian is in roughly the same camp...).

[lamont-granquist]

This is actually done on the MechJeb side.

See https://github.com/KSP-RO/RP-0/issues/497 for the RP-0 bug to implement it on that side.

Other consumers should look in that issue for a pointer to the API to use.