Sign in to follow this  
bigfooot

[web] another php problem, sry guys (solved)

Recommended Posts

okay guys, ive got another problem. im making a text-based mmorpg and i have a page where users can buy 'fans' for a certain amount of money. here is the script for the form: <? session_start(); ?> <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <title>buy fans</title> <meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1"> </head> <body> <form action="fanscheck2.php" method="post" name="" id=""> fans u want<input name="fansamount" type="text" id="fansamount"></td> <input type="submit" name="Submit" value="Submit"></td> </form> </body> </html> And here is the script to update the data in my database: <?php include 'db.php'; session_start(); echo $_SESSION['userid']; if (isset($_SESSION['userid'])) { $userid=$_SESSION['userid']; $userstatsall="SELECT * from userstats where userid='$userid'"; $userstatsall2=mysql_query($userstatsall) or die("Could not get user stats"); $userstatsall3=mysql_fetch_array($userstatsall2); if(isset($_POST['submit'])) { $fanswanted=$_POST['fansamount']; $fanswanted=strip_tags($fanswanted); $total=$fanswanted*100; if($fanswanted<0) { print "You cannot buy negative fans.."; } else if($userstatsall3['money']<$total) { print "You do not have enough money to buy that many fans."; } else if($userstatsall3['money']>=$total) { $getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userstatsall3[userid]'"; mysql_query($getfans) or die("Could not buy fans"); print "You bought $fanswanted fans. ."; } else { print "there was an error"; } } } else { print "Sorry, not logged in."; } print "test" ?> I get no error messages, but nothing comes up execpt for the user id, which i put to see if it worked and the "test". The data does not get updated in the database. Can anyone see what is wrong? thanks for your help. ps: Part of the script is copied from www.chipmunk-scripts.com's "Kill Monster" open-source game. [Edited by - bigfooot on October 22, 2005 9:56:50 PM]

Share this post


Link to post
Share on other sites
1. Put your source inside source or code tags on gamedev.net
2. Do not call variables things like $userstatsall2 and $userstatsall3
3. Error reporting. Always put

error_reporting(E_ALL);


Everywhere, otherwise important errors won't get reported.

Examine the server's error log files to see the error, if display_errors is off (which it should be in a production environment, but you may have it turned on on your dev server)

Ideally, write an error handler which crashes out loudly and in a verbose manner for any error happening at all (even a E_NOTICE). That's what I do.

4. I'm not sure that die() does what you think it does. Don't use die. Instead use user_error (which will of course invoke your loud and fatal error handler)

5. Always cast things that should be an int into an int, otherwise you might get problems with SQL injection vulnerabilities, for example:


$fanswanted = (int) $_POST['fansamount'];


Cheers

Mark

Share this post


Link to post
Share on other sites
I think you stubmled on an evil IE bug.

If you have a form with only one input and one submit, and you submit teh form by pressing the return key (instead of clicking on the button) then IE will NOT send the key/value pair attached to the button. In your case, it will only send fansamount=something to the server. It will not send submit=Submit. Hence, your script will not update the database (it responds to the submit).

You can easily check it yourself. Change the "post" on your form to "get" and look at the URL.

A fix: add an <input type="hidden" name="submit" value="1" /> to the forms that only have one input.

Share this post


Link to post
Share on other sites
hi,

I think your problem is, that you have no valid DB Connection
Param in your mysql_query() function...

you have to write:

mysql_query( $hDB, $sSQL ) or die("YOUR ERROR");


Hope this helps,

Marc

PS:

And i think, you don't havt to put $fanswanted in your SQL String in '',
(if it is an int field)

Share this post


Link to post
Share on other sites
thank you marcjulian ,

What do the $hDB, $sSQL variables mean/do/execute?? Did you use it to represent a variable from my script for example $getfans? Or is it used for something else?

alex

Share this post


Link to post
Share on other sites
the db.php looks like this:

<? 
/* Database Information - Required!! */
/* -- Configure the Variables Below --*/
$dbhost = 'localhost';
$dbusername = 'MYUSERNAME';
$dbpasswd = 'MYPASSWORD';
$database_name = 'MYDATABASENAME';

/* Database Stuff, do not modify below this line */

$connection = mysql_pconnect("$dbhost","$dbusername","$dbpasswd")
or die ("Couldn't connect to server.");

$db = mysql_select_db("$database_name", $connection)
or die("Couldn't select database.");
?>

Share this post


Link to post
Share on other sites
I don't see the text "could not buy fans" anywhere in your code... so I have no idea why that is showing up.

