C++ Question int to boolean

[Hlynkacg]

I'm having a problem with reading boolean states from a scenario file and need someone to check my work.

Is there any reason that the following shouldn't work?

void CSM::clbkLoadStateEx (FILEHANDLE scn, void *status)
{
	int i = 0;
	char *cbuf; 

	while (oapiReadScenario_nextline (scn, cbuf)) 
	{

		if (!_strnicmp (cbuf, "LES_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i", &LES_attached);	
		}

		if (!_strnicmp (cbuf, "SLA_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i", &SLA_attached);	
		}
...

LES_attached and SLA_attached are booleans

I've already confirmed that the 1s and 0s are being read they're just not making the translation into "true" and "false". I suspect that I've missed something obvious but have no clue what.

 

[Bibi Uncle]

Maybe you can try to cast it to a bool pointer. It should look like this:

void CSM::clbkLoadStateEx (FILEHANDLE scn, void *status)
{
	int i = 0;
	char *cbuf; 

	while (oapiReadScenario_nextline (scn, cbuf)) 
	{

		if (!_strnicmp (cbuf, "LES_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i", (bool*)&LES_attached);	
		}

		if (!_strnicmp (cbuf, "SLA_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i", (bool*)&SLA_attached);	
		}
//...
}

I don't know if this is the problem. The working code that I have that reads a bool from a scenario file actually uses a void pointer. That's why I explicitly casted it to a bool pointer.

Edit:
According to this thread, my code should not work. However, I can't find any issue, and my machine uses 4-bit integers and 1-bit booleans. Weird...

 

[csanders]

sscanf (cbuf+12, "%i", (bool*)&SLA_attached)

I think this will write 4 bytes into the address of SLA_attached

try

int temp;
sscanf (cbuf+12, "%i", &temp);
SLA_attached = temp;

---------- Post added at 05:26 AM ---------- Previous post was at 05:23 AM ----------

According to this thread, my code should not work. However, I can't find any issue, and my machine uses 4-bit integers and 1-bit booleans. Weird...

bytes, not bits, anyway...

It should "work", because the first byte will write to the right place. The next 3 however, are undefined. Should result in really strange behavior. So checking the first byte will work, whatever used the next 3 will be bonkers.

EXAMPLE:

someFunction(char *cbuf)
{
  BOOL SLA_attached = true;
  BOOL SLA_one = true;
  BOOL SLA_two = true;
  BOOL SLA_three = true;

  if (!_strnicmp (cbuf, "SLA_ATTACHED", 12))
  {
    sscanf (cbuf+12, "%i", (bool*)&SLA_attached);	
  }
  //if sscanf is reading a 0 or 1, then SLA_ATTACHED's value will be correct
  //SLA_one, SLA_two, and SLA_three should all now inexplicably be false
  //(assuming BOOL are 1 byte)
}

 

[Enjo]

Forget about insecure casting. This example is cleaner and less error prone:

void LaunchMFD::ReadStatus (FILEHANDLE scn)
{
    char * line; 
    while (oapiReadScenario_nextline (scn, line))
    {
        std::istringstream ss;
        ss.str(line);
        std::string id;
        if ( ss >> id )
        {
            if ( id == "BOOLALT" )      ss >> m_data->m_ManualAlt;
            else if ( id == "BOOLENG" ) ss >> m_data->CutEngines;
            else if ( id == "HOV" )     ss >> m_data->half_ov_reached;
            else if ( id == "BOOLLNC" ) ss >> m_data->launched;
            else if ( id == "BOOLTGT" ) ss >> m_data->tgt_set;
            else if ( id == "BOOLHUD" ) ss >> m_data->hud;
            else if ( id == "BOOLPTC" ) ss >> m_data->drawPitchError;
            else if ( id == "BOOLCOR" ) ss >> m_data->m_useOffplaneCorrector;
            else if ( id == "BOOLDAS" ) ss >> m_data->m_daSynchroOrbit; 
            else if ( id == "GCCONT" )  ss >> m_data->m_greatCircle.m_continuous;
            else if ( id == "INCL" )    ss >> m_data->GetTgtParam().incl;
            else if ( id == "IFACT" )   ss >> m_data->InclinationFactor;
            else if ( id == "AFACT" )   ss >> m_data->AltitudeFactor;
            else if ( id == "ALOCK" )   ss >> m_data->AzimuthLock;
            else if ( id == "PEA" )     ss >> m_data->PeA;
            else if ( id == "APA" )     ss >> m_data->ApA; 
            else if ( id == "GCSTEP" ) 
            { 
                double gcStep; 
                if ( ss >> gcStep && gcStep >= 0) 
                    m_data->m_greatCircle.SetPlotPrecision(gcStep / 1000.0); 
            } 
            else if ( id == "PVIEW" )
            {
                int view;
                ss >> view;
                m_data->pageView = (View)view;
            }
            else if ( id == "TGT" )
            {
                std::string tgt;
                ss >> tgt;
                std::string replaced = CharManipulations().replace(tgt, ">", " ");
                m_data->SetTargetStr( replaced );
            }
        }
    }
}

 

[dbeachy1]

A bool is one byte (8 bits) -- csanders is correct that reading an integer (which writes four bytes to the pointer) into a boolean will overwrite memory. So you could do something similar to what csanders suggested, although you should make sure to initialize 'temp' first because its value will be unchanged if the sscanf call fails to parse the integer value for any reason:

int temp = 0;  // must initialize this in case sscanf fails to parse the integer
sscanf (cbuf+12, "%i", &temp);  // writes 4 bytes to temp -- no need to typecast the pointer since it's just a void * anyway
SLA_attached = (temp != 0);     // to be tidy, a bool's value should either be true (1) or false (0)

If you didn't initialize temp first and sscanf failed the result assigned to SLA_attached would be undefined.

BTW if you need to reuse 'temp' in your code you could enclose the block in braces so 'temp' could be reused each time without relying on it being declared somewhere earlier; e.g.,

{   
    int temp = 0;  // must initialize this in case sscanf fails to parse the integer; default to 0 (false) if the parse fails.  
    sscanf (cbuf+12, "%i", &temp);  // writes 4 bytes to temp -- no need to typecast the pointer since it's just a void * anyway
    SLA_attached = (temp != 0);     // to be tidy, a bool's value should either be true (1) or false (0)
}

Of course, the parse shouldn't ever fail if the scenario file was written out correctly and hasn't been modified by the user, but it's good to code defensively when parsing scenario files.

EDIT:
Ninja'd by Enjo. :ninja:

 

[ADSWNJ]

A good old C vs C++ discussion. Enjo's version is new school, and lots would prefer it, but I think the sscanf version is more intuitive ��. Old C habits die hard.

Also note that Visual Studio would much prefer you to use sscanf_s instead of sscanf, unless you order it to be quiet. Nothing wrong with getting into the habit of using the _s variants, and it only asks you for a buffer size param, so it's very easy.

 

[Urwumpe]

I prefer Enjos... but do the C style code more than often. :lol:

 

[marooder86]

If you don't want to mangling with casting which I think would be terrible and unnecessary solution, or use Enjo's method, this should do the trick for you.

void CSM::clbkLoadStateEx (FILEHANDLE scn, void *status)
{
	int i = 0;
        [COLOR="Red"]int tempComp = 0;[/COLOR]
	char *cbuf; 

	while (oapiReadScenario_nextline (scn, cbuf)) 
	{

		if (!_strnicmp (cbuf, "LES_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i,[COLOR="Red"]&tempComp[/COLOR]);	
                        [COLOR="Red"]if(1 == tempComp)
                           LES_attached = true;
                        else
                           LES_attached = false; [/COLOR]
		}

		if (!_strnicmp (cbuf, "SLA_ATTACHED", 12))
		{
			sscanf (cbuf+12, "%i", [COLOR="Red"]&tempComp[/COLOR]);
                        [COLOR="Red"]if(1 == tempComp)
                           SLA_attached = true;
                        else
                           SLA_attached = false;[/COLOR]
		}
...

 

[orb]

                        [COLOR="Red"]if(1 == tempComp)
                           LES_attached = true;
                        else
                           LES_attached = false; [/COLOR]

No need for so many lines with condition checking. Dbeachy1 has provided the same solution in just one line, but compared to 0 instead (and comparing values gives you boolean, without need for any casting).

 

[marooder86]

No need for so many lines with condition checking. Dbeachy1 has provided the same solution in just one line, but compared to 0 instead (and comparing values gives you boolean, without need for any casting).

You're right I haven't noticed it, and yes it's much better and elegant solution although execution speed in scenario loading function isn't that crutial so I thought those few more lines won't do much harm and IMHO will bo more describeful in explaining that there is no need to try to put an int into boolean by force.

 

[Hlynkacg]

Thank's all.

dbeachy1's code did the trick. :thumbup:

	while (oapiReadScenario_nextline (scn, cbuf)) 
	{
		// Check for presence of Abort Tower  
		if (!_strnicmp (cbuf, "LES_ATTACHED", 12))// *Boolean parser courtesy of dbeachy1*
		{
			int temp = 0;					// must initialize temp in case sscanf fails to parse the integer; default to 0 (false) if the parse fails.  
			sscanf (cbuf+12, "%i", &temp);  // writes 4 bytes to temp -- no need to typecast the pointer since it's just a void * anyway
			LES_attached = (temp != 0);     // to be tidy, a bool's value should either be true (1) or false (0)
		}...

That said I've run into another issue that may be related. Or rather an old issue that has resurfaced. I've created an independent subclass to handle my cockpit panels and displays. I based on its structure & syntax on "PlBayOp" class in the stock Atlantis but the values read from the scenario file are not being written to the appropriate targets.

in my vessel's source code...

		...else 
		{
			if (vc->ParseScenarioLine (cbuf)) continue; // offer the line to Cockpit parser
            else ParseScenarioLineEx (cbuf, status);	// pass unrecognised options to Orbiter's generic parser
        }
	} // End "while (oapiReadScenario_nextline (scn, cbuf))"

} // End "clbkLoadStateEx"

the cockpit class' parser...

// --------------------------------------------------------------
// Read status from scenario file
// --------------------------------------------------------------
bool LM_Cockpit::ParseScenarioLine (char *line)
{
	int i = 0;
	
	// Load Panel 1 switch positions
	if (!_strnicmp (line, "PANEL1_SWITCHES", 15)) 
	{					  
		line += 16;
		for (i = 0; (i < P1_NSWITCH) && (*line); i++)
		{
			int state = 0;
			state = (*line++ - '0');
			SetSwitchState (&P1_Switch[i], state); 
		}
		return true;
	}
...

the contents of "SetSwitchState" function

bool Cockpit::SetSwitchState (enum switchstate* toggle, int newstate)
{
	int state = *toggle;

	if (state != newstate)
	{
		*toggle = (switchstate) newstate;
		return true;
	}
	else return false;
}

 

[csanders]

instead of:
int state = 0;
state = (*line++ - '0');

try:
int state = (*line) - '0';
++line;

More clear intent and operation. I think *line++ might increment the character where line is pointing, not the pointer. But I could be wrong there. Order of operator is arbitrary, so it's best to clearly define it with parenthesis.

 

[Hlynkacg]

instead of:
int state = 0;
state = (*line++ - '0');

try:
int state = (*line) - '0';
++line;

More clear intent and operation. I think *line++ might increment the character where line is pointing, not the pointer. But I could be wrong there. Order of operator is arbitrary, so it's best to clearly define it with parenthesis.

Just tried it, that's not it.

The issue appears to be in the cast from int to the enumerator "switchstate".

For some reason it seems to work when placed within the parent vessel's class but not after it's been broken off into it's own class.

 

[francisdrake]

A very much old school C-style would be replacing

sscanf (cbuf+12, "%i", &tempComp);
if(1 == tempComp)
SLA_attached = true;
else
SLA_attached = false;

written as:
sscanf (cbuf+12, "%i", &tempComp);
SLA_attached = (tempComp ? true : false);

It uses the preprocessor and the fact that everything not 0 is true.
But I agree the C++ style is more elegant

 

[Hlynkacg]

Already got that part working, now the problem is elsewhere.

 

[marooder86]

What type of objects you store in P1_Switch table?

 

[Hlynkacg]

P1 stands for "Panel 1". "Px_Switch[]" are set a series of enumerators that stores the positions of cockpit switches.

 

[orb]

That said I've run into another issue that may be related. Or rather an old issue that has resurfaced. I've created an independent subclass to handle my cockpit panels and displays. I based on its structure & syntax on "PlBayOp" class in the stock Atlantis but the values read from the scenario file are not being written to the appropriate targets.

I can't reproduce your problem with reading scenario in subclassed "virtual cockpit".

I took parts of your code for testing and the panel switch has the correct value after initializing the vessel. Here's my test code:


TheVessel.h:

#pragma once

class TheVirtualCockpit;


class TheVessel : public VESSEL3 {
public:
	TheVessel (OBJHANDLE hVessel, int flightModel);
	~TheVessel ();
	void clbkLoadStateEx (FILEHANDLE scn, void * vs);
	void clbkPostCreation ();

private:
	TheVirtualCockpit * vc;
};

TheVirtualCockpit.h:

#pragma once

#define P1_NSWITCH 5

enum SwitchStates {POS0, POS1, POS2, POS3, POS4, POS5, POS6, POS7};

class TheVessel;


class TheVirtualCockpit {
	friend class TheVessel;

public:
	TheVirtualCockpit ();
	~TheVirtualCockpit () {};

protected:
	bool ParseScenarioLine (char * line);

private:
	static bool SetSwitchState (SwitchStates * toggle, int newstate);

	SwitchStates P1_Switch [P1_NSWITCH];
};

TheVessel.cpp:

#define STRICT

#include "orbitersdk.h"
#include "TheVessel.h"
#include "TheVirtualCockpit.h"


TheVessel::TheVessel (OBJHANDLE hVessel, int flightModel) : VESSEL3 (hVessel, flightModel) {
	vc = new TheVirtualCockpit ();
}


TheVessel::~TheVessel () {
	delete vc;
}


void TheVessel::clbkPostCreation () {
	int i = (int) vc->P1_Switch [2];
[highlight]	// At this point 'i' has correct value read for the switch
	// from the scenario file (for any of the switches,
	// not only for the 3rd which is in example).[/highlight]
}



void TheVessel::clbkLoadStateEx (FILEHANDLE scn, void *vs) {
	char * cbuf;
	while (oapiReadScenario_nextline (scn, cbuf))  {
		if (vc->ParseScenarioLine (cbuf)) continue;
		ParseScenarioLineEx (cbuf, vs);
	}
}

TheVirtualCockpit.cpp:

#define STRICT

#include "orbitersdk.h"
#include "TheVirtualCockpit.h"


TheVirtualCockpit::TheVirtualCockpit () {
	for (int i = 0; i < P1_NSWITCH; P1_Switch [i++] = POS0);
}


bool TheVirtualCockpit::ParseScenarioLine (char * line) {
	int i = 0;
	
	if (!_strnicmp (line, "PANEL1_SWITCHES", 15))  {					  
		line += 16;
		for (i = 0; (i < P1_NSWITCH) && (*line); ++i) {
			int state = 0;
			state = (*line++ - '0');
			SetSwitchState (&P1_Switch [i], state); 
		}
		return true;
	}
	return false;
}


bool TheVirtualCockpit::SetSwitchState (SwitchStates * toggle, int newstate) {
	int state = *toggle;

	if (state != newstate) {
		*toggle = (SwitchStates) newstate;
		return true;
	}
	return false;
}

main.cpp:

#define STRICT
#define ORBITER_MODULE

#include "orbitersdk.h"
#include "TheVessel.h"


DLLCLBK VESSEL * ovcInit (OBJHANDLE hVessel, int flightModel) {
	return new TheVessel (hVessel, flightModel);
}


DLLCLBK void ovcExit (VESSEL * vessel) {
	if (vessel) delete (TheVessel *) vessel;
}

 

[Hlynkacg]

I can't reproduce your problem with reading scenario in subclassed "virtual cockpit".

I took parts of your code for testing and the panel switch has the correct value after initializing the vessel. Here's my test code...

Well I feel like an idiot.

While scrolling back through my code to add the "int i = (int) vc->P1_Switch [2];" line to clbkPostCreation, I noticed that while copy/pasting some generic functions from my LM to my CSM* that I had accidentally initialized the VC twice. :facepalm:

I deleted the second instance and now it's working.

Thanks for the help everyone :tiphat: and

ETA:
*Both are part of a generic "AAPvessel" class that I've built and reuse many of the same functions and basic code structure.

 

[orb]

While scrolling back through my code to add the "int i = (int) vc->P1_Switch [2];" line to clbkPostCreation, I noticed that while copy/pasting some generic functions from my LM to my CSM* that I had accidentally initialized the VC twice. :facepalm:

That's why I asked you either for you running it through the debugger, or for posting the full code for us, when you first got that problem.

Posting the code would allow us to find the source of issue, and running it through debugger would allow you to find where the issue is much faster, too.

Anyway, one issue fewer. :thumbup: