Increasing Bombardment Range

[Ekmek]

Does it look like then there is a BUG between how CanBombard and Bombard work. I'm thinking Bombard should be coded like CanBombard instead of using the IsNextTo. As it is now ArmyData:BOmbard doesnt even check if it CanBombard, instead it checks if the unit "IsNextTo"

As far as an army with different bombard range. I think it calculates as a single army each unit that can bombard. So (hypothetically) a catapult of 1 doesnt bombard but only the cannons of 2 can. Atleast how the code looks.

should it be

ORDER_RESULT ArmyData::Bombard(const MapPoint &orderPoint)
{

	MapPoint point = orderPoint;

	static CellUnitList defender;
	defender.Clear();
	g_theWorld->GetArmy(point, defender);
	sint32 i;
	
	BOOL isSpaceBombard = FALSE;
	if(point == m_pos) {
		return ORDER_RESULT_ILLEGAL;
	} else {
		if(!point.[b]CanBombard[/b](m_pos)) {
			return ORDER_RESULT_ILLEGAL;
		}
	}

[J Bytheway]

Originally posted by E
Does it look like then there is a BUG between how CanBombard and Bombard work. I'm thinking Bombard should be coded like CanBombard instead of using the IsNextTo

It's not so much a bug as an unimplemented feature, but if BombardRange is to be implemented, that is indeed what must be done.

As far as an army with different bombard range. I think it calculates as a single army each unit that can bombard. So (hypothetically) a catapult of 1 doesnt bombard but only the cannons of 2 can. Atleast how the code looks.

Yes, but as Solver pointed out, there is the "fire and forget" code lurking somewhere which first moves the army into range and then bombards, so - if this is not disabled altogether - should it move the army until something is in range, or until all bombarding units are in range? Personally I'd choose disabling it altogether, but I'm also going to have to go to bed now, so I'll leave you to search it out andfind out how it works .

[J Bytheway]

Originally posted by E
should it be

ORDER_RESULT ArmyData::Bombard(const MapPoint &orderPoint)
{

	MapPoint point = orderPoint;

	static CellUnitList defender;
	defender.Clear();
	g_theWorld->GetArmy(point, defender);
	sint32 i;
	
	BOOL isSpaceBombard = FALSE;
	if(point == m_pos) {
		return ORDER_RESULT_ILLEGAL;
	} else {
		if(!point.[b]CanBombard[/b]) (m_pos)) {
			return ORDER_RESULT_ILLEGAL;
		}
	}

Well, not exactly like that - you can't call point.CanBombard() because point is a variable of type MapPoint, whereas CanBombard is a method of the UnitData class. The calls to CanBombard should probably come further down in the code, where it's looping through all the units...

[Ekmek]

BOOL ArmyData::CanBombardTargetType(const CellUnitList & units) const
{
	sint32 i,j;
	
    for (i = m_nElements - 1; i>= 0; i--) 
	{
		for (j = 0; j < units.Num(); j++)
		{
			if ( m_array[i]->CanBombardType(units[j]) == TRUE)
				return TRUE;
		}
	}
	return FALSE;
}



BOOL ArmyData::CanBombard(const MapPoint &point) const
{
	static CellUnitList defender;
	defender.Clear();
	g_theWorld->GetArmy(point, defender);
	sint32 i;
	
	if (defender.Num() < 1)
		return false;

    for (i = m_nElements - 1; i>= 0; i--) { 
		
        if (m_array[i].CanBombard(defender)) 
			return TRUE;
	}
	return FALSE;
}

BOOL ArmyData::CanBombard() const
{
	sint32 i;
	
    for (i = m_nElements - 1; i>= 0; i--) { 
        if( m_array[i].GetDBRec()->GetCanBombard() &&
			m_array[i].CanPerformSpecialAction()) {
            return TRUE;
		}
	}
	return FALSE;

[Ekmek]

int32 numAttacks = 0;
	sint32 numAlive = m_nElements;
	BOOL out_of_fuel;

    for (i = m_nElements - 1; i>= 0; i--) { 
		if(!m_array[i].CanPerformSpecialAction())
			continue;
		
        if (m_array[i].CanBombard(defender)) { 
			if(m_array[i].Bombard(defender, FALSE)) {
				numAttacks++;
				
				
				g_director->[b]AddAttackPos[/b](m_array[i], point);
				
				AddSpecialActionUsed(m_array[i]);
				
				if(!m_array[i].GetDBRec()->GetMovementTypeAir()) {
					m_array[i].SetMovementPoints(0.0);
				} else {
					m_array[i].DeductMoveCost(k_MOVE_COMBAT_COST, out_of_fuel);

I think the AddAttackPos is where the fire and forget comes in play...

[Ekmek]

ctp2_code\gfx\spritesys\director.cpp

but I'm not sure it does what I thought...

void Director::AddAttackPos(Unit attacker, MapPoint &pos)
{
	DQActionAttackPos	*action = new DQActionAttackPos;
	DQItem				*item = new DQItem(DQITEM_ATTACKPOS, action, dh_attackpos);
	item->SetOwner(attacker.GetOwner());

	action->attackpos_attacker = attacker.GetActor();
	action->attackpos_attacker_pos = attacker.RetPos();
	action->attackpos_target_pos = pos;
	action->attackpos_soundID = attacker.GetAttackSoundID();

	m_itemQueue->AddTail(item);

	if (g_player[g_selected_item->GetVisiblePlayer()] &&
		g_player[g_selected_item->GetVisiblePlayer()]->IsVisible(pos))
		if (attacker.m_id != 0) {
			AddCombatFlash(pos);

[Flinx]

Originally posted by Solver
ArmyData:BombardCity seems of interest as it has the (unused apparently) possibilities of damaging citizens/buildings during city bombardment.

IIRC there is a value in const.txt which sets the possibility of this to 0 but I could be wrong or that was what they had wanted to implement but were not able to and the values are not used now

[Solver]

Yes, but as Solver pointed out, there is the "fire and forget" code lurking somewhere which first moves the army into range and then bombards, so - if this is not disabled altogether - should it move the army until something is in range, or until all bombarding units are in range? Personally I'd choose disabling it altogether, but I'm also going to have to go to bed now, so I'll leave you to search it out andfind out how it works

It seems obvious that the code doesn't move armies into range to bombard - instead, the same code moves armies in range to execute any order, also bioinfections, franchises, etc. So we have two options: one is to disable it for bombard orders only (requires writing extra code), second is to disable it altogether (only need to take the code out). I'm strongly in favor of the latter. It is simpler to implement, and the "order and forget" movement feauture was annoying to me nonetheless - you better move units to the targets yourself. Also, disabling that will, I guess, prevent possible bugs with ranged bombard/other orders.

For the Bombard code, how about simply this:

ORDER_RESULT ArmyData::Bombard(const MapPoint &orderPoint)
{

	MapPoint point = orderPoint;

	static CellUnitList defender;
	defender.Clear();
	g_theWorld->GetArmy(point, defender);
	sint32 i;
	
	BOOL isSpaceBombard = FALSE;
	if(point == m_pos) {
		return ORDER_RESULT_ILLEGAL;
	} else {
		if(! CanBombard(point)) {
			return ORDER_RESULT_ILLEGAL;
		}
}

So we immediately check if the army can bombard the target point... and then ArmyData::CanBombard should probably be where we check for the range properly. I really need to access the code and MSVC now...

So, do the squared distance functions work properly now for anything like that we may want to code? I find the whole squared distance instead of 'squares to move' type of algorithm annoying, because it involves extra geometrical calculations.

MapPoint::GetSquaredDistance does exactly what it says (modulo some tweaks to account for the isometric tile set and wrapping, of course...). The reason it adds 0.5 is so that diagonally adjacent squares (which have a squaredistance of 2) fit inside distance 1 (which has rsq=(1+0.5)^2=2.25).

But it would work the same if diagonally adjacent squared were distance=1, right?



EDIT: CanBombard gotta check for point, not m_pos. m_pos is where the unit ordered to bombard is located, point is where the target is.

[Solver]

        if (m_array[i].CanBombard(defender)) { 
			if(m_array[i].Bombard(defender, FALSE)) {
				numAttacks++;
				
				
				g_director->AddAttackPos(m_array[i], point);

Weird. If that's the fire and forget code, why would it be invoked specifically for bombard, not just for any order? Besides, g_director is apparently an object of the Director class, and since we have

ctp2_code\gfx\spritesys\director.cpp

and the Director::AddAttackPos does not refer to Unit or Army classes, it seems that that is merely graphical code. I can't see where it adds a move order to the army there, so I think the fire and forget is elsewhere.

[Flinx]

ASSAULT_DESTROY_BUILDING_CHANCE 0.1  # chance of an assault on a city destroying a building
BOMBARD_DESTROY_BUILDING_CHANCE 0.1  # chance of a bombardment on a city destroying a building
BOMBARD_KILL_POP_CHANCE 0.1
ASSAULT_KILL_POP_CHANCE 0.4
CAPTURE_KILL_POP_CHANCE 1.0

[Flinx]

Originally posted by Solver
It seems obvious that the code doesn't move armies into range to bombard - instead, the same code moves armies in range to execute any order, also bioinfections, franchises, etc. So we have two options: one is to disable it for bombard orders only (requires writing extra code), second is to disable it altogether (only need to take the code out). I'm strongly in favor of the latter. It is simpler to implement, and the "order and forget" movement feauture was annoying to me nonetheless - you better move units to the targets yourself. Also, disabling that will, I guess, prevent possible bugs with ranged bombard/other orders.

Would the AI be affected? Does the AI use this code?

[Solver]

Would the AI be affected? Does the AI use this code?

I haven't yet found the code in question, but I'd think no. After all, it's more of an interface feauture, and only the human uses the interface. AI just sets goals for its units...

[J Bytheway]

Originally posted by Solver
But it would work the same if diagonally adjacent squared were distance=1, right?

Yes, it would be the same for distance 1 (and 0), but different for every other distance.

[Solver]

Grr, I can't test changes to the code. I finally compiled & linked a Debug version on this comp, but I keep getting pummeled by various assertion faults when strating a new game. As for compilning a non-Debug version, that one crashes when starting a new game.

John, haven't you, by chance, tried to compile and run the latest playtest?

[J Bytheway]

Originally posted by Solver
John, haven't you, by chance, tried to compile and run the latest playtest?

I uploaded it, so it would be a little negligent of me not to test it first at least briefly... So, yes, I have compiled and run it, but I only played it for a few turns. Other people seem to be running it fine, though...