On another note (and I hope you don't take this the wrong way), your code is pretty ugly. Now, I myself am the king of ugly code, so i can sympathize.... but if you really want to publish this game at some point in time, I'd highly suggest working on making your code as modular as possible.

Share this post


Link to post
Share on other sites
Quote:
Original post by Cygnus_X
I don't see the text "could not buy fans" anywhere in your code... so I have no idea why that is showing up.

On another note (and I hope you don't take this the wrong way), your code is pretty ugly. Now, I myself am the king of ugly code, so i can sympathize.... but if you really want to publish this game at some point in time, I'd highly suggest working on making your code as modular as possible.


The text "could not buy fans" comes from here:



{
$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userstatsall3[userid]'";
mysql_query($getfans) or die("Could not buy fans");
print "You bought $fanswanted fans. .";
}



Lol, i know my code is ugly, im still a noob. Do you know of any tutorials/articles to making more modular code?

Share this post


Link to post
Share on other sites
Ah ha!

WHERE userid='$userstatsall3[userid]'"; <-- You can't do that

instead, try

$userid = $userstatsall3['userid'];

$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";


And I don't know any good websites on making good php code, but I do have a good book or two. PHP and Mysql web development by Luke Wellington and Laura Thomson is quite good.

But, as a general practice... anything you're going to write more than once needs to be a function. For example... all of this:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title>buy fans</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>


could be written as a single function. It would look like this:

function make_html_header($Title)
{

?>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title><?php echo $Title; ?></title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>


<?php

}

That way, all you'd need to do is call make_html_header('PageTitle') at the top of each page, and it would save you several lines of code and a ton of redundancies.

Also, you need to find better names for your variables ($userstatsall3 has no implied meaning what-so-ever). If it were just $UserStats, you'd be ok. But 4 months from now when you're going through your code.... you're not going to remember what the 3 indicates.

In addition, you need to capitalize the first letter of each word. $UserStatsAll_3 would at least be easier to read than $userstatsall3.

Last but not least, avoid nested if statements. Sometimes, its unavoidable, but (IMO) its good practice not to use them if possible. You'd be 10 times better off storing both the username and password in a session variable, creating a function called 'loggedin' that tests the username/password combo to see if its valid, then throwing an error and exiting the script if loggedin returned false.

There are about dozen or so other things I'd do differently.... but as is, someones already going to pick a fight by tomorrow mourning telling me that nested ifs are the greatest invention and that I'm out of my mind. Mostly its a matter of preference, but I'd start now with making your code as clean and as efficent (for both you and others to read) as possible before trying to expand into a 12000 line program.

Share this post


Link to post
Share on other sites
thanks for your reply, but the script still does not work. Here is the newest version of the code:


<?php
include 'db.php';
session_start();

ini_set ('display_errors', 1);
error_reporting (E_ALL & ~E_NOTICE);

echo $_SESSION['userid'];

if (isset($_SESSION['userid']))
{
$userid=$_SESSION['userid'];
$userstatsall="SELECT * from userstats where userid='$userid'";
$userstatsall2=mysql_query($userstatsall) or die("Could not get user stats");
$userstatsall3=mysql_fetch_array($userstatsall2);
$userid = $userstatsall3['userid'];
if(isset($_POST['submit']))
{
$fanswanted = (int) $_POST['fansamount'];
$fanswanted=strip_tags($fanswanted);
$total=$fanswanted*100;
if($fanswanted<0)
{
print "You cannot buy negative fans..";
}
else if($userstatsall3['money']<$total)
{
print "You do not have enough money to buy that many fans.";
}
else if($userstatsall3['money']>=$total)
{
$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";
mysql_query($getfans) or die("Could not buy fans");
print "You bought $fanswanted fans. .";
}
else
{
print "there was an error";
}
}
}
else
{
print "Sorry, not logged in.";
}

print "test"

?>


And here is the database dump, in case it is usefull:


# phpMyAdmin SQL Dump
# version 2.5.3
# http://www.phpmyadmin.net
#
# Host: localhost
# Generation Time: Oct 22, 2005 at 03:50 PM
# Server version: 4.0.15
# PHP Version: 4.3.3
#
# Database : `databasev001`
#

# --------------------------------------------------------

#
# Table structure for table `users`
#

CREATE TABLE `users` (
`userid` int(25) NOT NULL auto_increment,
`first_name` varchar(25) NOT NULL default '',
`last_name` varchar(25) NOT NULL default '',
`email_address` varchar(25) NOT NULL default '',
`username` varchar(25) NOT NULL default '',
`PASSWORD` varchar(255) NOT NULL default '',
`info` text NOT NULL,
`user_level` enum('0','1','2','3') NOT NULL default '0',
`signup_date` datetime NOT NULL default '0000-00-00 00:00:00',
`last_login` datetime NOT NULL default '0000-00-00 00:00:00',
`activated` enum('0','1') NOT NULL default '0',
PRIMARY KEY (`userid`)
) TYPE=MyISAM COMMENT='Membership Information' AUTO_INCREMENT=2 ;

#
# Dumping data for table `users`
#

INSERT INTO `users` VALUES (1, 'my name', 'my lastname', 'My email', 'Username', 'MYPASSWORD', 'MYINFO', '0', '2005-10-20 09:01:28', '2005-10-22 15:44:39', '1');

# --------------------------------------------------------

#
# Table structure for table `userstats`
#

CREATE TABLE `userstats` (
`userid` int(25) NOT NULL auto_increment,
`money` int(25) NOT NULL default '1000',
`fans` int(25) NOT NULL default '15',
PRIMARY KEY (`userid`)
) TYPE=MyISAM AUTO_INCREMENT=2 ;

#
# Dumping data for table `userstats`
#

INSERT INTO `userstats` VALUES (1, 1000, 10);

Share this post


Link to post
Share on other sites
Is it still saying "Could not buy fans" or does it give you another error?

Also, why do you have:

$fanswanted=strip_tags($fanswanted);

Isn't $fanswanted suppose to be an integer? If so, you need to have a function that checks to see if a value is an integer or not. It should return either true or false.


Also, the statment:

$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";

should not have a comma after '$fanswanted'.



Share this post


Link to post
Share on other sites
YES YES YES YES

Thank you Cygnus_X so much, it works now. Its because of the comma after the $fanswanted.

Ill neaten up the code now and stuff.

Thank you also to all the other guys who helped.

alex

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this