BUG: strange AddOn behavior

[ETFVoyageur]

I have a simple AddOn that just monitors window creation, destruction, and state changes. It provides a little GUI / window handling functionality that I find convenient. It is coded as a singleton, but there is no way to tell NT that. I had to make the constructor public because otherwise NT won't play nice -- it logs errors and does not call the virtual functions I override.

The problem -- IMHO an NT bug -- is that NT calls the AddOn's constructor three times. I haven't the faintest idea why it needs more than one copy, but NT thinks it does.

• It uses instance #1 to get everything initialized. One of the things that happens there is that event callbacks get registered within my code.
• I have no idea what instance #2 ever gets used for.
• Then it delivers the virtual function calls for window creation to instance #3. The problem is that instance #1 has the callback registrations. As far as instance #3 knows, nothing was ever registered.

IMHO NT has absolutely no business calling into a freshly-constructed instance #3. I might be able to see it if instance #3 had come from cloning, but that's not what NT does. IMHO this is a big bug.

In my case there is a pretty straightforward workaround -- I made all interesting values static. That is obviously not satisfactory in general, though. If NT really does need multiple objects it had better clone them at an appropriate time, such as just before using them. Simply substituting a constructed object without the operational history is a non-starter. Please fix.

--EV

[NinjaTrader_ZacharyG]

Hello ETFVoyageur,

Thank you for writing in.

I have created a simple AddOn attached to this post that merely prints to the Log when its constructor is called. From my testing, I see that the print only occurs once.

Can you please provide a simplified script demonstrating that the constructor is called multiple times in an AddOn?

[ETFVoyageur]

There must be more to this.

• My original AddOn constructor gets called three times --- I just re-verified this.
• I copied it to a new file and renamed the class. The constructor for the new one gets called twice. That is sufficient to be a bug, so I did not try to see why it is not called three times.
• The code to produce the double call is unbelievably simple, so there must be something about my setup, workspace, etc

Here's the big bad test case showing the constructor getting called twice.

using NinjaTrader.Cbi;

//This namespace holds Add ons in this folder and is required. Do not change it. 
namespace NinjaTrader.NinjaScript.AddOns
{
    public sealed class ConstructorBug : NinjaTrader.NinjaScript.AddOnBase
    {
        public ConstructorBug()
        {
            Log("Constructor called for ConstructorBug", LogLevel.Warning);
        }
    }
}

These multiple calls happen with or without Visual Studio. I can provide full call stacks if that would help.

[ETFVoyageur]

A slightly improved test case -- it occurred to me that I need to also establish that Clone() is not being called. It's not -- just the constructor getting called twice.

--EV

using NinjaTrader.Cbi;

//This namespace holds Add ons in this folder and is required. Do not change it. 
namespace NinjaTrader.NinjaScript.AddOns
{
    public sealed class ConstructorBug : NinjaTrader.NinjaScript.AddOnBase
    {
        public ConstructorBug()
        {
            Log("Constructor() called for ConstructorBug", LogLevel.Warning);
        }

        public override object Clone()
        {
            Log("Clone() called for ConstructorBug", LogLevel.Warning);
            return base.Clone();
        }
    }
}

[NinjaTrader_ZacharyG]

Hello ETFVoyageur,

With the sample you have provided, I am only seeing the constructor being called once.

I would suggest excluding all non-default NinjaScript files from your compilation and see if you experience the same. To do this, open the NinjaScript Editor, right-click on a particular NinjaScript file and select Exclude from Compilation. Once you have excluded all other files, recompile and restart the platform.

Do you still experience multiple constructor calls for the ConstructorBug script?

[ETFVoyageur]

Two constructor calls still get logged.

I turned off everything of mine other than ConstructorBug, saved the workspace, exited and restarted. After the session break the log includes

Vendor assembly 'NinjaTraderAddOnProject' version='1.0.0.0' loaded.
Constructor() called for ConstructorBug
Vendor assembly 'xNinjaTrader.Custom' version='8.0.0.12' loaded.
Constructor() called for ConstructorBug
Global simulation mode enabled

Re-enabling everything, putting a Log() call in the constructor of the real class (VsaWindow), and restarting, the log now includes:

Constructor() called for VsaWindow
Constructor() called for ConstructorBug
Vendor assembly 'xNinjaTrader.Custom' version='8.0.0.12' loaded.
Vendor assembly 'NinjaTraderAddOnProject' version='1.0.0.0' loaded.
Constructor() called for VsaWindow
Constructor() called for VsaWindow
Constructor() called for ConstructorBug
Global simulation mode enabled

Note the extra call to the VsaWindow constructor (i.e. 3 calls to the VsaWindow constructor, but only 2 calls to the ConstructorBug constructor)

--EV

[NinjaTrader_ZacharyG]

Hello ETFVoyageur,

At this point, I would like to suggest a clean NinjaTrader 8 user folder and only a new sample test AddOn script to see if this is still occurring.

Please follow the steps below to create a clean NinjaTrader 8 user folder. You will not lose any items by following these steps:

• Navigate to your Documents folder and rename NinjaTrader 8 to NinjaTrader 8 OLD
• Redownload the NinjaTrader 8 installer: http://ninjatrader.com/PlatformDirect
• Run the installer

Once NinjaTrader 8 has been reinstalled, start the platform and create a new AddOn test script and test again.

[ETFVoyageur]

I did as you said and found some interesting things. I believe I have found the bug, too.

GOOD: Doing this did eliminate one constructor call. ConstructorBug() now gets called only once, and VsaWindow now gets called only twice. That's progress. I guess my NT8 installation must have been somehow corrupted?

BAD: it appears that the NT algorithm is not compatible with static objects. VsaWindow is a static singleton. That means it gets constructed the first time there is a reference to that static object. Here's what happens:

