Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DockingPort:undock causes crash #1321

Closed
gisikw opened this issue Dec 20, 2015 · 11 comments · Fixed by #1348
Closed

DockingPort:undock causes crash #1321

gisikw opened this issue Dec 20, 2015 · 11 comments · Fixed by #1348
Assignees
Labels
bug Weird outcome is probably not what the mod programmer expected.
Milestone

Comments

@gisikw
Copy link
Contributor

gisikw commented Dec 20, 2015

Can be reproduced on a simple CPU+Probe+DockingPort craft on the launchpad.

SHIP:DOCKINGPORTS[0]:UNDOCK

The part does indeed undock, but the program crashes with the following error:

Object reference not set to an instance of an object
(Cannot show kOS Error Location - error might really be internal. See kOS devs.)

Encountered this behavior both on develop, and in a clean install of the 0.18.2 release.

@gisikw gisikw changed the title DockingPort:UNDOCK causes crash DockingPort:undock causes crash Dec 20, 2015
@gisikw
Copy link
Contributor Author

gisikw commented Dec 20, 2015

Same behavior is encountered via SHIP:DOCKINGPORTS[0]:GETMODULE("ModuleDockingNode"):DOACTION("decouple node", true). and SHIP:DOCKINGPORTS[0]:GETMODULE("ModuleDockingNode"):DOEVENT("decouple node").

@hvacengi
Copy link
Member

Can you post the log file that goes along with this error? Internal errors should be logged to KSP.log which will tell us where to start looking.

@hvacengi hvacengi added the bug Weird outcome is probably not what the mod programmer expected. label Dec 21, 2015
@gisikw
Copy link
Contributor Author

gisikw commented Dec 21, 2015

[LOG 13:56:22.745] kOS: (Cannot Show kOS Error Location - error might really be internal. See kOS devs.)
[LOG 13:56:22.746] System.NullReferenceException: Object reference not set to an instance of an object
  at Part.Undock (.DockedVesselInfo newVesselInfo) [0x00000] in <filename unknown>:0 
  at ModuleDockingNode.Undock () [0x00000] in <filename unknown>:0 
  at ModuleDockingNode.Undock () [0x00000] in <filename unknown>:0 
  at kOS.Suffixed.Part.DockingPortValue.<DockingInitializeSuffixes>b__2_10 () [0x00000] in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 

@hvacengi
Copy link
Member

Looks like we may have to just wrap that method. The exception is being thrown from within KSP itself at a point where we don't have any parameters to pass to KSP. I did a quick peek using ILSpy, and it looks like there's a 2nd method called UndockSameVessel() but it isn't used by anything in KSP. Here is the code for the "Undock Node" action (which should be what happens when you trigger it by action group):

[KSPAction("Undock Node")]
public void UndockAction(KSPActionParam param)
{
    if (base.Events[��.�(166435)].active)
    {
        while (true)
        {
            switch (5)
            {
            case 0:
                continue;
            }
            break;
        }
        if (!true)
        {
            RuntimeMethodHandle arg_34_0 = methodof(ModuleDockingNode.UndockAction(KSPActionParam)).MethodHandle;
        }
        this.Undock();
    }
}

There's some obfuscation going on there, but essentially it looks like "If the undock event is active, execute this.Undock();". Our suffix is currently set up like this:

            AddSuffix("UNDOCK", new NoArgsSuffix(() => module.Undock()));

The exeception is coming from within the Part's definition though, not ModuleDockingNode, and that method is much longer with a number of obfuscated members. Unless we can track down the source of the error, adding a a try ... catch ... block to catch this issue might be our only option (though that seems very "hacky").

@hvacengi
Copy link
Member

hvacengi commented Jan 4, 2016

I just had opportunity to test this on the current development build, and I cannot seem to get it replicated. @gisikw could you please test again using the most recent develop branch?

For background, did it seem to be happening every time when you tested it before, or was it intermittent?

@gisikw
Copy link
Contributor Author

gisikw commented Jan 4, 2016

Unfortunately, I'm still encountering the same issue on the latest develop. Test Craft

