C++ Question Populating a list of functions.

Hlynkacg · Nov 25, 2013

I'm not really sure how best to describe what I'm trying to do this but I will give it my best shot.

So apollo space craft have a lot of switches. In orbiter terms each of the switchs have some common variables that need to be saved/manipulated by the user. To facilitate this (and kepp my cockpit manager class somewhat manageable, I have created a structure called VCSWITCHSPEC.

Code:

// --------------------------------------------------------------
// Individual VC switch specification
// --------------------------------------------------------------
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
	
	// ??? **function pointer thingy goes here once I figure out how to make it work** // Associated function

} VCSWITCHSPEC;

I then have a "createVCswitch ()" function in my cockpit manager class that returns a VCSWITCHSPEC based on the arguments entered. This has made basic coding a lot easier as my original LM's cockpit code was an unmanageable mess. The problem is that I still end up with a lot of redundant and copy/pastaed code like the following in the body of my vessel's code.

Code:

// --------------------------------------------------------------
// Process cockpit mouse event notifications
// --------------------------------------------------------------
bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p)
{
	if (P1_AID) return ActivateSwitch (&p1_switch[id - P1_SWITCH_AID], event);
	else if (P2_AID) return ActivateSwitch (&p2_switch[id - P2_SWITCH_AID], event);
	else if (P3_AID) return ActivateSwitch (&p3_switch[id - P3_SWITCH_AID], event);

etc, etc, etc...

What i'd like to do is have an list or array containing all the switches that have been added to the cockpit thus far so that in clbkMouseEvent and other similar functions I can simply write a function to "check the list" rather than having to ensure that each switch has been properly added to the cockpit in 4 seperate places. (clbkMouseEvent, redrawevent, register active areas, and constructor/destructor)

In psuedo-code I imagine it would look something like ...

Code:

bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p)
{
  CheckVCSwitchList (); 
  if (MatchFound) ActivateSwitch (switch_id, event);
  else return false;
}

I also imagine that the simples way to do this would be to have a line in the the create and deleteswitch functions that says "add/remove this switch to/from list"

I just don't know what such a function or list would actually look like and I'm not sure of the proper vocabulary needed to search or ask.

BruceJohnJennerLawso · Nov 25, 2013

Hlynkacg said:
I'm not really sure how best to describe what I'm trying to do this but I will give it my best shot.

So apollo space craft have a lot of switches. In orbiter terms each of the switchs have some common variables that need to be saved/manipulated by the user. To facilitate this (and kepp my cockpit manager class somewhat manageable, I have created a structure called VCSWITCHSPEC.

Code:

// -------------------------------------------------------------- // Individual VC switch specification // -------------------------------------------------------------- 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 // ??? **function pointer thingy goes here once I figure out how to make it work** // Associated function } VCSWITCHSPEC;

I then have a "createVCswitch ()" function in my cockpit manager class that returns a VCSWITCHSPEC based on the arguments entered. This has made basic coding a lot easier as my original LM's cockpit code was an unmanageable mess. The problem is that I still end up with a lot of redundant and copy/pastaed code like the following in the body of my vessel's code.

Code:

// -------------------------------------------------------------- // Process cockpit mouse event notifications // -------------------------------------------------------------- bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p) { if (P1_AID) return ActivateSwitch (&p1_switch[id - P1_SWITCH_AID], event); else if (P2_AID) return ActivateSwitch (&p2_switch[id - P2_SWITCH_AID], event); else if (P3_AID) return ActivateSwitch (&p3_switch[id - P3_SWITCH_AID], event); etc, etc, etc...

What i'd like to do is have an list or array containing all the switches that have been added to the cockpit thus far so that in clbkMouseEvent and other similar functions I can simply write a function to "check the list" rather than having to ensure that each switch has been properly added to the cockpit in 4 seperate places. (clbkMouseEvent, redrawevent, register active areas, and constructor/destructor)

In psuedo-code I imagine it would look something like ...

Code:

bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p) { CheckVCSwitchList (); if (MatchFound) ActivateSwitch (switch_id, event); else return false; }

