What would you change? If anything...

[PaulRyan]

I have created a battle script for an online RPG I am creating, I've come up with the following, which does exactly what I need.

I would like to know if there is anything you would change, or anything that could be done a different/better way?

 

The battle should include the following:

- Highest agility attacks first (random number between 90%-110% of current agility, gives lower levels a chance to attack first)

- A chance to guard the attack using the guard value (5% chance currently)

- A chance to hit a power strike using the power strike value (5% chance currently) will hit 150%-200% of players attack power.

- If its a hit and not a power strike, a value between 95%-105% of players attack power.

 

I have filled the attacker and defender arrays with sample data, this will be retrieved from the database before the battle and replaced with real values which will be subject to change if skills/spells are active, items that are worn and special effects from various quests etc.

 

The reason for the attack array, is not only for displaying, but it makes it simpler to view/edit as well as the fact I can access the stats of the battle via the array, for successful attacks/blocks/power strike etc. for which I am wanting to store as everyone loves stats, and those will be pretty cool to view over an accounts life time.

 

Side note: I hate this code plugin, messes my code indenting/spacing.

 

<?PHP
//### Define attack ranges
$attackRanges = array('normal_attack_low' => 0.95,
				 'normal_attack_high' => 1.05,
				 'power_attack_low' => 1.5,
				 'power_attack_high' => 2.0,
				 'agility_range_low' => 0.9,
				 'agility_range_high' => 1.1
				 );
//### Assign the attacking players details
$attacker = array('name'		 => 'Player 1',
			 'attack'	 => 100,
			 'health'	 => 400,
			 'power_strike' => 5.0,
			 'guard'	 => 5.0,
			 'agility'	 => 500
			 );

//### Assign defending players details				
$defender = array('name'		 => 'Player 2',
			 'attack'	 => 100,
			 'health'	 => 400,
			 'power_strike' => 5.0,
			 'guard'	 => 5.0,
			 'agility'	 => 500
			 );

//### Randomly generate each players agility, with a percentage increase/decrease				
$attacker['agility'] = mt_rand($attacker['agility']*$attackRanges['agility_range_low'], $attacker['agility']*$attackRanges['agility_range_high']);
$defender['agility'] = mt_rand($defender['agility']*$attackRanges['agility_range_low'], $defender['agility']*$attackRanges['agility_range_high']);

//### Work out who attacks first
$attackTurn = $attacker['agility'] >= $defender['agility'] ? '1' : '2';

//### While both players are still alive, alternate who attacks
while($attacker['health'] > 0 && $defender['health'] > 0) {

//### Attacking players turn to attack
if($attackTurn == 1) {

 //### Does the defending player guard the attack?
 $defensiveGuard	 = mt_rand(0, 10000);
 $defensiveGuardRange = array('high' => 5000*((100+$defender['guard'])/100),
							 'low' => 5000*((100-$defender['guard'])/100)
							 );

 //### If the defensive guard is within the defensive range, the attack is blocked
 if($defensiveGuard >= $defensiveGuardRange['low'] && $defensiveGuard <= $defensiveGuardRange['high']) {

 $attackPower = mt_rand($attacker['attack']*$attackRanges['normal_attack_low'], $attacker['attack']*$attackRanges['normal_attack_high']);
 $attackHit = 0;
 $powerStrike = 0;

 //### If the defensive guard is not within the defensive range, the attack is not blocked
 } else {

 //### Is the attack a power strike?
 $offensiveStrike	 = mt_rand(0, 10000);
 $offensiveStrikeRange = array('high' => 5000*((100+$attacker['power_strike'])/100),
								 'low' => 5000*((100-$attacker['power_strike'])/100)
								 );

 //### If the power strike is within the power strike range, perform power strike							
 if($offensiveStrike >= $offensiveStrikeRange['low'] && $offensiveStrike <= $offensiveStrikeRange['high']) {

	 $attackPower = mt_rand($attacker['attack']*$attackRanges['power_attack_low'], $attacker['attack']*$attackRanges['power_attack_high']);
	 $attackHit = 1;
	 $powerStrike = 1;

 //### If the power strike is not within the power strike range, perform normal strike		
 } else {

	 $attackPower = mt_rand($attacker['attack']*$attackRanges['normal_attack_low'], $attacker['attack']*$attackRanges['normal_attack_high']);
	 $attackHit = 1;
	 $powerStrike = 0;

 }			
 }

 //### Make sure the defenders health does no go below 0
 if($attackHit == 1) {
 if(($defender['health']-$attackPower) < 0) {
	 $defender['health'] = 0;
 } else {
	 $defender['health'] -= $attackPower;
 }
 }

 //### Add the attack turn to the attack array
 $attackArray[] = array('whos_turn'	 => $attacker['name'],
						 'attack_hit' => $attackHit,
						 'power_strike' => $powerStrike,
						 'attack_power' => $attackPower,
						 'defend_health' => $defender['health']
					 );

 //### Define who won the battle
 $winner = $attacker['name'];

 //### Alternate to defenders attack turn
 $attackTurn = 2;

//### Defending players turn to attack
} else {

 //### Does the attacking player guard the attack?
 $offensiveGuard	 = mt_rand(0, 1000);
 $offensiveGuardRange = array('high' => 500*((100+$attacker['guard'])/100),
							 'low' => 500*((100-$attacker['guard'])/100)
							 );

 //### If the offensive guard is within the offensive range, the attack is blocked
 if($offensiveGuard >= $offensiveGuardRange['low'] && $offensiveGuard <= $offensiveGuardRange['high']) {
 $attackPower = mt_rand($defender['attack']*$attackRanges['normal_attack_low'], $defender['attack']*$attackRanges['normal_attack_high']);
 $attackHit = 0;
 $powerStrike = 0;

 //### If the offensive guard is not within the offensive range, the attack is not blocked
 } else {

 //### Is the attack a power strike?
 $defensiveStrike	 = mt_rand(0, 10000);
 $defensiveStrikeRange = array('high' => 5000*((100+$defender['power_strike'])/100),
								 'low' => 5000*((100-$defender['power_strike'])/100)
								 );

 //### If the power strike is within the power strike range, perform power strike							
 if($defensiveStrike >= $defensiveStrikeRange['low'] && $defensiveStrike <= $defensiveStrikeRange['high']) {

	 $attackPower = mt_rand($defender['attack']*$attackRanges['power_attack_low'], $defender['attack']*$attackRanges['power_attack_high']);
	 $attackHit = 1;
	 $powerStrike = 1;

 //### If the power strike is not within the power strike range, perform normal strike		
 } else {

	 $attackPower = mt_rand($defender['attack']*$attackRanges['normal_attack_low'], $defender['attack']*$attackRanges['normal_attack_high']);
	 $attackHit = 1;
	 $powerStrike = 0;

 }	
 }

 //### Make sure the defenders health does no go below 0
 if($attackHit == 1) {
 if(($attacker['health']-$attackPower) < 0) {
	 $attacker['health'] = 0;
 } else {
	 $attacker['health'] -= $attackPower;
 }
 }

 //### Add the attack turn to the attack array
 $attackArray[] = array('whos_turn'	 => $defender['name'],
						 'attack_hit' => $attackHit,
						 'power_strike' => $powerStrike,
						 'attack_power' => $attackPower,
						 'defend_health' => $attacker['health']
					 );

 //### Define who won the battle
 $winner = $defender['name'];

 //### Alternate to attackers attack turn
 $attackTurn = 1;
}
}

echo '<pre>';

foreach($attackArray AS $attackTurn) {
if($attackTurn['attack_hit'] == 0) {
 echo $attackTurn['whos_turn'] .' attacked for '. number_format($attackTurn['attack_power']) .' but the attack was blocked ['. $attackTurn['defend_health'] .']<br>';
} else if($attackTurn['power_strike'] == 1) {
 echo 'Power Strike! '. $attackTurn['whos_turn'] .' attacked for '. number_format($attackTurn['attack_power']) .' ['. $attackTurn['defend_health'] .']<br>';
} else {
 echo $attackTurn['whos_turn'] .' attacked for '. number_format($attackTurn['attack_power']) .' ['. $attackTurn['defend_health'] .']<br>';
}
}

echo '<br>'. $winner .' is victorious.';

?>

[scootstah]

Probably the biggest change/most beneficial change would be to make it OOP. This will allow you to re-use code, and maintenance will be easier.

 

For example, I see this block of code being used multiple times:

//### Make sure the defenders health does no go below 0
if($attackHit == 1) {
if(($defender['health']-$attackPower) < 0) {
    $defender['health'] = 0;
} else {
    $defender['health'] -= $attackPower;
}
}

 

Wouldn't it be better if, when you need to damage someone, you didn't have to repeat this logic? You could simply call a class method like, $defender->decreaseHealth($attackPower);

 

If you come up with another way that someone might take damage (like some form of self damage, maybe from falling or from a confusion spell), you only have to call this method.

 

On that note, your logic for this particular case can be simplified to:

if ($attackHit == 1) {
$defender['heath'] = max(($defender['heath'] - $attackPower), 0);
}

 

The max function, in this case, will not let the number go below 0. It will return the higher value of the arguments given, and since 0 is > -N, it will return 0.

 

I didn't thoroughly go over your code, this just stuck out at me as I was looking for an OOP example.

 

Also, try to use proper data types in your code. Instead of having $attackHit equal to 1 or 0, make it an actual boolean (true/false). It makes more sense to people reading your code.

 

[PaulRyan]

Thanks for the suggestions.

 

Firstly, I myself am not too keen on OOP, I've heard many postive things about it, I just cannot get my head around it. This is something I will consider though, so thanks.

 

Secondly, thanks for the max(), I didn't think to use it in such a way.

 

Thirdly, I will do as you say, I always forget to use true/false, been so used to using 1 and 0, I naturally associate the numbers with true/false in my head, so I use that format.

[shlumph]

You don't have to use OOP to make your code reusable, more clear, and easier to maintain. I would encapsulate a lot of what you have into functions. You could also consider getting rid of the loops, and using recursion.