combat odds help missing for all terrain unit

[davidlallen]

In Dune Wars, Maniac has written some code to allow units with DOMAIN_SEA and the flag bCanMoveAllTerrain. Think of them like galleys or triremes that can travel on land. They can attack land units. When a regular unit is selected and I hold down the right mouse button over an enemy unit, I get a combat odds hover help. My problem is that when I select an all terrain unit and hold down the right mouse button over the enemy unit, no combat odds are displayed.

This mod is based on top of RevDCM and BUG. I suspect something in the combat odds routine decides early on that a DOMAIN_SEA unit cannot attack a DOMAIN_LAND, without checking bCanMoveAllTerrain. So it exits the routine with no display. But when I actually execute the attack, that works fine.

Any suggestions about where to add the missing check for bCanMoveAllTerrain?

 

[Maniac]

bool CvGameTextMgr::setCombatPlotHelp(CvWStringBuffer &szString, CvPlot* pPlot)

...

	case DOMAIN_LAND:
		bValid = (!(pPlot->isWater()) || pPlot->isCity());
		if (pAttacker != NULL) // Planetfall Maniac
		{
			//if (pAttacker->canMoveAllTerrain())
			if (pPlot->isValidDomainForLocation(*pAttacker))
			{
				bValid = true;
			}
		}
		break;

Edit: Woops, misread the post. Anyway, something like this will also work for the DOMAIN_SEA case.

 

[Thomas SG]

Search for DOMAIN_LAND!

You will find it in CvGameTextMgr. And that's where the problem is. Same also fpr others too. Like DOMAIN_LAND attacking sea.

