1. We have added the ability to collapse/expand forum categories and widgets on forum home.
    Dismiss Notice
  2. Photobucket has changed its policy concerning hotlinking images and now requires an account with a $399.00 annual fee to allow hotlink. More information is available at: this link.
    Dismiss Notice
  3. All Civ avatars are brought back and available for selection in the Avatar Gallery! There are 945 avatars total.
    Dismiss Notice
  4. To make the site more secure, we have installed SSL certificates and enabled HTTPS for both the main site and forums.
    Dismiss Notice
  5. Civ6 is released! Order now! (Amazon US | Amazon UK | Amazon CA | Amazon DE | Amazon FR)
    Dismiss Notice
  6. Dismiss Notice
  7. Forum account upgrades are available for ad-free browsing.
    Dismiss Notice

CvUnit bugs with ranged strikes

Discussion in 'Civ4 - Unofficial Patches' started by God-Emperor, Mar 30, 2010.

  1. God-Emperor

    God-Emperor Chieftain

    Joined:
    Jul 18, 2009
    Messages:
    3,551
    Location:
    Texas
    File CvUnit.cpp:

    I believe I have found one or two bugs in the CvUnit::rangeStrike and one in CvUnit::canRangeStrikeAt.

    The net result is that calling rangeStrike(iX,iY) will never reject an attack on a plot due to range, whether there is terrain or features that should be blocking the attack, or the target plot not currently being visible (or ever revealed to the attacker). The possible second bug in rangeStrike may be related to OOS states that happen from ranged strikes.

    (All line numbers given are for the unaltered BtS CvUnit.cpp file.)

    1) CvUnit::rangeStrike problems:
    1.a) On line 11912 it is using "pPlot" as the first argument in the call to canRangeStrike.
    Code:
    	if (!canRangeStrikeAt(pPlot, iX, iY))
    The pPlot is derived from the iX,iY values that are passed to rangeStrike and are being passed to canRangeStrikeAt as the 2nd and 3rd arguments, so it is the same plot. This causes canRangeStrikeAt to determine if the plot at iX, iY can have a ranged strike done on it from a unit in pPlot, which is the same plot. The answer is always "True" as long as there is a valid target in the plot since the check to see if it is in range (line 18970) will always find that the range of 0 from the plot to itself is less than the unit's air range. On line 11890 it will also find out that you can always see a plot from the plot itself.

    So it seems clear that it in rangeStrike it should be passing the unit's own position's plot, not the target's plot, as the first argument. Every other unit mission I looked at (recon, airdrop, nuke, etc.) uses "plot()" for the first argument to the corresponding canThingAt() method).

    Proposed fix:
    In CvUnit::rangeStrike, line 11912, change "canRangeStrikeAt(pPlot, iX, iY)" to "canRangeStrikeAt(plot(), iX, iY)" so that it is checking to see if the unit can do a rangeStrike on plot iX,iY from the attacking unit's current plot instead of from the target plot.
    Code:
    	if (!canRangeStrikeAt(plot(), iX, iY))
    1.b) A potential OOS problem (ranged attacks are know to have such problems).
    At the end of the rangeStrike function (line 11958) there is this:
    Code:
    		//delay death
    		pDefender->getGroup()->setMissionTimer(GC.getMissionInfo(MISSION_RANGE_ATTACK).getTime());
    This is inside the "if (pPlot->isActiveVisible(false))" block, thus it will be run on computers where the human player can see the plot where the attack happened. It will not be run on computers where the human player can not see it. In multiplayer games this could be introducing timing differences, leading to state differences. On computers where the human player can not see the plot there is no change to the defending unit's mission timer, but on computers where the human player can see the plot there is a delay. I'm not certain this is a problem, but it certainly can not be a good thing if anything happens during the delay interval.

    Proposed fix:
    I'm not sure. Either remove the statement entirely, so it happens for nobody, or move it out of the if block so that it happens for everybody. I don't know what the exact effects of doing either of them would be.


    2) There is also a bug in canRangeStrikeAt.
    It checks the passed pPlot to see if it is visible to the attacking player but it doesn't check to see if the pTargetPlot derrived from the passed iX,iY is visibile.

    It seems like around the point it checks pPlot->isVisible (line 11874), it ought to also check pTargetPlot->isVisible. This would not be a bug, as such, if the last pPlot->canSeePlot test ever checked the target plot's basic visibility, but it doesn't. It is only checking to see if it is possible to see the specified plot from the current plot, not if it is actually visible at this time.

    Net result: if there is a valid target on the plot at iX,iY and if nothing is blocking the path, like a mountain or asteroid field (in Final Frontier) then canRangeStrikeAt returns true even if the target plot is not currently visible (or has never even been revealed to the player).

    Proposed fix:
    In CvUnit::canRangeStrikeAt, at line 11878, add another if block like the immediately preceding one but checking the visibility for pTargetPlot rather than pPlot.
    Old:
    Code:
    	if (!pPlot->isVisible(getTeam(), false))
    	{
    		return false;
    	}
    
    	if (plotDistance(pPlot->getX_INLINE(), pPlot->getY_INLINE(), pTargetPlot->getX_INLINE(), pTargetPlot->getY_INLINE()) > airRange())
    New:
    Code:
    	if (!pPlot->isVisible(getTeam(), false))
    	{
    		return false;
    	}
    
    	if (!pTargetPlot->isVisible(getTeam(), false))
    	{
    		return false;
    	}
    
    	if (plotDistance(pPlot->getX_INLINE(), pPlot->getY_INLINE(), pTargetPlot->getX_INLINE(), pTargetPlot->getY_INLINE()) > airRange())
    It is possible that instead of adding another visibility check changing the existing check to be for the target plot would be acceptable. If canRangedStrikeAt is never used by the AI to decide if it should move a unit to do the attack, then changing the existing test is probably the way to go.
     
  2. God-Emperor

    God-Emperor Chieftain

    Joined:
    Jul 18, 2009
    Messages:
    3,551
    Location:
    Texas
    After a bit more thought, it turns out that the "not currently being visible (or ever revealed to the attacker)" part at the end of
    is not correct. Due to the combination of bugs "1.a" and "2" it is currently checking the visibility of the target plot since the target plot and the source plot are currently the same plot in the call to canRangeStrikeAt, which currently checks the visibility of the source plot.
     
  3. jdog5000

    jdog5000 Revolutionary

    Joined:
    Nov 25, 2003
    Messages:
    2,601
    Location:
    California
    Thanks for the reports! Will fold this in.
     

Share This Page