API Question Vessel deletion in multi-vessel multi-MFD AP

[Mythos]

Hello,

I searched the forum and found some hints to this topic like deriving from template multi-vessel-class, or saying "always check vessel by name before using". Since I want not only multi-vessel but also multi-MFD (within one vessel) persistancy, I do almost standard but had do it on my own: I have 2 classes MfdData and VesselData, each are stored in global lists MfdList and VesselList.

If a deletion is noticed, I use the (deprecated) method:

DLLCLBK void opcDeleteVessel( OBJHANDLE  hVessel )
{
  mfdList->Delete( hVessel );
  vesselList->Delete( hVessel );
}

My AP is called each step (AP is a vessel-topic not a MFD-topic):

DLLCLBK void opcPreStep (double SimT, double SimDT, double mjd)
{
  vesselList->Autopilot_Execute(SimT, SimDT, mjd);
  // that will call the AP_Execute method for each vesselData in list
}

For safety then has to be checked, if the vessel for that the AP is called really still exists. As mentioned before I found "check by name" rule. But I don't feel quite well with string comparison for each element in list each timestep

So I think this must be done by handle. But when I try this, orbiter crashes (my Data holds the vessel pointer and the vessel handle):

void VesselData::AP_Execute(double SimT, double SimDT, double mjd)
{
  if(SimT - t_ap > dt_ap)
  {
    if(vessel != oapiGetVesselInterface(handle))
    {
      // ignore and disable AP, so it may be ignored forever
      ...

My intention was oapiGetVesselInterface will return NULL, if the handle doesn't exist (anymore), what is obviously doesn't

Any other ideas? Am I on the right way?

Greets
Mythos

 

[Mythos]

if anyone is interested, now I am using

if(!oapiIsVessel(handle) || (vessel != oapiGetVesselInterface(handle)))
{
  // ignore and disable AP, so it may be ignored forever
  ...

 

[Face]

I didn't get why you have to check the handles first place, if you already delete handles if notified by Orbiter. Order of callbacks, maybe?

I have a similar setup in OMP (although using the new module callbacks instead of the deprecated opc-callbacks) and it works out just fine.

regards,
Face

 

[Mythos]

Yes, I don't know which one comes first. And it is always better to check a handle before using ist.

All I know how to to deleting stuff I get here from this forum. They say I have to check it first. And I found the way to check for deletion only in old style. Any hint to get started with new style?

 

[Face]

At the few places I have to do this in my code base (e.g. here), I do it like you outlined in your second post.

For the "new style", you have to create a class derived from oapi::Module, create an instance and register it. The easiest way to do this is by means of the following code:

class MyModule : public oapi::Module
{
public:
    MyModule(HINSTANCE hDLL);
    ~MyModule(void);
    virtual void clbkDeleteVessel(OBJHANDLE hVessel);
private:
    HINSTANCE hDLL;
};

MyModule::MyModule(HINSTANCE hDLL):Module(hDLL)
{
    this->hDLL=hDLL;
    //Do your init code
}

MyModule::~MyModule(void)
{
    //Do your exit code
}

void MyModule::clbkDeleteVessel(OBJHANDLE hVessel)
{
    //Do your vessel delete code
}

DLLCLBK void InitModule(HINSTANCE hDLL)
{
    oapiRegisterModule(new MyModule(hDLL));
}

regards,
Face

 

[Hielor]

Perhaps I'm completely missing something, but doesn't the MFD class get created once per instance of the MFD? You shouldn't need to be keeping track of your own vessels/MFDs in a global variable in your module, I'd think...

 

[Enjo]

I have a project just for that, which can be found here:
http://enjomitchsorbit.svn.sourceforge.net/viewvc/enjomitchsorbit/multipleVesselsMFD/

Look here how I handle multiple vessels:
http://enjomitchsorbit.svn.sourcefo...nMultipleVessels.cpp?revision=203&view=markup

The project is documented in Launch MFD docs. You can derive from my classes, as it's done in the example project. There's only one global variable for all of that.

I also must add, that your and my "Storage" class deletion is very dangerous. It will break some addons, probably those which hold pointers to invalid vessels (yuck!), therefore I only delete these structures in the destructor, upon simulation exit.

 

[Face]

I also must add, that your and my "Storage" class deletion is very dangerous. It will break some addons, probably those which hold pointers to invalid vessels (yuck!), therefore I only delete these structures in the destructor, upon simulation exit.

I don't think that it will break addons that are developed independent of the MFD module (i.e.: don't share headers or exchange data). If an addon holds pointers to invalid vessels, it will break anyway.

I also don't think it is dangerous, or that it smells of a hack. The whole point of having a delete callback is to clean up data structures, isn't it? The method of holding only the vessel name and always resolve it to the handle feels more like a hack for me.

regards,
Face

 

[Enjo]

Theoretically you're absolutely right but I have my experiences that tell me that any deletions in Orbiter are insecure and lead to unreproducible CTDs when theoretically they shouldn't. It may be something with the addons, and it may be something with the Orbiter Core. The method I presented just keeps the problem away at a small cost of keeping the structures in memory until the simulation ends.

The method of holding only the vessel name and always resolve it to the handle feels more like a hack for me.

Did you mean a pointer? What's the problem with that if it prevents CTDs at undefined moments?

 

[Mythos]

Wow, now it gets more like a productive discussion in here

Thank you Face, that gave me the clue to do it on the more modern way :thumbup:

I left my InitModule() and ExitModule() where it was (in my MFD class that was a copy of martins MFDTemplate.cpp). In InitModule I register a new instance of my new Module class.

Then I put the global lists (I never liked them to be there) in my Module class. And there I built clbkDeleteVessel() and clbkPreStep() and removed the DLLCLBK opc versions after moving all lines to the new class.

Just a little work adding a new class, moving some lines of code and it is modern style
And the best: MFD and AP still work, even after deleting a vessel :thumbup:

Thanks to Enjo anyway, I studied LaunchMFD months ago when I started my procect. I didn't understand very well (today I do better) so decided to rebuild on my own.

Like Face I don't think this could be a problem for any other addons, since we just keep lists with pointers or handles to vessels that used our addon. No other code uses our lists, so there is no problem. And keeping the lists up to date at deletion is for stability of our addon only.

And like I said before and Face now repeated, keeping vessel names and resolving them every timestep didn't feel right and wasn't necessary for me.

I think now I got that cleared for me. Thank you!

 

[Enjo]

I've recalled what the problem had been:
It was not other addons actually. The problem was that Orbiter sometimes first deletes a vessel, calls the deletion callback and THEN calls MFD::Update, only to find that there's a deleted storage class, but you cannot check it anymore without causing a CTD. That's why I invalidate the storage object in the callback and check its boolean value in MFD::Update.

 

[Mythos]

For such cases was meant my

if(!oapiIsVessel(handle) || (vessel != oapiGetVesselInterface(handle)))
{
  // ignore and disable AP, so it may be ignored forever

I have to admit, that maybe the "Storage" data instance may stay in memory and in my lists (only if the clbkDeleteVessel() never occurs). Important is that its vessel pointer/handle is never used. So I set an AP_Enabled to false. So it never will be used by my AP that goes over all list entries. And of couse it will never be used for a MFD, since a deleted vessel won't display MFDs anymore.

 

[Face]

Glad to see it worked out for you.

IMHO, if you use dynamic memory management, you should always strive to get it "right". It is all too tempting to deal with the symptoms instead of curing the disease in this case.

Easier said than done, I know. I'm certainly guilty of it myself, too. But never stop trying .

regards,
Face