User Input Validation on GetOption

The GetResult enum from GetOption.Get() is always ignored in the github c# examples. First they check GetOption.CommandResult() for success before checking the GetResult enum from GetOption.Result().

I’m wondering why GetOption.Get() doesn’t return CommandResult if it has to be checked first? If I hadn’t studied the sample projects, I would have:

if (go.Get() == Rhino.Input.GetResult.Option)
    doStuff(go.Option);
return go.CommandResult();

@nathanletwory What do you think?

F# translation of this:

    [<System.Runtime.InteropServices.Guid(IDs.SampleFsEtoPanelCommand)>]
        type SampleFsEtoPanelCommand ()  = 
            inherit Rhino.Commands.Command()
    
    do 
        let assem = typeof<SampleFsEto>.Assembly
        let icon = new System.Drawing.Icon(assem.GetManifestResourceStream("SampleFsEto.embeddedresources.plugin-utility.ico"))
        Panels.RegisterPanel( SampleFsEto.Instance, typeof<SampleFsEtoPanel>, "Sample",  icon)

    static member val Instance = new SampleFsEtoPanelCommand() 
                    
    override this.EnglishName = "SampleFsEtoPanel" //The command name as it appears on the Rhino command line.
           
    override this.RunCommand (doc, mode)  =  
    
        let visible = Panels.IsPanelVisible(SampleFsEtoPanel.PanelID)
        let prompt = if (visible) then "Panel visible; new value?" else "Panel hidden; new value?"

        let go = new GetOption()
        go.AcceptNothing true
        go.SetCommandPrompt prompt
        let idxHide = go.AddOption("Hide")
        let idxShow = go.AddOption("Show")
        let idxToggle = go.AddOption("Toggle")

        let doOption (index: int) (result: Result): Result = 
            match visible with
            | true when index = idxHide || index = idxToggle  -> Panels.ClosePanel(LayersPanel.PanelID); Result.Success 
            | false when index = idxShow || index = idxToggle -> Panels.OpenPanel(LayersPanel.PanelID); Result.Success
            | _ -> Result.Failure
       
        match go.Get() with 
        | GetResult.Nothing -> doOption idxToggle (go.CommandResult()) //no selection, default to toggle
        | GetResult.Option -> doOption (go.OptionIndex()) (go.CommandResult()) //selected option
        | _ -> go.CommandResult()

@EricM, there is a difference between the CommandResult and the GetResult enum usage intentions:

https://developer.rhino3d.com/api/RhinoCommon/html/T_Rhino_Commands_Result.htm

https://developer.rhino3d.com/api/RhinoCommon/html/T_Rhino_Input_GetResult.htm

Rhino.Commands.Result is to decide what the outcome of the entire command is. Rhino.Input.GetResult is to help determine what input you got from a user. An (incorrect) input from the user doesn’t have to automatically result in command failure…

I think the F# translation looks good. The third blanket match on visible is not necessary though, since you have covered all (both) cases already. And leaving _ out will point out if there ever become more truth values available :slight_smile:

That is my understanding which leads to my confusion. Why do the sample files ignore Input.GetResult until it checks Command.Result? What case is not covered by GetResult? I can only see Esc/Cancel, Enter/Default, or some type of selection, all of which I’m handling without filtering on Command.Result. I’m just using it to return the result.

I admit that a third option on a binary decision is weird, but it’s actually (bool * int). In this case, if I get an unexpected OptionIndex, I should report a failure. I tried for a while to make it clear, but finally realized it’s impossible to “or” with specific tuples and switched to a “when”.

Every day there’s a new hard lesson to learn as I get very little done. F# has a lot of positives, but the peripheral is brutal. Like having to use ILDasm to figure out how the icon embedded resource is placed in the manifest (F# doesn’t have c#'s autogenerated StronglyTypedResourceBuilder) or having to read F#'s compiler spec to figure out how to get the inherited, default, and optional constructors to work with the Eto.Forms.Panel.

I’m not fluent at F#, but the static constructor looks suspicious. You shouldn’t typically directly be creating instances of command derived classes.

That was part of the template @Goswin made. Looking closer at the c#, the static Instance ref is always updated to the last instance that was created by Rhino. This should function identically:

[<AllowNullLiteral>]
[<System.Runtime.InteropServices.Guid(IDs.SampleFsEtoPanelCommand)>]
type SampleFsEtoPanelCommand  () as this = // calls for the command that starts the script editor
    inherit Rhino.Commands.Command()
    static let mutable instance: SampleFsEtoPanelCommand  = null

    do 
        let assem = typeof<SampleFsEto>.Assembly
        let icon = new System.Drawing.Icon(assem.GetManifestResourceStream("SampleFsEto.embeddedresources.plugin-utility.ico"))
        Panels.RegisterPanel( SampleFsEto.Instance, typeof<SampleFsEtoPanel>, "Sample",  icon)

        instance <- this

    static member Instance = instance

I believe the C# and example codes are incorrect, as that is not how the Singleton pattern works.

Also, in Rhino the assumption is that there is always ever one instance, and overwriting is wrong. That said, probably better is to create the instance on first Instance access and not overwrite when new access is done.

So add a null check on instance to prevent overwrites (if (instance = null) then instance = this)?

Back to my original question, I’m good skipping checks on CommandResult before dealing with Rhino.Input classes? Only use CommandResult to tell Rhino the result of Command.RunCommand?

Eric does this work for you :

        use go = new Input.Custom.GetObject()            
        go.AcceptNothing(true)            
        if go.Get() <> Input.GetResult.Object then None
        else
            let objref = go.Object(0)
            let obj = objref.Object()
            Some obj.Id

It’s what works for Rhino. I would never have thought to check CommandResult before checking user input, but all of the sample files stress that.