C++ Question Populating a list of functions.

[Hlynkacg]

Whoops :facepalm:

I was trying to delete/erase the pointer in the first position, but, as the code stands, it tries to remove the pointer with an id of one. That code works for a vector, but not for a map.

The simple fix is

LM_Cockpit::~LM_Cockpit()
{
    while (vcSwitchList.size() != 0)
    {
        delete[COLOR="Red"] (*vcSwitchList.begin())[/COLOR];
        vcSwitchList.erase([COLOR="Red"]vcSwitchList.begin()[/COLOR]);
    }
}

Tested but got a compiler error...

are you sure that

delete (*vcSwitchList.begin());

shouldn't be

delete [COLOR="Red"]&[/COLOR](*vcSwitchList.begin());

??

 

[meson800]

Yep, that does lead to a compilation error. I wrote it while I was away from my computer, so I couldn't test it.

I'm unsure if your fix will work, as iterators can get weird. However,

LM_Cockpit::~LM_Cockpit()
{
    while (vcSwitchList.size() != 0)
    {
        delete vcSwitchList.begin()[COLOR="Red"]->second[/COLOR];
        vcSwitchList.erase(vcSwitchList.begin());
    }
}

will work, as it access the "second" value in the key-value pair key, which is just the value(the pointer to the struct)

 

[Hlynkacg]

Thank you meson, I have everything up and working now, but I also have a follow up question.

Is there some way to add a pointer in VCSWITCHSPEC to another void or function that would be called each time the switch's state changes?

I'm thinking that this would be more efficient in terms of memory and processing power than running if() checks every timestep as I am now. Am I right?

 

[meson800]

Yes, but it gets pretty complicated pretty fast, and I'm not sure if it would be much more efficient. The first method (directly storing a pointer to a function) requires function pointers and pointers to member functions. Say you have the following function (note, not in a class)

void makeCoffee(int cupsOfCoffee) 
{ 
  makeCoffee();
  getCups(cupsOfCoffee);
  fillCups();
}

Then, you would modify your struct to contain a function pointer

typedef struct
{
	int aid;		// VC active area id
	int nstate;		// number of valid switch positions;
	switchstate state;// current switch position
	double size;		// switch size (radius of clickable area in VC) 
	VECTOR3 pos;	// switch position in VC
	int sound_id;	// sound to play when clicked on
	UINT anim;		// associated animation
	[COLOR="Red"]void (*updateFunction)(int)	//my awesome function pointer[/COLOR]
	
} VCSWITCHSPEC;

This means that the struct stores a pointer to some function that takes a single integer argument and returns void. Then, for each of your switch cases (coffee maker, etc), you would have to do the following

VCSWITCHSPEC* AAPO_Cockpit::CreateSwitch (int id, VECTOR3 pos, int nstate = 2, double size = 0.01, int sound = SOUND_CLICK, UINT anim = NULL)
{
	VCSWITCHSPEC switchspec;

	switchspec.aid		= id;
	switchspec.pos		= pos;
	switchspec.nstate	= (nstate < 2 ?  2 : nstate);
	switchspec.size		= size;
	switchspec.sound_id = sound;
	switchspec.anim		= anim;
	switchspec.state	= DOWN;
	[COLOR="Red"]switchspec.updateFunction = &makeCoffee;[/COLOR]
        return new VCSWITCHSPEC(switchspec);
}

assuming that the createSwitch method can "see" the makeCoffee function.

Now, you can invoke that function by doing

VCSWITCHSPEC coffeeMaker;    //already initialized somewhere
coffeeMaker.updateFunction(10);       //because Orbiter requires many, many cups of coffee

Pointers to member functions (functions residing inside a member class) get even more complicated, involving "fun" operators like

->*
.*

------------------------------------------------------------------------
The second method might be better, and may make your code clearer.

In this case, you can create several subclasses of VCSWITCHSPEC, each of which overloads a certain function.

