Update the DOCS with about DISPOSE() mehtod

[ttodua]

I suggest that NT dev team revised it's definition about usage of Dispose() method.

In the past we used to dispose objects (like DxBrush and etc..) in `OnStateChange->Terminated` event.


However, a while ago, NT suggested to migrate that code in `OnRenderTargetChanged`:
https://ninjatrader.com/support/help...getchanged.htm

which made the things more complicated (to destroy and recreate objects in each ORTC event.
It has been quite unflexible, because generally, a typical user, when using many Disposable resources, we create them in "DataLoaded" (or Configured) state:

..DXBrush myDX;

protected override void OnStateChange()
{
    if (State == State.Configure)
    {
       myDX = myBrush.ToDxBrush(RenderTarget);
    }
}

and easily disposed in Terminated state.

But, changing that to ORTC , made coding a bit more complicated.
However, a while ago, i've found out that NinjaTrader has some hidden state, which does an excellent job.

it's State.Finalized.

So, I suggest that update that in the docs, so instead of doing this:

protected override void OnRenderTargetChanged()
{
  if (myDX != null)
  {
    myDX.Dispose();
  }

  if (RenderTarget != null)
    myDX = myBrush.ToDxBrush(RenderTarget);
}

we can do this only:

protected override void OnRenderTargetChanged()
{
    if (State == State.Finalized)
    {
         myDX.Dispose();
    }
}

what about that? so, no need to re-create resources in TWICE place in code.

also, please read my other posts below, as they further reveal another problem.

[ttodua]

NinjaTrader_Jesse what your recommendation about this? isnt this better?

[NinjaTrader_Jesse]

Hello TazoTodua,

Thank you for the post.

I would still stick with what we suggest in the help guide as this is what is documented and known to work. As this state is not documented I couldn't really say with certainty this would work in all use cases. If you have found it does work in all the cases you have used it, you can certainly use this.

I will put in a feature request to have this state reviewed for documentation.



I look forward to being of further assistance.

[ttodua]

1) As far as i tested a simple behavior:

        protected override void OnStateChange()
        {
            Print("OST:"+State);
        }



        public override void OnRenderTargetChanged()
        {
            Print("ORTC:"+State);
        }

Here provided output: from the time of opening the indicators list -> adding this indicator on chart->and closing the chart (or instead of closing, just removing the indicator, same)

OST:SetDefaults
OST:SetDefaults
OST:SetDefaults
OST:Terminated
OST:Terminated
OST:SetDefaults
OST:Configure
ORTC:Configure
OST: DataLoaded
OST:Historical
OST:Transition
OST:Realtime
OST:Terminated
OST:Terminated
ORTC:Finalized

2) Also i might suggest that it might be worth to mention on that page, that the disposable elements should not be created in SetDefaults.
3) as a reference, i am putting here a link to another similar topic about State.Finalized .

[NinjaTrader_Jim]

Hello TazoTodua,

For our purposes below, 'DX resources' refers to any disposable resource which relies on RenderTarget and persists between methods.

We advise creating and disposing your DX resources in OnRenderTargetChanged because OnRenderTargetChanged will be called every time the RenderTarget is recreated.

If you don't create/recreate your DX resources there you introduce potential bugs in your code, such as your DX resources trying to render with the wrong RenderTarget.

You do not need to create these resources in two places, you only create them in OnRenderTargetChanged.

To your other post (https://ninjatrader.com/support/foru...tate-finalized)

Your example might not unhook the event until after the hosting indicator has already reached State.Terminated and therefore would be prone to leaking memory. If you need to perform cleanup in your plugins you could make a Cleanup method which you called from your indicator's OnStateChange method when State == State.Terminated.

Thanks

[ttodua]

Jim, many thanks for the answer. I doubt i cant fully follow you. Can I reword your answer into the question better?

1) first question: the docs (as i mentioned already: https://ninjatrader.com/support/help...getchanged.htm ) suggest to use this code:

protected override void OnStateChange()
{
  if (State == State.SetDefaults)
  {
    Name = "OnRenderTargetChanged Example";
    IsOverlay = false;
    UserBrush = Brushes.Red; // user selection pushed to the UI
  }

  else if (State == State.Terminated)
  {
    // dispose of SharpDX brush when finished
    if (dxBrush != null)
    {
        dxBrush.Dispose();
        dxBrush = null;
    }
  }
}
public override void OnRenderTargetChanged()
{
  if (dxBrush != null)
  {
    dxBrush.Dispose();
  }

  if (RenderTarget != null)
    dxBrush = UserBrush.ToDxBrush(RenderTarget);
}

excuse me, but it's really very poor implementation of the idea, because there IS ACTUALLY needed to have the disposal code-block into 2 different places, which is quite annyoing, when we have to code into two places, for the aim to just dispose the brush when indicator id terminated. Can't you see? that "dxBrush" is said there to be handled in 2 different places. so, that's what NT had something bad in design. We need to have a place, where we can create ONCE and Dispose once (for that , yes, we used only Terminate state in the past, which seems to be incorrect).

The only even what that `dxBrush` is disposed, is in "State.Finalized" event. In all other cases, `(RenderTarget!=null)` is evaluated to true, and the `dxBrush` is being recreating. Is not that true?

That's why I suggested, that to reduce the burden and extra coding-requirements, we could (from above example) remove that code-block from "State.Terminated" area, and make a code to have the dispose-handing only in once place. I suggested to have either this:

A)

