Jump to content

Recommended Posts

These couple of loops are frustrating to say the least.

 

There is a few thousand database entries for my `Weapon` table, and each player has hundreds of thousands of Soldiers.

 

The following 2 loops assign soldiers weapons until there is either no more weapons to assign, or no more soldiers to assign the weapons to.

 

The problem is I get script time outs with the loops as they just can't finish with the stronger players.

 

Is there a way the loop can be replaced but still do the same task?

          $wI = 0;
          // keep adding strength until finally out of equippable units
// $wepAlloc["trainedW"] and untrainedW are the soldiers
          for ($i = 0; $i < $wepAlloc["trainedW"]; $i++, $wI++) {
          	  // add the weapon strength to strike action
              $strike += $weapons[$wI]->weaponStrength; // adding the weapons strength
          }
          for ($i = 0; $i < $wepAlloc["untrainedW"]; $i++, $wI++) {
            $strike += $weapons[$wI]->weaponStrength;
          }

 

Regards, ACE

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/
Share on other sites

here is the functions...

  function getWeaponAllocation($user, $weaponA, $trainedCount, $untrainedCount)
  {
      if (count($weaponA) > $trainedCount) {
          $trainedW = $trainedCount;
          $untrainedUnW = $untrainedCount - (count($weaponA) + $trainedW);
          if ($untrainedUnW < 0)
              $untrainedUnW = 0;
          $untrainedW = $untrainedCount - $untrainedUnW;
          $trainedUnW = 0;
      } else {
          
          $trainedW = count($weaponA);
          $untrainedW = 0;
          $trainedUnW = $trainedCount - $trainedW;
          $untrainedUnW = $untrainedCount;
      }
      if ($untrainedW < 0)
          $untrainedW = 0;
      
      $wepAlloc["trainedW"] = $trainedW;
      $wepAlloc["untrainedUnW"] = $untrainedUnW;
      $wepAlloc["untrainedW"] = $untrainedW;
      $wepAlloc["trainedUnW"] = $trainedUnW;
      
      return $wepAlloc;
  }

  function getWeapons($type,$userID)
  {
	 	if($type == "attack") {
	  	$Weapons = mysql_query("SELECT * FROM `Weapon` WHERE  userID='{$userID}' && isAtack='1' ORDER BY `weaponStrength` DESC") or die(mysql_error());
	  	$Weapons = mysql_fetch_array($Weapons);
	 	} elseif($type == "defence") {
	  	$Weapons = mysql_query("SELECT * FROM `Weapon` WHERE  userID='{$userID}' && isAtack='0' ORDER BY `weaponStrength` DESC") or die(mysql_error());
	  	$Weapons = mysql_fetch_array($Weapons);
	 	}
return $Weapons;
  }
  
function getStrikeAction($user)
  {
      global $conf;
      // default strike is zero
      $strike = 0;
      
      // Total Trained Attack
      $trainedCount = $user->trainedAttackSold + $user->trainedAttackMerc;
      // Total Untrained
      $untrainedCount = $user->untrainedMerc + $user->untrainedSold;
      // Total Units
      $totalCount = $trainedCount + $untrainedCount;
      
      // Power of Trained Attack Soldiers
      $attackSoldiersPower = $user->trainedAttackSold * 5;
      // Power of Trained Attack Mercenaries
      $attackMercsPower = $user->trainedAttackMerc * 4;
      // Power of Untrained Soldiers
      $untrainedSoldiersPower = $user->untrainedSold * 2;
      // Power of Untrained Mercenaries
      $untrainedMercsPower = $user->untrainedMerc * 2;
      
      // calculate current strike action
      $strike += $attackSoldiersPower;
      $strike += $attackMercsPower;
      $strike += $untrainedSoldiersPower;
      $strike += $untrainedMercsPower;
      
      // grab Attack Weapons
   	  $weapons = getWeapons("attack",$user->ID);
   	  
   	  // if the player has Attack Weapons...
if ($weapons) 
{
/* equip weapons from strongest to weakest. First on trained attack soldiers then attack mercenaries, then the untrained */ 
	  // workout how many soldiers and mercenaries can equip weapons
          $wepAlloc = getWeaponAllocation($user, $weapons, $trainedCount, $untrainedCount);
          $wI = 0;
          // keep adding strength until finally out of equippable units
          for ($i = 0; $i < $wepAlloc["trainedW"]; $i++, $wI++) {
          	  // add the weapon strength to strike action
              $strike += $weapons[$wI]->weaponStrength;
          }
          for ($i = 0; $i < $wepAlloc["untrainedW"]; $i++, $wI++) {
            $strike += $weapons[$wI]->weaponStrength;
          }
}
      
      // add Siege bonuses
      for ($i = 0; $i <= $user->siegeLevel; $i++) {
          if ($conf["race"][$user->race]["siege"][$i]["attack"]) {
              $strike += round(($strike * $conf["race"][$user->race]["siege"][$i]["attack"]) / 100);
          }
      }
      
      // add race bonuses
      if ($conf["race"][$user->race]["attack"]) {
          $strike += round(($strike * $conf["race"][$user->race]["attack"]) / 100);
      }
      
      // fix up strike incase it is a string
      $strike = (int)($strike+0);
      // return final strike action
      return $strike;
  } 

 