ship:dockingports[0]:undock.
[LOG 13:18:53.583] kOS: At interpreter history, line 3
ship:Dockingports[0]:undock.
                     ^

[LOG 13:18:53.584] System.NullReferenceException: Object reference not set to an instance of an object
  at Part.Undock (.DockedVesselInfo newVesselInfo) [0x00000] in <filename unknown>:0 
  at ModuleDockingNode.Undock () [0x00000] in <filename unknown>:0 
  at ModuleDockingNode.Undock () [0x00000] in <filename unknown>:0 
  at kOS.Suffixed.Part.DockingPortValue.<DockingInitializeSuffixes>b__2_10 () [0x00000] in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[],System.Exception&)
  at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0 
[LOG 13:18:53.589] Code Fragment
File                 Line:Col IP   label opcode operand
----                 ----:--- ---- -------------------------------  
                        0:0   0000  jump +1 
interpreter history     1:0   0001 @0001 nop 
interpreter history     2:1   0002 @0002 push $undock 
interpreter history     2:1   0003 @0003 pop 
interpreter history     3:1   0004 @0004 push $ship 
interpreter history     3:6   0005 @0005 push dockingports 
interpreter history     3:6   0006 @0006 getmember 
interpreter history     3:19  0007 @0007 push 0 
interpreter history     3:19  0008 @0008 getindex 
interpreter history     3:22  0009 @0009 push undock 
interpreter history     3:22  0010 @0010 getmember <<--INSTRUCTION POINTER--
interpreter history     3:22  0011 @0011 pop 
                        0:0   0012  EOF 

[LOG 13:18:53.593] kOS: Stack dump: stackPointer = -1

@gisikw
Copy link
Contributor Author

gisikw commented Jan 4, 2016

Oh, and to your latter question, no it was/is happening consistently.

@hvacengi
Copy link
Member

hvacengi commented Jan 4, 2016

Hm, it's an important piece of information that the error is occurring with parts that were never "docked" in flight, but rather were put together in the editor. [That is to say, this is an "unusual" case in that it isn't the primary function of "docking", not saying that it's a particularly big deal that the detail was omitted from the initial report just identifying it]

I've got it throwing the exception now, however I can't make it break using doevent("decouple node") or doaction("decouple node", true). It doesn't seem to matter which node I use (index 0 or 1) I can still "decouple" without exception, which you had reported issues with before. Can you try using "decouple node" again and attach that stack trace? Decoupling uses a different method in ModuleDockingNode and the underlying Part. If I can identify the differences between the two, there's a chance we could simply call a different method to avoid the exception.

@gisikw
Copy link
Contributor Author

gisikw commented Jan 4, 2016

Ah, hmm, yeah, I originally encountered the issue when decoupling a ship in orbit, in order to fit stuff within fairings. The craft I linked was created afterward just for testing.

doevent("decouple node") no longer throws an error for me with the test craft! (It was throwing the same error when this bug was filed)

Testing against ports that I know were docked in orbit, undock. works without error.

So yeah, it looks like at present, undock is broken for decoupling VAB/SPH-coupled ports, but doevent("decouple node") works, so if undock can alias, that should fix the issue.

@hvacengi
Copy link
Member

hvacengi commented Jan 4, 2016

The really strange thing is that we haven't touched the docking port code in the last 2 months, so we didn't "accidentally" fix it.

I'm trying to figure out a way to detect when to use "decouple" vs "undock", I may just end up having to poll the event visibility (since "undock" is hidden in the gui at that point).

@gisikw
Copy link
Contributor Author

gisikw commented Jan 4, 2016

Yeah, it's weird. I had used undock for a video, with VAB-docked ports, and it ran fine, on 0.18.1 (I think). Then on 0.18.2 and develop, it broke, and I couldn't see any tweaks that would explain the change in behavior :-/

@hvacengi hvacengi self-assigned this Jan 4, 2016
@hvacengi hvacengi modified the milestone: v0.19.0 Feb 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Weird outcome is probably not what the mod programmer expected.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants