widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

I added the try/catch because I was suspecting holes in the genstats
array, but the hole is in miltary_strength. So, the important point here
is to check for military_strength.empty() - if miltary_strength.back()
gives us nothing, we do get a division by 0. The try/catch isn't that
important really, so if you think it's not necessary, we can revert to
using genstats[j - 1] rather than player_attackable.at(j - 1). I just
thought it can't hurt to leave it in.



01/11/2014 20:09, sgrìobh TiborB:
> GunChleoc - this is not about zero division but about out-of-range - I think.
> 
> Do you think that problem is that genstats is not populated some very short time after loading and AI tries to access members that are not there yet? So that genstats.size()==0? 
> 
> If this is the case I would use other aproach then try-catch...
> 
> Also see comments in diff, default decision for AI if it can not get data on military strength of enemy should be 'do not attack'
> 
> 
> 
> 
> Diff comments:
> 
>> === modified file 'src/ai/defaultai.cc'
>> --- src/ai/defaultai.cc	2014-10-31 20:49:02 +0000
>> +++ src/ai/defaultai.cc	2014-11-01 14:10:33 +0000
>> @@ -698,7 +698,7 @@
>>  		}
>>  	}
>>  
>> -	// folowing is done allways (regardless of military or not)
>> +	// the following is done always (regardless of military or not)
>>  
>>  	// we get immovables with higher radius
>>  	immovables.clear();
>> @@ -710,6 +710,7 @@
>>  	field.military_presence_ = 0;
>>  
>>  	for (uint32_t i = 0; i < immovables.size(); ++i) {
>> +
>>  		const BaseImmovable& base_immovable = *immovables.at(i).object;
>>  
>>  		// testing if it is enemy-owned field
>> @@ -3089,20 +3090,27 @@
>>  	const Game::GeneralStatsVector& genstats = game().get_general_statistics();
>>  	for (uint8_t j = 1; j <= plr_in_game; ++j) {
>>  		if (pn == j) {
>> -			player_attackable[j - 1] = false;
>> +			player_attackable.at(j - 1) = false;
>>  			continue;
>>  		}
>>  
>> -		if (genstats[j - 1].miltary_strength.back() == 0) {
>> -			// to avoid improbable zero division
>> -			player_attackable[j - 1] = true;
>> -			any_attackable = true;
>> -		} else if ((genstats[pn - 1].miltary_strength.back() * 100 /
>> -		            genstats[j - 1].miltary_strength.back()) > treshold_ratio) {
>> -			player_attackable[j - 1] = true;
>> -			any_attackable = true;
>> -		} else {
>> -			player_attackable[j - 1] = false;
>> +		try {
>> +			// Avoid division by zero
>> +			if (genstats.at(j - 1).miltary_strength.empty() ||
>> +				 genstats.at(j - 1).miltary_strength.back() == 0) {
>> +				player_attackable.at(j - 1) = true;
>> +				any_attackable = true;
>> +			// Check threshold
>> +			} else if ((genstats.at(pn - 1).miltary_strength.back() * 100 /
>> +							genstats.at(j - 1).miltary_strength.back()) > treshold_ratio) {
>> +				player_attackable.at(j - 1) = true;
>> +				any_attackable = true;
>> +			} else {
>> +				player_attackable.at(j - 1) = false;
>> +			}
>> +		} catch (const std::out_of_range&) {
>> +			player_attackable.at(j - 1) = true;
> 
> change to false
> 
>> +			any_attackable = true;
> 
> remove the line
> 
>>  		}
>>  	}
>>  
>>
> 
> 

-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1388028/+merge/240357
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1388028.