and here is the database setup for those functions...

-- --------------------------------------------------------

 

--

-- Table structure for table `UserDetails`

--

 

CREATE TABLE IF NOT EXISTS `UserDetails` (

  `ID` int(11) NOT NULL AUTO_INCREMENT,

  `userName` varchar(100) NOT NULL DEFAULT '',

  `race` tinyint(4) NOT NULL DEFAULT '0',

  `e_mail` varchar(255) NOT NULL DEFAULT '',

  `password` varchar(50) NOT NULL DEFAULT '',

  `commander` int(11) NOT NULL DEFAULT '0',

  `active` int(1) NOT NULL DEFAULT '0',

  `uniqueLink` varchar(12) NOT NULL DEFAULT '',

  `strikeaction` bigint(255) unsigned NOT NULL DEFAULT '0',

  `defenceaction` bigint(255) unsigned NOT NULL DEFAULT '0',

  `covertaction` bigint(255) unsigned NOT NULL DEFAULT '0',

  `fortificationLevel` bigint(255) NOT NULL DEFAULT '0',

  `siegeLevel` bigint(255) NOT NULL DEFAULT '0',

  `gold` bigint(255) unsigned NOT NULL DEFAULT '0',

  `bank` bigint(255) unsigned NOT NULL DEFAULT '0',

  `attackTurns` bigint(255) NOT NULL DEFAULT '0',

  `currentUnitProduction` bigint(255) DEFAULT NULL,

  `currentSpySkill` bigint(255) DEFAULT NULL,

  `trainedAttackSold` bigint(255) DEFAULT NULL,

  `trainedAttackMerc` bigint(255) DEFAULT NULL,

  `trainedDefSold` bigint(255) DEFAULT NULL,

  `trainedDefMerc` bigint(255) DEFAULT NULL,

  `untrainedSold` bigint(255) DEFAULT NULL,

  `untrainedMerc` bigint(255) DEFAULT NULL,

  `spies` bigint(255) DEFAULT NULL,

  `miners` bigint(225) unsigned NOT NULL,

  `morale` int(225) NOT NULL DEFAULT '100',

  `lastTurnTime` int(11) NOT NULL DEFAULT '0',

  `income` bigint(255) NOT NULL DEFAULT '0',

  `title` int(50) NOT NULL DEFAULT '0',

  `bet_warg` varchar(75) NOT NULL,

  `bet_copper` bigint(225) unsigned NOT NULL DEFAULT '0',

  `support` int(50) NOT NULL DEFAULT '0',

  `changerace` int(50) NOT NULL DEFAULT '1',

  `changename` int(50) NOT NULL DEFAULT '0',

  `SScount` int(225) NOT NULL DEFAULT '0',

  `USScount` int(225) NOT NULL DEFAULT '0',

  `protection` int(255) NOT NULL DEFAULT '0',

  `Alliance` varchar(50) NOT NULL,

  `Alliance_Status` mediumint(50) NOT NULL DEFAULT '0',

  `Alliance_Ranks` int(1) NOT NULL DEFAULT '0',

  `Online` mediumint(50) NOT NULL DEFAULT '0',

  `Online_Status` mediumint(50) NOT NULL DEFAULT '0',

  `banned` tinyint(1) NOT NULL DEFAULT '0',

  `ban_count` tinyint(1) NOT NULL DEFAULT '0',

  `ban_comment` varchar(255) NOT NULL,

  `banned_by` varchar(150) NOT NULL,

  `last_chat` tinyint(1) NOT NULL DEFAULT '1',

  `last_chat2` int(50) NOT NULL,

  `SSused` int(225) NOT NULL DEFAULT '0',

  `aat` int(2) NOT NULL DEFAULT '0',

  `last_achat2` int(255) NOT NULL,

  `referrals` int(255) NOT NULL DEFAULT '0',

  `referred_by` int(255) NOT NULL DEFAULT '0',

  `last_achat` int(255) NOT NULL,

  `ainvites` int(1) NOT NULL DEFAULT '0',

  `filter` int(1) NOT NULL DEFAULT '1',

  `IE` int(1) NOT NULL DEFAULT '0',

  PRIMARY KEY (`ID`)

) ENGINE=MyISAM  DEFAULT CHARSET=latin1 AUTO_INCREMENT=483 ;

 

