Jump to content

Help with a script please going insane!


ddmbest88

Recommended Posts

Hello guys and thank in advance for help.

 

I have a custom game server  and i am building maps that shows wheter a fortress has been conquered and by who and show the correct faction icon upon the map. here is my script:

<?php
include_once 'dbconnectgs.php'; //creating a connection with the database
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<link rel="stylesheet" href="styles/eye.css">
</head>
<body>
  <?php 
  $Divine=mysql_query("SELECT id,race FROM siege_locations WHERE id = '1011'");// selecting the race of the conquering faction from the table of the fortresses where the id of the fortress is 1011
  ?>
  <div id ="Divine">
	<?php $row = mysql_fetch_row($Divine); //fetching data
	//echo $row[1] showing resoult just to be sure that data are ok: it shows correct resoult BALAUR
	if ($row = "ELYOS") { //conditions to show the icon, i have 3 options: faction 1 faction 2 or computer faction 3
    echo "<img src=\"http://someaddress.me/sieges/img/fortezze/elyos.png\" border=0>";
	} elseif ($row = "ASMODIANS") {
    echo "<img src=\"http://someaddress.me/sieges/img/fortezze/asmodian.png\" border=0>";
	} elseif ($row = "BALAUR") {
    echo "<img src=\"http://someaddress.me/sieges/img/fortezze/balaur.png\" border=0>";
	}
?> 	
	</div>
 </body>
</html>

Now problem is that even if the fetch shows the fortress as currently conquered by faction 3 (balaur) the if condition thinks that it's always faction 1 (elyos), and shows faction 1 logo, where do i mess up?

Edited by ddmbest88
Link to comment
Share on other sites

one = is an assignment operator. two  == is a comparison operator. the conditional tests in your code are assigning values to $row, then testing the value that was just assigned. your code is not testing the value in $row.

 

also, $row is an array, from the msyql_fetch_row() statement (the mysql_ functions are obsolete and have been removed from the version of php that was just released, you should be using PDO or msyqli_ statements.) it will never be equal to those string values. you would need to test a specific element in the array in $row.  in general, you should use an associative fetch statement, so that your code is more readable, and more maintainable. using $row['race'], instead of $row[1], will make it easier to write and read your code, both for you and for anyone else that must deal with it.

 

lastly, DRY (Don't Repeat Yourself.) your code should not repeat program logic and html markup over and over, when just a small part of it is the dynamic/changing part. ideally your mapping of race values to image base names should be one to one, with the same image base name as the race value. this would eliminate all the if/elseif logic. you would have just one line of code - 

echo "<img src=\"http://someaddress.me/sieges/img/fortezze/{$row['race']}.png\" border=0>";

if for some reason you cannot make the race value and the image base name the same, you should use a simple mapping array to give the image base name that corresponds to the race name - 

$map["ELYOS"] = 'elyos';
$map["ASMODIANS"] = 'asmodian';
$map["BALAUR"] = 'balaur';
// add any other race/image base name pairs here...

echo "<img src=\"http://someaddress.me/sieges/img/fortezze/{$map[$row['race']]}.png\" border=0>";
Edited by mac_gyver
Link to comment
Share on other sites

Some tips:

 

1. Put all your logic (i.e. PHP code) at the top of the script. Use it to assign dynamic content to variables. Then create the output (i.e. HTML) at the bottom of the script using just PHP echo statements for the content created within PHP. This separates the logic from the presentation. As you build more complicated applications you will completely separate the logic and presentation into separate files. but, for now, go ahead and use a single script.

 

2. mysql_fetch_row() will return an array into $row, but then you are testing the value against a string. They will never compare. Plus, you are returning id and race, but only looking for one value apparently. Just pull the data you need.

 

3. Instead of multiple if/elseif statements, use a switch() statement

 

4. There is no else condition. Perhaps there are no records where a value would not equal one of those options, but it is a bad design. Code should never 'assume'. Create conditions to handle unintended scenarios. 

 

5. The three conditions in your if/elseif statements should only define what is different - in this case just the src for the image tag. Right now, it is fairly simple, but since each condition will output an image, that part should not be included in the condition statements. As the output of such conditions gets more complicated it becomes all too easy to introduce bugs. For example, if you want to change the border of the image to 1 you would have to change it in three places instead of just one.

 

6. Use PDO or mysqli for database operations. mysql_ has been deprecated for years.

 

7. If the select statement will take a user defined parameter for the id, you should be using prepared statements

 

Not tested

 

<?php
 
//Set the race ID to be queried
//Can be user supplied via $_GET/$_POST
$raceID = '1101';
 
//creating a connection with the database
include_once 'dbconnectgs.php';
//Replace the contents of 'dbconnectgs.php' with the following
// - using appropriate values for host, database bname, username & password
## $db = new PDO('mysql:host=localhost;dbname=testdb;charset=utf8', 'username', 'password');
## $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
## $db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
 
//Create & run prepared statement
$query = "SELECT race FROM siege_locations WHERE id=:id LIMIT 1";
$stmt = $db->prepare($query);
$stmt->execute(array(':id' => $raceID));
//Get the race from the query results
$row = $stmt->fetchAll(PDO::FETCH_ASSOC);
$race = $row['race'];
 
//Determine correct image name
switch($race) {
    case 'ELYOS':
        $imgName = "elyos.png";
        break;
    case 'ASMODIANS':
        $imgName = "asmodian.png";
        break;
    case 'BALAUR':
        $imgName = "balaur.png";
        break;
    default:
        $imgName = "unknown.png";
}
 
//Define the full IMG tag one with just the variable image name
$raceImage = "<img src=\"http://someaddress.me/sieges/img/fortezze/{$imgName}\" border=0>";
 
?>
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<link rel="stylesheet" href="styles/eye.css">
</head>
<body>
  <div id ="Divine">
    <?php echo $raceImage; ?>
  </div>
 </body>
</html>
Link to comment
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.