Problem
I have a flight class that looks like this:
class Flight {
public $id; // unique id
public $structures = array(); // each "structure" indicates one fighter class assembled in this flight class
public $dogfights = array() if the flight is dogfighting, this array holds the ID of each opposing flight we are dogfighting
}
class Fighter { // this is whats inside $Flight->structures
public $id
public $parentid
}
If two opposing flights are close to each other, I initiate a dogfight. I determine if they are dogfighting like this:
public function initiateDogfights(){
$dogfights = array();
for ($i = 0; $i < sizeof($this->ships)-1; $i++){
for ($j = $i+1; $j < sizeof($this->ships); $j++){
$dist = Math::getDist2($this->ships[$i]->getCurrentPosition(), $this->ships[$j]->getCurrentPosition());
if ($dist <= $this->ships[$i]->size / 2 + $this->ships[$j]->size / 2){
$new = true;
for ($k = 0; $k < sizeof($this->ships[$i]->dogfights); $k++){
if ($this->ships[$i]->dogfights[$k] == $this->ships[$j]->id){
$new = false;
}
}
if ($new){
$this->ships[$i]->dogfights[] = $this->ships[$j]->id;
$this->ships[$j]->dogfights[] = $this->ships[$i]->id;
$dogfights[] = array(0 => $this->ships[$i]->id, 1 => $this->ships[$j]->id);
}
}
}
}
if (sizeof($dogfights)){
DBManager::app()->insertDogfights($this->gameid, $this->turn, $dogfights);
}
}
Basicly, I compare all ships versus all ships (aka flights), compare their distance to each other. If it’s below a threshold, I push the opposing flights ID into the flights->dogfights array.
I check if the dogfights are new, because they might have been transferred from the DB and alas be initiated a turn ago. So, each flight’s $dogfights prop holds the IDs of all opposing flights this flight is tackling. Now, this part is working correctly.
Now, here comes the tricky part:
Each flight has a certain amount of Fighters (in the $structures prop). The amount of dogfights a flight can participate in is unlimited. So, it’s very much possible that one friendly flight is dogfighting two enemy flights. As the number of structures / fighters in a flight is unlimited, a single flight of 5 fighters could fight against 2 flights divided into 2 and 10 fighters.
My 5 fighters now need to determine which enemy fighter, or flight, they target, and I want it to be weighted evenly, depending on the amount of fighters in a flight. In the above example, engaging 2 and 10 fighters with 5 fighters, for each of my own fighters, there is a chance of 20%/80% to pick a target from the 2/10 man flight.
I determine this here:
public function createDogfightTargetData(){
$fires = array();
for ($i = 0; $i < sizeof($this->ships); $i++){
$flights = array();
$counts = array(0);
if ($this->ships[$i]->flight && sizeof($this->ships[$i]->dogfights)){
for ($j = 0; $j < sizeof($this->ships[$i]->dogfights); $j++){
$count = $counts[sizeof($counts)-1];
$flights[] = $this->getUnitById($this->ships[$i]->dogfights[$j]);
for ($k = 0; $k < sizeof($flights[sizeof($flights)-1]->structures); $k++){
if (! $flights[sizeof($flights)-1]->structures[$k]->destroyed){
$count++;
}
}
$counts[] = $count;
}
$fires = array_merge($fires, $this->ships[$i]->createFireOrders($this->gameid, $this->turn, $flights, $counts));
}
}
if (sizeof($fires)){
DBManager::app()->insertFireOrders($this->gameid, $this->turn, $fires);
}
return true;
}
Basically, I’m setting up an array that holds the numbers of enemy units in each index and sum it up. In the above example, where I’m fighting 2 and 10 fighters, the array would like this:
$counts = [0, 2, 12]
I pass this $counts array AND the associated Flights array (which holds object pointers to each Flight class involved in dogfights with this Flight) to another function to determine which enemy flight each friendly fighter is going to hit.
public function createFireOrders($gameid, $turn, $flights, $counts){
$fires = array();
//echo $this->id; echo "</br>";
//var_export($counts);
for ($i = 0; $i < sizeof($this->structures); $i++){
if (!$this->structures[$i]->destroyed){
$roll = mt_rand(0, $counts[sizeof($counts)-1]);
$index = -1;
for ($j = sizeof($counts)-1; $j >= 0; $j--){
if ($roll == 0){
$index = 0;
}
else if ($roll <= $counts[$j] && $roll > $counts[$j-1]){
$index = $j-1;
}
if ($index > -1){
$fires[] = array(
"gameid" => $gameid,
"turn" =>$turn,
"shooterid" => $this->id,
"targetid" => $flights[$index]->id,
"weaponid" => $this->structures[$i]->systems[0]->id,
"shots" => $this->structures[$i]->systems[0]->shots
);
break;
}
}
}
}
return $fires;
}
Here, I’m checking my own flight for “intact” fighters and if found, roll a dice from count[0] to count(sizeof($count)1), then compare the dice to each index of the $count and henceforth associate the index in $count with the flight from $flights (passed parameter).
Example:
$targets = [0, 2, 12]
$flights = [$id4, $id6]
$roll 7 -> between $targets[1] [2] -> pick $flights[2]
This function is working flawlessly, however, it feels kinda awkward.
Can this be improved in a reasonable way, and if so, how?
Solution
I agree with Mike Brant’s comments that it would be better to have the complete code posted for a broad review but that likely won’t happen- if you do want to do so then I advise you to do so in a new post, since editing your post might invalidate the advice below. Nonetheless i see a few idiomatic PHP aspects that could be improved.
The code could be simplified using PHP’s foreach loops instead of for loops- e.g.
for ($i = 0; $i < sizeof($this->ships)-1; $i++){
for ($j = $i+1; $j < sizeof($this->ships); $j++){
this could be simplified using foreach loops with the range() function:
foreach (range(0, sizeof($this->ships)-2) as $i) {
foreach (range($i, sizeof($this->ships)-1) as $j) {
and also the loop in createFireOrders():
for ($i = 0; $i < sizeof($this->structures); $i++){
if (!$this->structures[$i]->destroyed){
with foreach there is no need to do the bookkeeping on the counter variable:
foreach ($this->structures as $i => $structure) {
and instead of referring to $this->structures[$i] just use $structure.
The array on this line in initiateDogfights():
$dogfights[] = array(0 => $this->ships[$i]->id, 1 => $this->ships[$j]->id);
doesn’t need to have the indexes explicitly set:
$dogfights[] = array($this->ships[$i]->id, $this->ships[$j]->id);