Example:
This is the way I do it in my (as of yet) unpublished simpit controller addon. You can access it if you want at https://github.com/meson800/simpit-controller/tree/develop)

In mine, I have the following vector (dynamic array).

class SimpitManager : public oapi::Module
{
public:
	SimpitManager(HINSTANCE hDLL);
	--some more methods--
private:
	vector<IO *> modules;
};

The IO class contains several methods and variables, similar to your VCSWITCHSPEC. Then, I create subclasses of IO. Because they are all subclasses, I am allowed to insert pointers to them into the vector. Here is a section for the "loading" function which initializes all of the modules. (PanelEventOutput,SerialInput,PanelClickRecorderOutput,and SimpitMFD are all subclasses of IO)

if (strcmp(moduleName, "BEGIN PANEL_EVENT_OUTPUT") == 0)
				modules.push_back(new PanelEventOutput());
			else if(strcmp(moduleName, "BEGIN SERIAL_INPUT") == 0)
				modules.push_back(new SerialInput());
			else if(strcmp(moduleName, "BEGIN PANEL_CLICK_RECORDER") == 0)
				modules.push_back(new PanelClickRecorderOutput());
			else if(strcmp(moduleName, "BEGIN SIMPIT_MFD") == 0)
				modules.push_back(new SimpitMFD(hDLL));

Each of the sub-modules override methods in IO, so the SimpitController class can correctly call each overloading function without knowing which subclass of IO each pointer refers to

Here's an example of that

for (unsigned int i = 0; i < modules.size(); i++)
	{
		modules[i]->handleEvent(ev);
	}

Enough of that example. If you want to, instead of using copious if statements, insert that code into the class, here's how you would do it.

First, define a function inside VCSWITCHSPEC like the following. Include any arguments that you want to pass to the switch before updating. (I made the example without arguments, I'm not sure what you want to do.

typedef struct
{
	int aid;		// VC active area id
	int nstate;		// number of valid switch positions;
	switchstate state;// current switch position
	double size;		// switch size (radius of clickable area in VC) 
	VECTOR3 pos;	// switch position in VC
	int sound_id;	// sound to play when clicked on
	UINT anim;		// associated animation
	[COLOR="Red"]virtual void updateSwitch() {}	//virtual so it can be overloaded
	AAPO_Cockpit * cockpitPointer;	//so we can access private variables inside the cockpit, might have to add "friend VCSWITCHSPEC" in AAPO_Cockpit[/COLOR]
} VCSWITCHSPEC;

In CreateSwitch, add the following so that the switch gets the correct pointer so it can do its stuff.

switchspec.cockpitPointer = this;

Then, for each unique switch type, create a subclass of VCSWITCHSPEC and overload updateSwitch. Put the update logic inside there.

struct CoffeeMaker : public VCSWITCHSPEC
{
    void updateSwitch() 
    {
     if (state == ON)
        {
          cockpitPointer->MainBusAmps -= 9.0;
          cockpitPointer->CoffeeReserve += BREWRATE * simdt;
   }
};

Repeat for each switch and its update logic.

Then, you have to initialize a unique instance of each subclass for each switch. You must do something like this.

vcSwitchList[id] = (CoffeeMaker*)CreateSwitch (id, P1_SWITCH_POS[i]);
vcSwitchList[id] = (GearSwitch*)CreateSwitch (id, P1_SWITCH_POS[i]);

Then, instead of if statements, just add the following near the ActivateSwitch code

ActivateSwitch(vcSwitchList[id],event);
[COLOR="Red"]vcSwitchList[id]->updateSwitch();[/COLOR]

This will automatically call the correct update code, regardless of if it is a coffee maker or a cargo door switch.

tl;dr
Having the switch call the update logic instead of AAPO_Cockpit involves a lot of work, but it does separate the logic in a more logical fashion. If you feel as if you can maintain the code right now, I probably wouldn't switch. If AAPO is going to get more complex with accurate subsystems and such, you might want to switch.

In what you have shown so far, there probably wouldn't be a large increase in efficiency.