C++ Question

[General Tso]

I'm trying to something that should be very simple, but I can't get it to work. I'm guessing that my problem is caused by some aspect of C++ that I don't understand. I'm trying to add a new variable to the CvGame.cpp file then access it in the usual fashion, but I keep getting the following error when I try to compile the SDK.

1>CvGameInterface.cpp(906) : error C2662: 'CvGame::setSelectedUnitPlot' : cannot convert 'this' pointer from 'const CvGame' to 'CvGame &'
1>        Conversion loses qualifiers

Here is what I have done to make the error appear.

I added this to CvGame.h

int m_iSelectedUnitPlot;

I added these to CvGame.cpp

int m_iSelectedUnitPlot = -1;

and

void CvGame::setSelectedUnitPlot(CvUnit* pUnit)
{
	if (pUnit != NULL)
	{
		m_iSelectedUnitPlot = GC.getMapINLINE().plotNum(pUnit->getX(), pUnit->getY());
	}
}

CvPlot* CvGame::getSelectedUnitPlot() const
{
	return GC.getMapINLINE().plotByIndex(m_iSelectedUnitPlot);
}

When I call setSelectedUnitPlot from CvGame.ccp or CvGameInterface.cpp I get the error mentioned above. (I haven't tried calling from anywhere else.)

I created m_iSelectedUnitPlot as a protected variable and it is initialized in CvGame::CvGame.

setSelectedUnitPlot and getSelectedUnitPlot are created as public functions.

Anybody have any ideas?

 

[phungus420]

You need to declare setSelectedUnitPlot in CvGame.h

 

[EmperorFool]

What is the code where this error occurs? It sounds like you are trying to call a non-const function from a const function on the same object. A const function is not allowed to modify the object on which it is called. Most setFoo() functions do not qualify by their very nature while getFoo() functions typically do.

1. Is this the case?
2. Can you make your new function const? It doesn't look like it.
3. Can you remove the const from the calling function? That may have rippling effects to functions that call it.

 

[Afforess]

Just interjecting here, mainly because this is an error I've experience too, and I understood why I received the error (the difference between const and const-omitted is fairly clear), but why is there a difference between the two? Why not omit all the const's? Wouldn't that always ensure compatibility? I'm sure there is a good reason, I just don't know it.

 

[EmperorFool]

From a design point of view, it can be helpful to understand what functions will modify an object and which will not. It is part of the function's contract: what it is designed to do and how it does it.

Another reason is that it allows the compiler to make better optimization decisions. Say you have a loop that uses a member field of an object, and inside the loop it calls a function on that object. If the function called isn't const, it's possible for it to modify that member variable. This will force the compiler to re-acquire the value from the object each time through the loop. If the function is marked const, however, the compiler can load the value into a register and use it from there.

 

[General Tso]

Thanks for the suggestions, but it seems that none of them apply to this problem.

I understand the function of const for the most part. But I still don't understand why this is happening. There is original game code that appears to be doing the same thing that I am doing and it works OK. I even tried placing one of the original non-const game functions (setTeamRank) in place of my function call - it gave no errors when compiling. Here's some more info - maybe it will help show what I'm doing wrong.

Here's the declarations for the functions.

void setSelectedUnitPlot(CvUnit* pUnit);
CvPlot* getSelectedUnitPlot() const;

setSelectedUnitPlot is being called from cyclePlotUnits (within CvGameInterface.cpp) It's marked in red near the bottom of this code block.

bool CvGame::cyclePlotUnits(CvPlot* pPlot, bool bForward, bool bAuto, int iCount) const
{
	CLLNode<IDInfo>* pUnitNode;
	CvUnit* pSelectedUnit;
	CvUnit* pLoopUnit = NULL;

	FAssertMsg(iCount >= -1, "iCount expected to be >= -1");

	CvGlobals::Echo("CvGame::cyclePlotUnits");

	if (iCount == -1)
	{
		pUnitNode = pPlot->headUnitNode();

		while (pUnitNode != NULL)
		{
			pLoopUnit = ::getUnit(pUnitNode->m_data);

			if (pLoopUnit->IsSelected())
			{
				break;
			}

			pUnitNode = pPlot->nextUnitNode(pUnitNode);
		}
	}
	else
	{
		pUnitNode = pPlot->headUnitNode();

		while (pUnitNode != NULL)
		{
			pLoopUnit = ::getUnit(pUnitNode->m_data);

			if ((iCount - 1) == 0)
			{
				break;
			}

			if (iCount > 0)
			{
				iCount--;
			}

			pUnitNode = pPlot->nextUnitNode(pUnitNode);
		}

		if (pUnitNode == NULL)
		{
			pUnitNode = pPlot->tailUnitNode();

			if (pUnitNode != NULL)
			{
				pLoopUnit = ::getUnit(pUnitNode->m_data);
			}
		}
	}

	if (pUnitNode != NULL)
	{
		pSelectedUnit = pLoopUnit;

		while (true)
		{
			if (bForward)
			{
				pUnitNode = pPlot->nextUnitNode(pUnitNode);
				if (pUnitNode == NULL)
				{
					pUnitNode = pPlot->headUnitNode();
				}
			}
			else
			{
				pUnitNode = pPlot->prevUnitNode(pUnitNode);
				if (pUnitNode == NULL)
				{
					pUnitNode = pPlot->tailUnitNode();
				}
			}

			pLoopUnit = ::getUnit(pUnitNode->m_data);

			if (iCount == -1)
			{
				if (pLoopUnit == pSelectedUnit)
				{
					break;
				}
			}

			if (pLoopUnit->getOwnerINLINE() == getActivePlayer())
			{
				if (bAuto)
				{
					if (pLoopUnit->getGroup()->readyToSelect())
					{
						gDLL->getInterfaceIFace()->selectUnit(pLoopUnit, true);
[COLOR="DarkRed"]						setSelectedUnitPlot(pLoopUnit);[/COLOR]
						return true;
					}
				}
				else
				{
					gDLL->getInterfaceIFace()->insertIntoSelectionList(pLoopUnit, true, false);
					return true;
				}
			}

			if (pLoopUnit == pSelectedUnit)
			{
				break;
			}
		}
	}

	return false;
}

I don't think it has anything to do with the problem, but getSelectedUnitPlot is being called from updateSelectionList (within CvGameInterface.cpp) like this.

void CvGame::updateSelectionList()
{
	CvUnit* pHeadSelectedUnit;

	if (GET_PLAYER(getActivePlayer()).isOption(PLAYEROPTION_NO_UNIT_CYCLING))
	{
		return;
	}

	pHeadSelectedUnit = gDLL->getInterfaceIFace()->getHeadSelectedUnit();

	if ((pHeadSelectedUnit == NULL) || !(pHeadSelectedUnit->getGroup()->readyToSelect(true)))
	{
		if ((gDLL->getInterfaceIFace()->getOriginalPlot() == NULL) || !(cyclePlotUnits(gDLL->getInterfaceIFace()->getOriginalPlot(), true, true, gDLL->getInterfaceIFace()->getOriginalPlotCount())))
		{
			if ((gDLL->getInterfaceIFace()->getSelectionPlot() == NULL) || !(cyclePlotUnits(gDLL->getInterfaceIFace()->getSelectionPlot(), true, true)))
			{
				[COLOR="DarkRed"]CvPlot* pPlot = getSelectedUnitPlot();
				if ((pPlot == NULL) || !(cyclePlotUnits(pPlot, true, true)))[/COLOR]				
                                {
					cycleSelectionGroups(true);
				}
			}
		}

		pHeadSelectedUnit = gDLL->getInterfaceIFace()->getHeadSelectedUnit();

		if (pHeadSelectedUnit != NULL)
		{
			if (!(pHeadSelectedUnit->getGroup()->readyToSelect()))
			{
				gDLL->getInterfaceIFace()->clearSelectionList();
			}
		}
	}
}

Any ideas what I'm doing wrong?

 

[EmperorFool]

I still think that it's a problem with constness. The question on that page is a bit cryptic for some (operator overloading isn't used in Civ4), but the ramifications are clear: const methods cannot call non-const methods on the same object.