protected override void OnStateChange()
{
  if (State == State.SetDefaults)
  {
    Name = "OnRenderTargetChanged Example";
    IsOverlay = false;
    UserBrush = Brushes.Red;
  }

  if (State == State.DataLoaded)
  {
     if (RenderTarget != null)
        dxBrush = UserBrush.ToDxBrush(RenderTarget);
     }
  }
}

public override void OnRenderTargetChanged()
{
  if (State==State.Finalized)
  {
     if (dxBrush != null)
     {
       dxBrush.Dispose();
     }
  }

or

B)

protected override void OnStateChange()
{
  if (State == State.SetDefaults)
  {
    Name = "OnRenderTargetChanged Example";
    IsOverlay = false;
    UserBrush = Brushes.Red;
  }
}

public override void OnRenderTargetChanged()
{
  if (State==State.Finalized)
  {
     if (dxBrush != null)
     {
       dxBrush.Dispose();
     }
  }
  else
  {
     if (RenderTarget != null)
        dxBrush = UserBrush.ToDxBrush(RenderTarget);
     }
  }

so, as you see, there is no longer need to have that handling of disposals and duplicated code in two places.

`OnRenderTargetChanged` really DOES happen when indicator is removed/chart closed. and that always happens when State is FINALIZED.


2) I can't understand"

• Your example might not unhook the event until after the hosting indicator has already reached State.Terminated and therefore would be prone to leaking memory. If you need to perform cleanup in your plugins you could make a Cleanup method which you called from your indicator's OnStateChange method when State == State.Terminated.

So, you say when when "State.Terminated' happens, my exammple of `OnRenderTargetChanged` might not be evaluated? But if that could happen, then your DOCS example will also fail, because as i said above, the real disposal, according to your docs, could theorically happen when State is Finalized . Even your example code, is evaluated in State.Finalized, not just my example. In my example, i just show that State is Finalized at that moment. So, that's why I cant understand any advantage of your DOCS example code over my suggested method..

3) actually, OnRenderTargetChange happens numerous times, when we click on chart and etc, so, on all those cases, we are disposing and recreating again the resources. that's why it's inefficient.

[NinjaTrader_Jim]

Hello TazoTodua,

The example quoted in the Help Guide is an older example which had missed a documentation update. This example will be amended to exclude disposing of the brush in State.Terminated.

I have created a video demonstration printing the states iterated so we can observe the NinjaScript's LifeCycle, when OnRenderTargetChanged occurs, and if RenderTarget is null when OnRenderTargetChanged occurs.

Demo - https://drive.google.com/file/d/1EnC...w?usp=drivesdk

LifeCycle - https://ninjatrader.com/support/help...fecycle_of.htm

As we can see, OnRenderTargetChanged is always called with RenderTarget == null after State.Terminated occurs at the end of the NinjaScript's LifeCycle. NinjaScripts will reach State.Terminated throughout the cloning process, but the important takeaway is that OnRenderTargetChanged gets called with RenderTarget == null at the end of the script's life, so the resources get disposed and will not be recreated.

There are going to be cases when NinjaTrader will need to recreate the RenderTarget, and SharpDX resources will need to be recreated. These cases are noted in the OnRenderTargetChanged help guide documentation. This is a bare minimum requirement for creating valid resources for render passes without going to the extreme of creating at the beginning of OnRender() and disposing at the end of OnRender(), which would be less efficient.