bool CvGameTextMgr::setCombatPlotHelp(CvWStringBuffer &szString, CvPlot* pPlot)
{
	PROFILE_FUNC();

	CvUnit* pAttacker;
	CvUnit* pDefender;
	CvWString szTempBuffer;
	CvWString szOffenseOdds;
	CvWString szDefenseOdds;
	bool bValid;
	int iModifier;

	if (gDLL->getInterfaceIFace()->getLengthSelectionList() == 0)
	{
		return false;
	}

	bValid = false;

	switch (gDLL->getInterfaceIFace()->getSelectionList()->getDomainType())
	{
	case DOMAIN_SEA:
		bValid = pPlot->isWater();
		break;

	case DOMAIN_AIR:
		bValid = true;
		break;

	case DOMAIN_LAND:
		bValid = !(pPlot->isWater());
		break;
   .
   .
   .

 

[killmeplease]

hi!
where can I get all-terrain unit sources?

sorry for offtopic

 

[davidlallen]

@ Maniac, Thomas: thank you for the pointer, I will try it out.

@ killme: What we have tried to do is very specific to Dune Wars, it may not work for everybody. In particular it is *not* a magic solution for land transport. It is just the ability for ocean transports to pick up and drop off on land. The code is not separated out. Maniac wrote the original code inside his planetfall mod. Koma13 has incorporated it into dune wars and I have made additional changes inside dune wars to prevent land units inside transports on land, from taking most actions. Both mods include the sources, but I am not sure how you would separate out the changes from planetfall. For dune wars, you could separate out most changes by searching for "koma13" and "davidlallen" comments; we have not made many other changes inside the source.

 

[killmeplease]

Both mods include the sources, but I am not sure how you would separate out the changes from planetfall. For dune wars, you could separate out most changes by searching for "koma13" and "davidlallen" comments; we have not made many other changes inside the source.

Thanks for the reference! I will try to study Dune wars code

 

[davidlallen]

I have modified CvGameTextMgr::setCombatPlotHelp() as suggested, and it is working. Thanks!

 

[Maniac]

but I am not sure how you would separate out the changes from planetfall.

Search for canMoveAllTerrain

 

[davidlallen]

So, I tested the change locally for some cases, it worked, and I put it into the latest big release of Dune Wars. Sadly it completely does not work for a common case.

Please see the original function CvGameTextMgr::setCombatPlotHelp(). I have modified the beginning to check for all terrain units, before giving up due to domains. Here is the modified code.

	if (gDLL->getInterfaceIFace()->getLengthSelectionList() == 0)
	{
		return false;
	}
	// davidlallen: allterrain action start
	// moved from below so that switch...getDomainType can use pAttacker
	int iOdds;
	pAttacker = gDLL->getInterfaceIFace()->getSelectionList()->AI_getBestGroupAttacker(pPlot, false, iOdds);
	// davidlallen: allterrain action end

	bValid = false;

	switch (gDLL->getInterfaceIFace()->getSelectionList()->getDomainType())
	{
	case DOMAIN_SEA:
		// davidlallen: allterrain action next line, add cMAT check
		bValid = (pPlot->isWater()) || (pAttacker->canMoveAllTerrain());
		break;

	case DOMAIN_AIR:
		bValid = true;
		break;

	case DOMAIN_LAND:
		// davidlallen: allterrain action next line, add cMAT check
		bValid = (!(pPlot->isWater())) || (pAttacker->canMoveAllTerrain());
		break;

	case DOMAIN_IMMOBILE:
		break;

	default:
		FAssert(false);
		break;
	}

	if (!bValid)
	{
		return false;
	}

	if (pAttacker == NULL)
	{
		pAttacker = gDLL->getInterfaceIFace()->getSelectionList()->AI_getBestGroupAttacker(pPlot, false, iOdds, true);
	}

As you can see from the comment, the intention is to move the call to set pAttacker up sooner, and then check pAttacker for canMoveAllTerrain. However, in retrospect this code is clearly not correct since pAttacker may be passed in or null, and the call to getBestGroupAttacker may also return null. So using pAttacker->canMoveAllTerrain is clearly not advisable. In the case of "no valid attacker", this causes a null reference and the result is an assert within the game.

But, what is a better way to fix this? I can scatter null checks all around the code I added. Is there a better approach?

 

[EmperorFool]

It's picking the domain to check based on the selection list. Since you can only select units of one domain at a time (all land or all sea or all air), this works fine. However, I think you want to check if any unit in the selection list can move all terrain, correct?

For example, if you select one all-terrain unit and two normal land units and move to attack a sea stack, should it show the odds for the all-terrain unit? If so, you probably need to modify AI_getBestGroupAttacker() to take this into account, considering only the all-terrain units when necessary.

As for checking for NULLs, I would instead set up a local boolean variable, bAllTerrain, that you set to true if the selection list contains any all-terrain units before the switch. This allows you to have a single NULL check.

	if (gDLL->getInterfaceIFace()->getLengthSelectionList() == 0)
	{
		return false;
	}
	bValid = false;
	// davidlallen: allterrain action start
	[B]bool bAllTerrain = gDLL->getInterfaceIFace()->getSelectionList()->containsAllTerrainUnit();[/B]
	// davidlallen: allterrain action end

	switch (gDLL->getInterfaceIFace()->getSelectionList()->getDomainType())
	{
	case DOMAIN_SEA:
		// davidlallen: allterrain action next line, add cMAT check
		bValid = (pPlot->isWater()) || ([B]bAllTerrain[/B]);
		break;

	case DOMAIN_AIR:
		bValid = true;
		break;

	case DOMAIN_LAND:
		// davidlallen: allterrain action next line, add cMAT check
		bValid = (!(pPlot->isWater())) || ([B]bAllTerrain[/B]);
		break;

	case DOMAIN_IMMOBILE:
		break;

	default:
		FAssert(false);
		break;
	}

	if (!bValid)
	{
		return false;
	}

	if (pAttacker == NULL)
	{
		pAttacker = gDLL->getInterfaceIFace()->getSelectionList()->AI_getBestGroupAttacker(pPlot, false, iOdds, true);
	}

This requires that you add a containsAllTerrainUnit() function to CvSelectionList that loops over all the units in the list and returns true as soon as it finds an all-terrain unit; otherwise it returns false.

 

[davidlallen]

For example, if you select one all-terrain unit and two normal land units and move to attack a sea stack, should it show the odds for the all-terrain unit?

I have not tried selecting and attacking with various mixed stacks. But I do not think this is an issue. In vanilla, if some selected units can attack and some cannot, for whatever reason, then the attack is not allowed. For example, you have a stack of two attackers. You select *one* and then attack. Now you select *both* and attack again. The game will not let you, because some of the units in the stack cannot attack.

So in your example, the game prevents you from attacking already, and I think this behavior is consistent.

 

[EmperorFool]

That's fine. Change the new function to CvSelectionList::isAllTerrain() and return true only if every single unit in the list is all-terrain.