Can you show how you tested with that other code? Maybe your test wasn't done correctly or you were calling a method on a different object without realizing it.

BTW, StackOverflow is a great place for finding answers to tough problems like this and asking them yourself. People there cover all languages.

 

[General Tso]

I'll check out StackOver flow, thanks.

I just tried something that got rid of the compiling error.

I changed this.
setSelectedUnitPlot(pLoopUnit);

To this.
CvGame& kGame = GC.getGameINLINE();
kGame.setSelectedUnitPlot(pLoopUnit);

I don't know why it's necessary but it got rid of the error. Any ideas why it worked? For now I'm off to see if the code actually works.


Edit: Actually my question isn't why did it work, it's why is it necessary?

 

[EmperorFool]

It worked because you grabbed a non-const reference to the CvGame object. Since there is only one CvGame, you are violating the const-correctness of CvGame::cyclePlotUnits(). You could just as easily have stripped the constness from the "this" pointer to do the same thing, but you'd still be violating const-correctness.

CvGame::cyclePlotUnits() claims it will not modify the CvGame, but it does. That's what I mean by violating const-correctness. You have to decide if that's okay. Why not just remove the const from CvGame::cyclePlotUnits()? It's your own new function, right? It's name suggests to me that it will modify the CvGame.

One thing that may not be clear here: the CvGame is not what is "const." Only the reference to it is declared to be const. That means the CvGame can be altered normally, just not via that reference. In this function, "this" is the reference and it acquires const status because of the function's declaration. By calling GC.getGameINLINE(), you are getting a non-const pointer to the same object, tricking the compiler since it doesn't know it's the same object.

 

[General Tso]

One thing that may not be clear here: the CvGame is not what is "const." Only the reference to it is declared to be const. That means the CvGame can be altered normally, just not via that reference. In this function, "this" is the reference and it acquires const status because of the function's declaration. By calling GC.getGameINLINE(), you are getting a non-const pointer to the same object, tricking the compiler since it doesn't know it's the same object.

Ahh, thanks that part made it clear. I thought const only applied to the function that contains it, not fuctions called from it. I have sinced moved my function to CvGame::selectUnit. I'll try removing the const from that function and see what happens.

 

[General Tso]

Unforunately I can't remove the const statement from selectUnit because it's called from the executable and can't be changed. I guess I'll have to continue with the current method, even thought I would prefer doing it "by the book". Oh well, at least it works .