As SharpDX resources are handled with OnRender() and OnRenderTargetChange() you would only need to use these methods for SharpDX resource management and rendering. State.Finalized would not be the advised way to dispose of resources, and we would advise disposing of any other resources (like a timer) in State.Terminated for the script that adds the resource.

Please let us know if you have any additional questions.

[ttodua]

1) Jim, then I think you have to update that docs page https://ninjatrader.com/support/help...getchanged.htm ( I see probably you changed something there today, but the bottom example still uses the disposal code in "State.Terminated" example).
2) well, of course what you say, is of more expertise. Just i dont know what disadvantages State.Finalized has (that was what i was interested) and if there are cases when it is not reliable.

Thanks Jim ! good luck.

[NinjaTrader_Jim]

Hello TazoTodua,

Product Management reviews changes to the Help Guide and we do not make these changes on our own. They are aware of this item and changes will be pushed.

State.Finalized was not exposed to be a user facing state. Because of which, we would not want to go into supporting it as everything should be possible with the current states. (SharpDX resources being managed in OnRender/OnRenderTargetChanged and not in OnStateChanged)

Moving forward, if you have any questions on how something can be disposed that you think would be necessary with State.Finalized, please let us know so we can give advice for how State.Terminated should be used instead.

For example, your code posted in the thread here should not check for State.Finalized in the OnMouseDownEvent, and instead the event should be subscribed in State.DataLoaded, and then unsubscribed in State.Terminated when we have already subscribed. You could use bools to ensure that you have subscribed before attempting to unsubscribe. Our SampleCustomEvents example which adds a timer should be referenced for proper (non-SharpDX) resource management and subscribing/unsubscribing.

SampleCustomEvents - https://ninjatrader.com/support/help...to_output_.htm

I look forward to being of any further assistance.

[ttodua]

thanks for the demonstration video too.. (just it was also good that in that demonstration video, in "OnRenderTargetChange" you have printed "State" too, to see what I was talking about).

Jim, yes, i wanted to say, that there are cases when something cant be done with State.Terminate cant do what State.Finalize can do.
And a simple example is that checking for State.Terminated, requires the access and modification to the script itself (writing an extra code in "OnStateChange", which happens only once).

So, without modification of the script, external scripts can hook onto State.Finalized, even after OnStateChange happend, and even if we hadn't any code in "OnStateChange"

The example is this addon (btw, I've committed that addon for the reason that you do a nice support to us
https://ninjatrader.com/support/foru...ebugger-window


there I used "State.Finalized" to check if the indicator was ended. That was the only way for me at least to succeed in such cases. So, State.Finalized is good for external scripts, to hook into target indicators without modification their code.

However, thank you for everything.

[NinjaTrader_Jim]

Hello TazoTodua,

Thank you for your comments.

We would not recommend using State.Finalized to externally check if a script has reached its end of life to dispose resources. We would recommend to instead call helper methods for creating and disposing resources in the hosting script's State.DataLoaded and State.Terminated states.

For example, instantiate your AddOn object in your indicator in State.DataLoaded and create any resources there. Within State.Terminated, check if that object has been created and dispose of the resources there. The AddOn should not need to monitor a NinjaScript state and should instead offer construction/destruction methods that would be used in State.DataLoaded and State.Terminated.

Your interest behind using State.Finalized is noted in our Suggestions and Features tracking system with the detail you have noted.

Please let us know if you have any additional questions.

[ttodua]

thanks Jim, that is what we are into different paths. Your suggested way (to modify the hosting scripts in order to add some functionalities onto exiting indicators/strategies) it radically against of what I want to achieve and what State.Finalized offers (in contrary to that suggestion).
however, thanks for help.

these two approaches are different, and I wouldnt recommend that "suggested" way to modify/update current scripts (even they might be hundreds) and even touching anything existing, in order to hook/add a functionality inside them. That's why I prefer the way i am talking about, and to say objectively, is much cleaner and easier to do, than modifying/altering existing scripts (even more, when we distribure our "hookable" addons, we can't modify the user's indicator, instead we need to hook into their functionality, and only State.Finalized gives us ability to check the ending-lifestate, as opposed to State.Terminated, which is only accessible only in a "onStateChange" event.)

many thanks, i am a bit worried that you didnt fully understand what i have been trying to explain and what was my desire, however discussing things so deep from you is more than appreciated!
thanks, i wont hold you more on this topic !
regards