-- --------------------------------------------------------

 

--

-- Table structure for table `Weapon`

--

 

CREATE TABLE IF NOT EXISTS `Weapon` (

  `ID` int(11) NOT NULL AUTO_INCREMENT,

  `weaponID` int(11) NOT NULL DEFAULT '0',

  `weaponStrength` bigint(255) NOT NULL DEFAULT '0',

  `weaponCount` bigint(255) NOT NULL DEFAULT '0',

  `isAtack` int(11) NOT NULL DEFAULT '0',

  `userID` int(11) NOT NULL DEFAULT '0',

  PRIMARY KEY (`ID`)

) ENGINE=MyISAM  DEFAULT CHARSET=latin1 AUTO_INCREMENT=603 ;

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-834528
Share on other sites

well...

if you really have to work like that...

have a look at the resource limits in your php.ini:

 

;;;;;;;;;;;;;;;;;;;
; Resource Limits ;
;;;;;;;;;;;;;;;;;;;

max_execution_time = 30     ; Maximum execution time of each script, in seconds
max_input_time = 60 ; Maximum amount of time each script may spend parsing request data
;max_input_nesting_level = 64 ; Maximum input variable nesting level
memory_limit = 128M      ; Maximum amount of memory a script may consume (16MB)

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-834806
Share on other sites

Consider restructuring your code to use

foreach($weapons as $weaponKey $weaponValue)

rather than

for ($i = 0; $i < $wepAlloc["trainedW"]; $i++, $wI++)

because foreach is normally more efficient

 

Minimise and optimise the code in your loop functions:

  function getWeaponAllocation($user, $weaponA, $trainedCount, $untrainedCount)
  {
      if (count($weaponA) > $trainedCount) {
          $trainedW = $trainedCount;
          $untrainedUnW = $untrainedCount - (count($weaponA) + $trainedW);
          if ($untrainedUnW < 0)
              $untrainedUnW = 0;
          $untrainedW = $untrainedCount - $untrainedUnW;
          $trainedUnW = 0;
      } else {
          
          $trainedW = count($weaponA);
          $untrainedW = 0;
          $trainedUnW = $trainedCount - $trainedW;
          $untrainedUnW = $untrainedCount;
      }
      if ($untrainedW < 0)
          $untrainedW = 0;
      
      $wepAlloc["trainedW"] = $trainedW;
      $wepAlloc["untrainedUnW"] = $untrainedUnW;
      $wepAlloc["untrainedW"] = $untrainedW;
      $wepAlloc["trainedUnW"] = $trainedUnW;
      
      return $wepAlloc;
  }

to

  function getWeaponAllocation($user, $weaponA, $trainedCount, $untrainedCount)
  {
      $weaponACount = count($weaponA);
      if ($weaponACount > $trainedCount) {
          $trainedW = $trainedCount;
          $untrainedUnW = $untrainedCount - ($weaponACount + $trainedW);
          if ($untrainedUnW < 0)
              $untrainedUnW = 0;
          $untrainedW = $untrainedCount - $untrainedUnW;
          $trainedUnW = 0;
      } else {
          $trainedW = $weaponACount;
          $untrainedW = 0;
          $trainedUnW = $trainedCount - $trainedW;
          $untrainedUnW = $untrainedCount;
      }
      if ($untrainedW < 0)
          $untrainedW = 0;
      
      return array( "trainedW"] => $trainedW,
                    "untrainedUnW" => $untrainedUnW,
                    "untrainedW"] => $untrainedW,
                    "trainedUnW" = $trainedUnW
                  );
  }