1) NT constructs an instance for its own purposes.
2) C# constructs another instance when VsaMenu references the static VsaWindow (to register event callbacks). All of the AddOns' code (VsaMenu and VsaWindow) uses this instance.
3) NT makes virtual function calls via the bogus instance it created --- a bug, because no AddOn code is using that instance.

===

What we have here is a design issue. Before I pontificate, let me ask:

1) Are AddOns required to be functionally singletons? If not, how does NT know enough to call the virtual functions that handle events in all instances? If AddOns are required to be functionally singletons, is that a deliberate design restriction?

2) What ordering guarantees are there? If, as in my case, one AddOn calls another AddOn, how can the caller know the callee has been constructed? C# handles that by lazy construction of statics. How should my code work in the NT environment so the caller is guaranteed to be calling an already-constructed object?

Unless the above questions have good answers, the NT design needs to be fixed. If there are good answers, I'll fix my code to work with them.

My conjecture is that all AddOns will have State.SetDefaults called before anything else happens. Then AddOns can assume other AddOns are functional beginning with State.Configure. Is that right? If so, I still have questions:

• What happens if an AddOn wants to have multiple instances running?
• How can I have a read-only static that references the instance? I understand the static part -- just assigning "this" to it. The problem is the readonly part -- assignment to it can be done only in a static constructor.

--EV

[ETFVoyageur]

Bottom line:

• There really was one problem with an extra constructor call. That was fixed by reinstalling NinjaTrader 8 -- some sort of corruption, evidently.
• The other extra constructor call ( to VsaWndow() ) was a result of not understanding how NT actually handles AddOns. I have detailed my current understanding below so any remaining misconceptions can be corrected. I also make the suggestion that the AddOns introductory information should explain this.

After some thought and trial, I do believe that AddOns operate as my earlier posting suggested. I have concluded that the things I was concerned about can all be handled. Here is my current understanding. Please look it over and let me know if I have anything wrong, or am missing anything.

Suggestion -- this needs to be covered in the AddOns introductory information. Please pass that along to your tech writers.

Design suggestion -- things would be more flexible if AddOnBase registered itself when constructed, rather than NT constructing and tracking those it constructs -- that would allow for such things as singletons and multiples that the current system does not support.

Thanks,
EV

=====

// NT will create exactly one instance of each class that is derived from NinjaTrader.NinjaScript.AddOnBase
//        * The class must have a public default constructor
//        * There is no way for NT to create a static instance
//        * NT is aware of only the instances it creates
//            * NT knows nothing of any instances that get created any other way
//            * Classes created otherwise do not somehow register themselves
//        * All such creation, including calling OnStateChange(State.SetDefaults), happens before any other calls are made
//            * State.SetDefaults is effectively part of the constructor.
//                * The base class calls State.SetDefaults just before returning to the derived class constructor
//            * In State.SetDefaults, objects cannot assume any other objects have yet been created
//            * In State.Configure, objects can safely assume all other objects have been constructed and can safely be called
//        * All AddOn objects are created before any windows are created
//            * Is there a race -- can other AddOns create windows at this time?
//        * NT makes its virtual function calls on the instances it creates
//            * These calls are not made on any other instances (because NT is not aware of them)
//
// That means NT does not support such things as:
//        * Static instances of AddOns
//        * Multiple instances of AddOns
//
// If the code needs that sort of thing:
//        * Create an interface / multiplexor AddOnBase class that NT can instantiate
//        * That interface object can create whatever other objects it likes, but not as subclasses of AddOnBase
//        * The interface object must relay calls between its children and NT as appropriate

[NinjaTrader_Brett]

Thanks for the feedback and post. You've got it figured out. Add On is new for us and always looking for a better way to attack the documentation to be clear. Your advice below is appreciated. Your suggestion on AddOn base is received.

Thanks again ETF.

-Brett

[ETFVoyageur]

I got to thinking about how to control creation of AddOns.. The issue is that something has to trigger creating an AddOn, and today's method does so. I just want to expand that. Consider the following suggestion:

• An AddOn class can have an attribute [AutoCreate(bool)]
• A missing attribute is treated as being true
• If the attribute is true, the class is created just as today. These three points together mean that no currently working code gets broken
• If the attribute is false then then there is no automatic creation for that class -- creation will be handled by the code itself. NT does not care when, how, or even whether an instance gets created. NT would not care whether or not there is a public default constructor for such a class, since it is not going to call the constructor.
• The base class constructor of an instance with the attribute value false registers its instance so it gets services, such as state change and the virtual function calls, exactly like the instances that NT automatically created.

This addition would allow flexibility that the current system lacks. My suggestions would allow true singleton instances, as well as AddOns where multiple instances are needed. It is true you can sort of do those things today, but there are issues. My proposal would be simpler, cleaner, and perform at least a little better.

• The only current way to get a true singleton (static, readonly) is to have an AddOn interface class that relays all calls from NT to the actual singleton. That is at least a little performance hit, and it is unnecessarily complicated.
• The only current way to get multiple instances is to have an AddOn interface class that spawns children that are not subclasses of AddOnBase and then relays all NT calls to them. That is at least a little performance hit, and it is unnecessarily complicated.

--EV

[NinjaTrader_Brett]

Yea not a bad idea, but for now we will move forward with our current design and see where it takes us. We could look at making changes as needed in a next major revision.

[ETFVoyageur]

OK. Note that my suggestion does not require any change to the current design, nor does it break any existing working code. It simply makes a small extension that provides added flexibility.

One thing that got me thinking about this is that AddOns are functionally singletons, in that there is only one of each class. It seems ironic that if you actually program one as a singleton, though, it will not work with the current scheme. That would work with my suggestion.

--EV