I also imagine that the simples way to do this would be to have a line in the the create and deleteswitch functions that says "add/remove this switch to/from list"

I just don't know what such a function or list would actually look like and I'm not sure of the proper vocabulary needed to search or ask.

I think std::vector might be along the lines of what you want, although it will require the standard library.

http://www.cplusplus.com/reference/vector/vector/

Urwumpe · Nov 25, 2013

Instead of a function list, you can also use a class, that defines a common interface, and let derived classes implement the behavior.

This is especially useful, if you have handlers, in which only a few variables change, like a action: "ACTIVATE SUBSYSTEM xyz".

Hlynkacg · Nov 25, 2013

Urwumpe said:
Instead of a function list, you can also use a class, that defines a common interface, and let derived classes implement the behavior.

This is especially useful, if you have handlers, in which only a few variables change, like a action: "ACTIVATE SUBSYSTEM xyz".

I was actually think along similar lines but wasn't sure how to implement it because the handler class would still need some way of knowing what ids and functions belonged to it and where to find them.

meson800 · Nov 25, 2013

The issue with std::vector is that it would be difficult to do

Code:

 if (MatchFound)

I would probally use an std::map because you need to find a switch based off of its id
http://www.cplusplus.com/reference/map/map/

You would use it like the following

Code:

//initialization
std::map<int,VCSWITCHSPEC *> vcSwitchList;

//createswitch function
void createSwitch(VCSWITCHSPEC * thisSwitch)
{
  vcSwitchList[thisSwitch->aid] = thisSwitch;
}

//deleteswitch function
void deleteSwitch(VCSWITCHSPEC * thisSwitch)
{
  vcSwitchList.erase(thisSwitch->aid);
}

Also, to do

Code:

bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p)
{
  CheckVCSwitchList (); 
  if (MatchFound) ActivateSwitch (switch_id, event);
  else return false;
}

you can do

Code:

bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p)
{
  if (vcSwitchList.count(id) != 0) 
    ActivateSwitch (id, event);
  else return false;
}

Urwumpe · Nov 25, 2013

Hlynkacg said:
I was actually think along similar lines but wasn't sure how to implement it because the handler class would still need some way of knowing what ids and functions belonged to it and where to find them.

Not really - where the event comes from is not that important. What matters is, where it should go to.

In SSU, we used the DiscreteBundle interface to simulate signals transmitted by setting or resetting a voltage on a cable, and reuse the interface also for simulating analog signals.

Where the signal comes from and how it is processed does not matter anymore, VC and simulation are decoupled. We could switch to a 2D panel, and the interface would still work. We could use the classes in a new vessel, and the interface would still work.

And that had been the goals of the interface.

Hlynkacg · Nov 25, 2013

Urwumpe said:
Not really - where the event comes from is not that important. What matters is, where it should go to.

In SSU, we used the DiscreteBundle interface to simulate signals transmitted by setting or resetting a voltage on a cable, and reuse the interface also for simulating analog signals.

Where the signal comes from and how it is processed does not matter anymore, VC and simulation are decoupled. We could switch to a 2D panel, and the interface would still work. We could use the classes in a new vessel, and the interface would still work.

And that had been the goals of the interface.

Ok but that does not seem to address the problem that I am trying to solve.

My VCSWITCHSPEC can already be easily modified to be generic (work with both 2d and 3d panels). Likewise, my APPO_VC class already contains a standardized interface for VCSWITCHSPEC. That is not the problem. My problem is that I want to access a switch based on the cockpit active area id without having to go through each switch individually looking for a match as I currently do.

If I add the switch in the initialize and in clbkMouseEvent but forget to add it to clbkRedrawEvent the whole thing comes crashing down. I'd like to avoid that by having a generic "thingy" that takes an id and returns a SWITCHSPEC without the need for lot of hard to maintain copy/pasting or long (we're talking 218 switch-long) chains of if, if else, if else, if else, if else... :compbash2:

BruceJohnJennerLawso · Nov 25, 2013

meson800 said:
The issue with std::vector is that it would be difficult to do

Code:

if (MatchFound)

I would probally use an std::map because you need to find a switch based off of its id
http://www.cplusplus.com/reference/map/map/

Thanks, I would imagine that will be better. I just use vector for everything right now since I know it and don't have time to study any of the other containers right now. I think you could still use vector for this, but it would require an iteration & search in order to find the right vector element based on an id.

Hlynkacg · Nov 25, 2013

meson800 said:
The issue with std::vector is that it would be difficult to do

Code:

if (MatchFound)

I would probally use an std::map because you need to find a switch based off of its id
http://www.cplusplus.com/reference/map/map/

You would use it like the following

Code:

//initialization std::map<int,VCSWITCHSPEC *> vcSwitchList; //createswitch function void createSwitch(VCSWITCHSPEC * thisSwitch) { vcSwitchList[thisSwitch->aid] = thisSwitch; } //deleteswitch function void deleteSwitch(VCSWITCHSPEC * thisSwitch) { vcSwitchList.erase(thisSwitch->aid); }

Also, to do
you can do

Code:

bool LM_Cockpit::clbkMouseEvent (int id, int event, VECTOR3 &p) { if (vcSwitchList.count(id) != 0) ActivateSwitch (id, event); else return false; }

Ok this "map" stuff looks promising I assume that "vcSwitchList" in this case be a cockpit class member? and that "std::map<int,VCSWITCHSPEC *> vcSwitchList;" goes in the class declaration?

Likewise my create switch function would go something like...

Code:

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"] vcSwitchList [id] = switchspec;[/COLOR]
	return switchspec;
}

just a quick example.

the basic syntax for the code in my cockpit initializer looks like

Code:

Switch_CoffeeMaker = CreateSwitch (blah blah blah);

then in the vessel's code proper (not the cockpit's)

Code:

if (Switch_CoffeeMaker->state == on) make coffee

ETA:
How do i go about getting/including the standard library and is that going to cause problems for people who download my addon and don't have it?

meson800 · Nov 25, 2013

Ok this "map" stuff looks promising I assume that "vcSwitchList" in this case be a cockpit class member? and that "std::map<int,VCSWITCHSPEC *> vcSwitchList;" goes in the class declaration?

Yes, with one change :thumbup:

Make the declaration this:

Code:

std::map<int,VCSWITCHSPEC> vcSwitchList;

You aren't storing polymorphic classes, so you can directly store a VCSWITCHSPEC in the map. In some of my code, I was using polymorphism, so I had to store a pointer to the base class. Luckily, you don't have to.

Hlynkacg · Nov 25, 2013

Cool, and again, I assume that to make this work I just need to make sure that I have "#include <map>" somewhere in my cockpit class' header?

meson800 · Nov 25, 2013

Yes :thumbup:

Good luck! :cheers:

Hlynkacg · Nov 25, 2013

Thank you, got VS open now so give me a few minutes and I'll let you know if it works.

---------- Post added at 14:41 ---------- Previous post was at 14:08 ----------

ok, problem.

my "ActivateSwitch()" takes the VCSWITCHSPEC that is to manipulated as it's argument not an int.

I figure that this would be easily adressed by...

Code:

// --------------------------------------------------------------
// Process cockpit mouse event notifications
// --------------------------------------------------------------
bool LM_Cockpit::MouseEvent (int id, int event, VECTOR3 &p)
{
	if (vcSwitchList.count(id) != 0)
	{
		VCSWITCHSPEC switchspec = vcSwitchList[id];
		return ActivateSwitch (&switchspec, event);
	}
	else return false;

but while the function is operating the switch's state is not being updated.

Thoughts?

PS:
Body of the Switch interface functions...

Code:

// --------------------------------------------------------------
// Switch interface logic
// --------------------------------------------------------------
bool AAPO_Cockpit::ActivateSwitch (VCSWITCHSPEC *switchspec, int event)							// Switch interface logic
{
	int state = switchspec->state;
	int newstate = 0;
	int maxstate = switchspec->nstate - 1;

	// If switch has 2 or fewer defined positions, process it as a binary (on/off) switch...
	if (switchspec->nstate <= 2)
	{
		if (event & PANEL_MOUSE_LBDOWN) newstate = UP;			// If left mouse button set switch to "UP" position
		else if (event & PANEL_MOUSE_RBDOWN) newstate = DOWN;	// Else if right mouse button set switch to "DOWN" position
	}
	// ...if not, process it as a multi-position switch/dial
	else
	{
		if (event & PANEL_MOUSE_LBDOWN) newstate = state + 1;		// If left mouse button increase state by 1.
		else if (event & PANEL_MOUSE_RBDOWN) newstate = state - 1;	// if right mouse button decrease it.

		// Sanity check, restrict "newstate" to valid range.
		if (newstate < 0) newstate = 0;
		else if (newstate > maxstate) newstate = maxstate; 
	}

	// Update switch position
	if (SetSwitchState (switchspec, newstate))
	{
		PlayVesselWave (v->OrbiterSoundID, switchspec->sound_id);
		return true;
	}
	else return false;
} 

// --------------------------------------------------------------
// Set switch state
// --------------------------------------------------------------
bool AAPO_Cockpit::SetSwitchState (VCSWITCHSPEC *switchspec, int newstate)
{
	int state = switchspec->state;

	if (state != newstate)
	{
		switchspec->state = (switchstate)newstate;
		return true;
	}
	else return false;
}

meson800 · Nov 25, 2013

Yes. When you do

Code:

VCSWITCHSPEC switchspec = vcSwitchList[id];

You create a copy of the switchspec. Then, you modify the copy's value, so you get the errors reported, as you never change the original switchspec.

You can fix this in two ways. Either do

Code:

bool LM_Cockpit::MouseEvent (int id, int event, VECTOR3 &p)
{
	if (vcSwitchList.count(id) != 0)
	{
		[COLOR="Red"]//remove this line[/COLOR]VCSWITCHSPEC switchspec = vcSwitchList[id];
		return ActivateSwitch ([COLOR="Red"]&(vcSwitchList[id])[/COLOR], event);
	}
	else return false;

to give ActivateSwitch a direct pointer to the VCSWITCHSPEC inside the map.

Or you can modify the map to directly store pointers to VCSWITCHSPEC's

First, modify the declaration

Code:

std::map<int,VCSWITCHSPEC [COLOR="Red"]*[/COLOR]> vcSwitchList;

Then modify the CreateSwitch function

Code:

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"]
  vcSwitchList [id] = new VCSWITCHSPEC(switchspec);[/COLOR]
	return switchspec;
}

Hlynkacg · Nov 25, 2013

went with the first option but still having problems.

parts of the switch functionality work while others do not. I suspect that the problem is that the problem is a variation on the "copying" problem from before where in my original switch pointers ie "Switch_CoffeeMaker" from my earlier example and "vcSwitchList [id]" are no longer pointing to the same spec instance.

I tried to address this by by having the create switch function return the map value but no luck.

Code:

// --------------------------------------------------------------
// Create cockpit switch
// --------------------------------------------------------------
VCSWITCHSPEC AAPO_Cockpit::CreateSwitch (int id, VECTOR3 pos, int nstate, double size, int sound, UINT anim)
{
	VCSWITCHSPEC switchspec;

	switchspec.aid		= id;							// id cockpit acrive area associated with this switch 
	switchspec.pos		= pos;							// position of this switch on panel
	switchspec.nstate	= (nstate < 2 ?  2 : nstate);	// number of valid switch positions minimum of 2 (on/off)
	switchspec.size		= size;							// radius of clickable area
	switchspec.sound_id = sound;						// sound to play when switch is activated
	switchspec.anim		= anim;							// animation to update when switch is activated ()
	switchspec.state	= (switchstate)0;				// initialize state variable

[COLOR="Red"]	vcSwitchList [id] = switchspec;						// add this switch to the cockpit map.
	return vcSwitchList[id];							// return switchspec.
[/COLOR]}

meson800 · Nov 26, 2013

Can you post all of the places where vcSwitchList is used?

"Switch_CoffeeMaker" from my earlier example and "vcSwitchList [id]" are no longer pointing to the same spec instance.

The issue is that vcSwitchList[id] is not a pointer, it is the spec instance. As you said, the issue probally crops up when Switch_CoffeeMaker points to a copy of vcSwitchList.

Also, as the code is right now, the function CreateSwitch is probably the culprit. Becaue the return value is just "VCSWITCHSPEC", CreateSwitch copies vcSwitchList[id] before returning it, and any use of the return value will just operate on a copy.

An easy fix is just to modify CreateSwitch so it returns a reference. Change the signature to

Code:

VCSWITCHSPEC [COLOR="Red"]&[/COLOR] AAPO_Cockpit::CreateSwitch (int id, VECTOR3 pos, int nstate, double size, int sound, UINT anim)
{
--snip--

Now, the function just returns a reference to vcSwitchList[id]. You do not have to change any other code for this to work. However, this MAY result in undefined behavior/crashing, as the reference is to a local variable.

Overall, you may want to change the map to storing pointers. This would eliminate any undefined behavior and (hopefully) remove the copying problem.

---------- Post added at 07:18 PM ---------- Previous post was at 07:15 PM ----------

I looked up the above, and it will cause stack corruption and errors. Don't do it. If you post all of the code operating on vcSwitchList, I can help with converting to pointers.

---------- Post added at 07:49 PM ---------- Previous post was at 07:18 PM ----------

Gaaa :facepalm:
Disregard the previous. As you are returning a reference to a non-stack variable (the struct inside the map), nothing goes wrong. There is no corruption. The corruption would occur if you returned switchspec, which you aren't doing.

Hlynkacg · Nov 26, 2013

Thank you,

I suppose that it would help to get some background on the basic coding layout/methodology first. I am working on an addon that comprises modest but growing family of vessels based on Apollo Applications Project. see my sig.

All player controllable vessels in the addon are derived from a parent class called AAPO_Vessel which in turn has two member "sub" classes called AAPO_Cockpit, and AAPO_Guidance. AAPO_Cockpit is the one that we are concerned with here as it's job is to interpret user inputs and update VC/Panel displays.

VCSWITCHSPEC's interface functions are all members of AAPO_Cockpit and the struct itself is in it's header file.

Code:

class AAPO_Cockpit
{
	public:
		AAPO_Cockpit (AAPO_Vessel *vessel);
		~AAPO_Cockpit ();
		
		friend class	AAPO_Vessel;
		friend class	AAPO_Guidance;

		// Switch Functions
		VCSWITCHSPEC	CreateSwitch (int id, VECTOR3 pos, int nstate = 2, double size = 0.01, int sound = SOUND_SWITCH, UINT anim = NULL); // Initialize a VCSWITCHSPEC
		void			DeleteSwitch (VCSWITCHSPEC *switchspec);
		bool			ActivateSwitch (VCSWITCHSPEC *switchspec, int event);							// Switch interface logic
		bool			AnimateSwitch (VCSWITCHSPEC *switchspec);										// Animate a physical switch (returns false if no animation defined)
		void			RegisterVCArea (VCSWITCHSPEC *switchspec, VECTOR3 ofs = NULL_VECTOR);			// Register VC active area
		bool			SetSwitchState (VCSWITCHSPEC *switchspec, int newstate);						// Modify switch state

		// Display Functions
etc...

I'm currently working on AAPO's lunar taxi so most of the work is going on in my LM_Cockpit class which is a derivative of AAPO_Cockpit specific to lunar module-based vessels.

Code:

// ==============================================================
// Cockpit class interface
// ==============================================================
class LM_Cockpit: public AAPO_Cockpit
{
	friend class AAPO_LM;

	public:
		LM_Cockpit (AAPO_LM *vessel);
		~LM_Cockpit ();

		bool	LoadVirtualCockpit (int id);							// Process cockpit mode requests 
		bool	MouseEvent (int id, int event, VECTOR3 &p);				// Process cockpit mouse event notifications
		bool	RedrawEvent (int id, int event, SURFHANDLE surf);		// Process cockpit redraw event notifications 
		void	SaveState (FILEHANDLE scn);								// Write satus to scenario file
		bool	ParseScenarioLine (char *line);							// Read scenario lines

		// Cockpit switches
		VCSWITCHSPEC	p1_switch[P1_SWITCH_COUNT], p2_switch[P2_SWITCH_COUNT], p3_switch[P3_SWITCH_COUNT], p4_switch[P4_SWITCH_COUNT]; 

		// Cockpit displays
		VCMFDSPEC mfd_CMD, mfd_LMP;
		VCHUDSPEC	 hud_COAS;
etc...

As you can see I have multiple arrays of VCSWITCHSPECs defined. These were initial put in place so a i could use for... functions to hand repetitive tasks.

for instance this is the initialization for Panel 1's switches in the LM_Cockpit class constructor...

Code:

	// Initialize panel 1 switches
	for (int i = 0; i < P1_SWITCH_COUNT; i++)
	{
		int id = P1_SWITCH_AID + i;

		switch (i)
		{
		case 3: case 15: case 16: case 18: // Three-position switches
			p1_switch[i] = CreateSwitch (id, P1_SWITCH_POS[i], 3);
			break;

		default: // basic 2-position toggles
			p1_switch[i] = CreateSwitch (id, P1_SWITCH_POS[i]);
			break;
		}
	}

These arrays are also used in conjunction with a header file called "LM_SwitchDefs" to communicate with the LM vessel itself.

to use an earlier example

in LM_SwitchDefs.h you might see

Code:

#define SWITCH_COFFEEMAKER		p4_switch[22].state

and then in LM.cpp you'd see something like...

Code:

void AAPO_LM::clbkPostStep (double simt, double simdt, double mjd)
{
   if (vc->SWITCH_COFFEEMAKER == ON)
   {
     MainBusAmps -= 9.0;
     CoffeeReserve += BREWRATE * simdt;
   }

or more realistically...

Code:

if ((vc->SWITCH_GEARDEPLOY == ON) && (!GearDeployed)) DeployLandingGear ();

The problem with this system of doing things is as I described before. certain calback functions, specifically LoadVC, MouseEvent, and ReDrawEvent become nightmares to maintain because they end up consisting of long chains of.

Code:

...
	else if (id == p2_switch[8].aid) return ActivateSwitch (&p2_switch[8], event);
	else if (id == p2_switch[9].aid) return ActivateSwitch (&p2_switch[9], event);
	else if (id == p2_switch[10].aid) return ActivateSwitch (&p2_switch[10], event);
	else if (id == p2_switch[11].aid) return ActivateSwitch (&p2_switch[11], event);

... and so on :suicide:

---------- Post added at 19:06 ---------- Previous post was at 18:13 ----------

So, with all that out of the way, here are all the places that the switchspecs are used...

in the LM_Cockpit class costructor they get initialized like so...

Code:

	// Panel 1 switches
	for (int i = 0; i < P1_SWITCH_COUNT; i++)
	{
		int id = P1_SWITCH_AID + i;

		switch (i)
		{
		case 3: case 15: case 16: case 18: // Three position switches
			p1_switch[i] = CreateSwitch (id, P1_SWITCH_POS[i], 3);
			break;

		default: // basic 2-position switches toggles
			p1_switch[i] = CreateSwitch (id, P1_SWITCH_POS[i]);
			break;
		}
	}

rinse wash repeat for each panel, 12 total.

in clbkLoadVC

Code:

		for (int i = 0; i < P1_SWITCH_COUNT; i++) RegisterVCArea (&p1_switch[i], ofs);
		for (int i = 0; i < P2_SWITCH_COUNT; i++) RegisterVCArea (&p2_switch[i], ofs);
		for (int i = 0; i < P3_SWITCH_COUNT; i++) RegisterVCArea (&p3_switch[i], ofs);
		for (int i = 0; i < P4_SWITCH_COUNT; i++) RegisterVCArea (&p4_switch[i], ofs);

etc...

There are also the animation definitions that look like...

Code:

void LM_Cockpit::AddAnimations ()
{
	// Define panel 1 animations 
	static UINT mgroup_P1switches[P1_SWITCH_COUNT];

	for (int i = 0; i < P1_SWITCH_COUNT; i++) 
	{
		mgroup_P1switches[i] = (GRP_VC_Switch_P1_000 + i);

		p1_switch[i].anim = v->CreateAnimation (0.5);
		mgt_P1_switch[i] = new MGROUP_ROTATE (mesh, &mgroup_P1switches[i], 1, P1_SWITCH_POS[i], X_AXIS, PI/4);
		v->AddAnimationComponent (p1_switch[i].anim, 0.0f, 1.0f, mgt_P1_switch[i]);
	}
...repeated for each panel

and finally in clbkMouseEvent we have...

Code:

if (id == p1_switch[0].aid) return ActivateSwitch (&p1_switch[0], event);
	else if (id == p1_switch[1].aid) return ActivateSwitch (&p1_switch[1], event);
	else if (id == p1_switch[2].aid) return ActivateSwitch (&p1_switch[2], event);
...ad infinitatum

and again in clbkRedrawEvent

Code:

if (id == p1_switch[0].aid) return AnimateSwitch (&p1_switch[0]);
	else if (id == p1_switch[1].aid) return AnimateSwitch (&p1_switch[1]);
	else if (id == p1_switch[2].aid) return AnimateSwitch (&p1_switch[2]);
...ad infinitatum

there are also numerous references in the format of my coffee maker example above scattered all over.

meson800 · Nov 26, 2013

Ok. Because all of your functions that act on VCSWITCHSPEC use pointers, my suggestion would be to use the map to store pointers.
In general, we'll use the map to store pointers to all of the switches. Everything will modify the map; also, because the map stores pointers, you won't have to worry about the "copying" issue.

Here's all of the changes required.

First, in AAOP_Cockpit, make CreateSwitch return a pointer

Code:

class AAPO_Cockpit
{
	public:
		AAPO_Cockpit (AAPO_Vessel *vessel);
		~AAPO_Cockpit ();
		
		friend class	AAPO_Vessel;
		friend class	AAPO_Guidance;

		// Switch Functions
		[COLOR="Red"]VCSWITCHSPEC*[/COLOR] 	        CreateSwitch (int id, VECTOR3 pos, int nstate = 2, double size = 0.01, int sound = SOUND_SWITCH, UINT anim = NULL); // Initialize a VCSWITCHSPEC
--snip--

Next, in the definition

Code:

[COLOR="Red"]VCSWITCHSPEC*[/COLOR] 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"] return new VCSWITCHSPEC(switchspec);[/COLOR]
}

As a side note, to prevent memory leaks, every "new" statement must have a corresponding "delete" statement. More about that later

Instead of using your arrays of structs, we'll use the map.

Code:

[COLOR="Red"]#include <map>[/COLOR]
// ==============================================================
// Cockpit class interface
// ==============================================================
class LM_Cockpit: public AAPO_Cockpit
{
	friend class AAPO_LM;

	public:
		LM_Cockpit (AAPO_LM *vessel);
		~LM_Cockpit ();

		bool	LoadVirtualCockpit (int id);							// Process cockpit mode requests 
		bool	MouseEvent (int id, int event, VECTOR3 &p);				// Process cockpit mouse event notifications
		bool	RedrawEvent (int id, int event, SURFHANDLE surf);		// Process cockpit redraw event notifications 
		void	SaveState (FILEHANDLE scn);								// Write satus to scenario file
		bool	ParseScenarioLine (char *line);							// Read scenario lines

		// Cockpit switches
		[COLOR="Red"]map<int,VCSWITCHSPEC*> vcSwitchList;[/COLOR]

Now, in the constructor, we need to push the switches into the map.

Code:

// Initialize panel 1 switches
	for (int i = 0; i < P1_SWITCH_COUNT; i++)
	{
		int id = P1_SWITCH_AID + i;

		switch (i)
		{
		case 3: case 15: case 16: case 18: // Three-position switches
			[COLOR="Red"]vcSwitchList[id][/COLOR] = CreateSwitch (id, P1_SWITCH_POS[i], 3);
			break;

		default: // basic 2-position toggles
			[COLOR="Red"]vcSwitchList[id][/COLOR] = CreateSwitch (id, P1_SWITCH_POS[i]);
			break;
		}
	}

Now because the arrays don't exist, you have to change the LM_SwitchDefs
Because the map keeps track of the struct's by id, the #define only needs to include the id.

Example LM_SwitchDefs:

Code:

#define SWITCH_COFFEEMAKER		12
#define SWITCH_GEARDEPLOY		13
#define SWITCH_SWEETBARRELROLL		14

Now, as the #define does not include ".state", your example checking code would be

Code:

if (([COLOR="Red"]vc->vcSwitchList[SWITCH_GEARDEPLOY]->state[/COLOR] == ON) && (!GearDeployed)) DeployLandingGear ();

Now, the magic happens. Instead of the long if else chains, all you have to do is replace

Code:

else if (id == p2_switch[8].aid) return ActivateSwitch (&p2_switch[8], event);
	else if (id == p2_switch[9].aid) return ActivateSwitch (&p2_switch[9], event);
	else if (id == p2_switch[10].aid) return ActivateSwitch (&p2_switch[10], event);
	else if (id == p2_switch[11].aid) return ActivateSwitch (&p2_switch[11], event);

with

Code:

ActivateSwitch(vcSwitchList[id],event);

That's it. :thumbup:

Now, to clean up the "new" statement, you have to add the following to the deconstructor.

Code:

LM_Cockpit::~LM_Cockpit()
{
    while (vcSwitchList.size() != 0)
    {
        delete vcSwitchList[0];
        vcSwitchList.erase(0);
    }
}

The delete removes the object at the pointed location, whereas the erase removes the (now useless, as it points to nothing) pointer from the map.

Hopefully that made sense. Please ask if something isn't clear, as I sometimes don't explain stuff well.
Glad I could help! :tiphat:

---------- Post added at 10:20 PM ---------- Previous post was at 10:16 PM ----------

Tomorrow, I'll look over what you added to the post, but not tonight

You should be able to extend the above to cover it, though.

Hlynkacg · Nov 26, 2013

Ok, took a little bit of time to go back through the code and fix everything that the change in format broke but I've got it up and running now :woohoo:

I'm actually considering making vcSwitchList a member of AAPO_Cockpit and changing my switch functions to take the id rather than the full pointer as an argument. coupled with some minor modifications to LM_SwitchDefs...

Code:

if (vc->vcSwitchList[SWITCH_GEARDEPLOY]->state == ON)...

becomes

Code:

if (vc->GetSwitchState (GEARDEPLOY) == ON)...

it's a little shorter, cleaner, and it allows me to make vcSwitchList a protected class variable.

One glitch that I've run into though is that your erasure function in the Destructor is causing hangs and CTDs. I suspect that the problem is that the switch ids are not in strict numerical sequence and that as a result the function is trying to delete things that are not there.

how would i go about adding a guard to prevent this?

in psuedo-code...

Code:

if (SlotFilled) delete vcSwitchList[id]
else skip to next id;

meson800 · Nov 26, 2013

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

Code:

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

Orbiter 2024 has been released!

C++ Question Populating a list of functions.

Aspiring rocket scientist

Dread Lord of the Idiots

Not funny anymore

Aspiring rocket scientist

Addon Developer

Not funny anymore

Aspiring rocket scientist

Dread Lord of the Idiots

Aspiring rocket scientist

Addon Developer

Aspiring rocket scientist

Addon Developer

Aspiring rocket scientist

Addon Developer

Aspiring rocket scientist

Addon Developer

Aspiring rocket scientist

Addon Developer

Aspiring rocket scientist

Addon Developer

Similar threads