precalc values such as count($weaponA) that are used several times and are otherwise being recalculated every time you use them.

Don't create unnecessary variables such as your $wepAlloc array, used only for the return value... return the data without creating the variable

 

Pre-increment (where possible) rather than post-increment

for ($i = 0; $i < $wepAlloc["trainedW"]; ++$i, ++$wI) {

 

There's just a few examples of things you can do to speed up your code

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-834907
Share on other sites

I have made the changes you suggested and have changed the for loop part to foreach but it isn't making any difference to the final strike action value. I'm not entirely sure if my logic behind it is correct.

 

changed this...

          $wI = 0;
          // keep adding strength until finally out of equippable units
          for ($i = 0; $i < $wepAlloc["trainedW"]; $i++, $wI++) {
          	  // add the weapon strength to strike action
              $strike += $weapons[$wI]->weaponStrength;
          }
          for ($i = 0; $i < $wepAlloc["untrainedW"]; $i++, $wI++) {
            $strike += $weapons[$wI]->weaponStrength;
          }

 

to this...

        // workout how many soldiers can equip weapons
        $difference = count($weapons) - ($wepAlloc["trainedW"] + $wepAlloc["untrainedW"]);
        
        foreach($weapons as $weaponKey => $weaponVal) {
        	// add weapon strength
        	$strike += $weaponKey['weaponStrength'];
        }
        
foreach($weapons as $weaponKey => $weaponVal) {
              // now take away weapons that soldiers and mercenaries haven't equipped
                if($difference > 0) {
        		$strike -= $difference * $weaponKey['weaponStrength'];	
        	}
     }

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-835097
Share on other sites

I have made the changes you suggested and have changed the for loop part to foreach but it isn't making any difference to the final strike action value. I'm not entirely sure if my logic behind it is correct.

Well if the logic is correct in the first place, and you've not said that your logic is wrong, then you would expect to get the same value returned. However, the change should return it more quickly than the unoptimised version

 

From your original post, I got the impression that speed within the loop was your issue, not that the code wasn't calculating the value correctly

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-835188
Share on other sites

speed was the issue. But...

 

this...

        // workout how many soldiers can equip weapons
        $difference = count($weapons) - ($wepAlloc["trainedW"] + $wepAlloc["untrainedW"]);
        
        foreach($weapons as $weaponKey => $weaponVal) {
        	// add weapon strength
        	$strike += $weaponKey['weaponStrength'];
        }
        
foreach($weapons as $weaponKey => $weaponVal) {
              // now take away weapons that soldiers and mercenaries haven't equipped
                if($difference > 0) {
        		$strike -= $difference * $weaponKey['weaponStrength'];	
        	}
     }

 

isn't adding the same amount to $strike as this was...

          $wI = 0;
          // keep adding strength until finally out of equippable units
// $wepAlloc["trainedW"] and untrainedW are the soldiers
          for ($i = 0; $i < $wepAlloc["trainedW"]; $i++, $wI++) {
          	  // add the weapon strength to strike action
              $strike += $weapons[$wI]->weaponStrength; // adding the weapons strength
          }
          for ($i = 0; $i < $wepAlloc["untrainedW"]; $i++, $wI++) {
            $strike += $weapons[$wI]->weaponStrength;
          }

 

so I'm not sure where I've gone wrong?

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-835556
Share on other sites

Surely

foreach($weapons as $weaponKey => $weaponVal) {
   // add weapon strength
   $strike += $weaponKey['weaponStrength'];
}

should be

foreach($weapons as $weaponVal) {
   // add weapon strength
   $strike += $weaponVal->weaponStrength;
}

or

foreach($weapons as $weaponKey => $weaponVal) {
   // add weapon strength
   $strike += $weapons[$weaponKey]->weaponStrength;
}

 

Link to comment
https://forums.phpfreaks.com/topic/158091-replace-loop/#findComment-835739
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.