I am still a Newbie at PHP but I'm learning alot from these forums. I have created a battle script that will produce a winner based on stats and give associated stats based on win or loss. If anybody can help me clean this script up I would appriciate it. As I said I am a noob and know that there is alot of junk in my script that shouldn't be there or could be written a lot cleaner. Thank you in advance. $enemy = mysql_query("SELECT * FROM skills WHERE uidskills='$_POST[uidenemy]'"); while($rowe = mysql_fetch_array($enemy)) { $enemyuid = $_POST['uidenemy']; $mhealth = $rowe['health']; $magility = $rowe['agility']; $mdexterity = $rowe['dexterity']; $mevade = $rowe['evade']; $mbarehands = $rowe['barehands']; $mstrength = $rowe['strength']; } $player = mysql_query("SELECT * FROM skills WHERE uidskills='$_SESSION[uid]'"); while($rowp = mysql_fetch_array($player)) { $phealth = $rowp['health']; $pagility = $rowp['agility']; $pdexterity = $rowp['dexterity']; $pevade = $rowp['evade']; $pbarehands = $rowp['barehands']; $pstrength = $rowp['strength']; $pstamina = $rowp['stamina']; } $pspeed = ($pagility + ($pdexterity/2)); $mspeed = ($magility + ($mdexterity/2)); $pattack = ($pbarehands + ($pstrength/2)); $mattack = ($mbarehands + ($mstrength/2)); $pdefence = ($pevade + ($pagility/2)); $mdefence = ($mevade + ($magility/2)); $pattacklow = ($pattack/2); $mattacklow = ($mattack/2); //Check for Positive Stamina if ($pstamina>0) { echo "<table width=750 border=2 cellspacing=0 cellpading=0>"; echo "<tr>"; echo "<td>"; while($mhealth>0 and $phealth>0) { if ($pspeed > $mspeed) { $pspeed = $pspeed - $mspeed; $pdmg = rand($pattacklow,$pattack); $mhealth = $mhealth - $pdmg; $ptotaldmg = ($ptotaldmg + $pdmg); } else { $mspeed = $mspeed - $pspeed; $mdmg = rand($mattacklow,$mattack); $phealth = $phealth - $mdmg; $mtotaldmg = ($mtotaldmg + $mdmg); } $pspeed += 1000; $mspeed += 1000; if ($phealth < 1) { echo "<font color=red>You did $ptotaldmg damage while taking $mtotaldmg damage and were defeated by $enemyuid losing $lostgold Gold and gaining no Skill !!!</font><br>"; $mskill = rand(1,3); if ($mskill==1) { $mevade += 1; $magility += 1; $pstamina += 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET evade = '$mevade' WHERE uidskills = '$enemyuid'"); mysql_query("UPDATE skills SET agility = '$magility' WHERE uidskills = '$enemyuid'"); } elseif ($mskill==2) { $mevade += 1; $mdexterity += 1; $pstamina += 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET evade = '$mevade' WHERE uidskills = '$enemyuid'"); mysql_query("UPDATE skills SET dexterity = '$mdexterity' WHERE uidskills = '$enemyuid'"); } else { $mdexterity += 1; $magility += 1; $pstamina += 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET agility = '$magility' WHERE uidskills = '$enemyuid'"); mysql_query("UPDATE skills SET dexterity = '$mdexterity' WHERE uidskills = '$enemyuid'"); } } elseif ($mhealth < 1) { echo "<font color=green>You did $ptotaldmg damage while taking $mtotaldmg damage and defeated $enemyuid gaining $gainedgold Gold and the skills below !!!</font><br>"; $pskill = rand(1,3); if ($pskill==1) { $mevade += 2; $pbarehands += 1; $pstrength += 1; $pstamina -= 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET barehands = '$pbarehands' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET strength = '$pstrength' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET evade = '$mevade' WHERE uidskills = '$enemyuid'"); echo "<font color=green>Your Barehands Skill has increased by 1 point! Barehands Skill is Now $pbarehands</font><br>"; echo "<font color=green>Your Strength Skill has increased by 1 point! Strength Skill is Now $pstrength</font>"; } elseif ($pskill==2) { $mevade += 2; $pbarehands += 1; $pdexterity += 1; $pstamina -= 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET evade = '$mevade' WHERE uidskills = '$enemyuid'"); mysql_query("UPDATE skills SET barehands = '$pbarehands' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET dexterity = '$pdexterity' WHERE uidskills = '$_SESSION[uid]'"); echo "<font color=green>Your Barehands Skill has increased by 1 point! Barehands Skill is Now $pbarehands</font><br>"; echo "<font color=green>Your Dexterity Skill has increased by 1 point! Dexterity Skill is Now $pdexterity</font>"; } else { $mevade += 2; $pagility += 1; $pstrength += 1; $pstamina -= 1; mysql_query("UPDATE skills SET stamina = '$pstamina' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET evade = '$mevade' WHERE uidskills = '$enemyuid'"); mysql_query("UPDATE skills SET agility = '$pagility' WHERE uidskills = '$_SESSION[uid]'"); mysql_query("UPDATE skills SET strength = '$pstrength' WHERE uidskills = '$_SESSION[uid]'"); echo "<font color=green>Your Agility Skill has increased by 1 point! Agility Skill is Now $pagility</font><br>"; echo "<font color=green>Your Strength Skill has increased by 1 point! Strength Skill is Now $pstrength</font>"; } } } echo "</td>"; echo "</tr>"; echo "</table>"; } else { echo "<font color=red>You do not have enough Stamina to perform the action requested Please explore for more or wait until tommorrow</font>"; } //Attack another player below this line $result = mysql_query("SELECT * FROM users ORDER BY username"); echo "<table width='750' border='1'> <tr> <th>Player's Name</th> <th>Level</th> <th>Title</th> <th>Guild Name</th> <th>Action</th> </tr>"; while($row = mysql_fetch_array($result)) { echo "<tr>"; echo "<td>" . $row['username'] . "</td>"; echo "<td>" . $row['lvl'] . "</td>"; echo "<td>" . $row['title'] . "</td>"; echo "<td>" . $row['guild'] . "</td>"; echo "<td>" . "<br><center><form action=battleplayer.php method=post><input type=hidden name=uidenemy value=$row[username]><input type=submit value=Attack></form></center>" . "</td>"; echo "</tr>"; } echo "</table>"; Code (markup):
You don't have to make a while for fetching the mysql data. You use a while when you get multiple rows from the database, and since you're looking up a unique ID, that wont be nessesary. Instead, just use: $rowp = mysql_fetch_array($player); $phealth = $rowp['health']; $pagility = $rowp['agility']; $pdexterity = $rowp['dexterity']; $pevade = $rowp['evade']; $pbarehands = $rowp['barehands']; $pstrength = $rowp['strength']; $pstamina = $rowp['stamina']; PHP: Another thing is to secure your inputs. As far as I can see,some of the information you're looking up in the database is coming from inputs. If users makes so-called mysql injections (google: mysql injection), it can be a major leak in your site. To prevent injections, use: trim(mysql_real_escape_string($_POST[variable])); instead of just using $_POST[variable]; So, like: $enemy = mysql_query("SELECT * FROM skills WHERE uidskills='".trim(mysql_real_escape_string($_POST[uidenemy]))."'"); PHP: To further optimize the response time on your server, you can set LIMIT 1 on the queries where you're only getting info on one user. like: $enemy = mysql_query("SELECT * FROM skills WHERE uidskills='".trim(mysql_real_escape_string($_POST[uidenemy]))."' LIMIT 1"); PHP: That means, that when you have found 1 row matching to the uidskills, it wont search for more rows (you don't need to do that, because uidenemy is a unique id ). Hope this made sense to you, the most important from what I wrote above is the secure querying (preventing mysql injections), if you don't do that, users would be able to manipulate with the database like getting passwords, emails, deleting stuff or updating stuff. Even if you make an input with type="hidden", you can easily manipulate the value with the firebug plugin for firefox or developer tools in IE8 etc.
Thank you I wondered how to get around people hacking variables. Thank you for taking the time to look over my code
You're welcome. You can even make a function to secure your variables like: function secureSQL($string){ return trim(mysql_real_escape_string(addslashes($string))); } PHP: then just call it with : secureSQL($_POST[something]); That function adds slashes, so remember in your output to strip your variable with stripslashes
So if I have the code below should I save the row into a variable or is it better to just call the row when needed? $mstrength = $rowe['strength']; Code (markup): Example: echo "$mstrength"; Code (markup): or just: echo "$rowe[strength]"; Code (markup):
Well... It would be easiest to just say $rowe[strength], so don't put it into a variable. Also, when you're echoing a variable, you don't have to quote it. Just do it like: echo $variable Quotes defines a string, but that's stored inside your variable (if its not an integer, integers - numbers etc. don't need quotes too).