Can you find a problem here?

[Sarcoth]

Can you find any reason why this code would occasionally remove double the money from the user?

if ($test->input['do'] == 'ownedby') {
$Location = $test->db->query_first("select p_legend, ownedby from test_place where pid='{$test->user['current_place']}'");
if ($Location['p_legend'] == 7) { $Land_Cost = 2; $Land_Tier = 1; }
if ($Location['p_legend'] == 8) { $Land_Cost = 4; $Land_Tier = 2; }
if ($Location['p_legend'] == 9) { $Land_Cost = 6; $Land_Tier = 3; }
if ($Location['p_legend'] == 10) { $Land_Cost = 8; $Land_Tier = 4; }
if ($Location['p_legend'] == 11) { $Land_Cost = 10; $Land_Tier = 5; }
if ($Location['p_legend'] == 12) { $Land_Cost = 12; $Land_Tier = 6; }
if ($Location['p_legend'] == 14) { $Land_Cost = 14; $Land_Tier = 7; }

if ($test->user['money'] < $Land_Cost) {
$test->error($test->parse_lang($test->lang['not_enough_money_for_ts'], $test->lang['money']));
}

if ($Land_Cost > 0) {
$test->db->query("update test_user set money=money-'{$Land_Cost}' where uid='{$test->user['uid']}'");
$result = $test->db->query("SELECT uid, current_place FROM test_user where current_place='{$test->user['current_place']}'");
while($row = $test->db->fetch_array($result)) {
if ($test->user['uid'] != $row['uid']) {
$test->db->query_write("update test_user set current_place='60' where uid='{$row['uid']}'");
}
}

$test->db->query("update test_place set ownedby='{$test->user['uid']}' where pid='{$test->user['current_place']}'");
}

$test->redirect($test->lang['purchased_land'], 'test.php?' . $test->system->systemvars['session'] . 'do=place&id=' . $test->user['current_place']);
}

I'll do my best to explain if you have any questions. I've been working on this for a couple days now and everything I try still causes it to occur twice now and then.

Thanks.

[Carnage]

well, a problem i had with a game i wrote was that i was comparing the money to a value loaded when the script started part way thru the scripts execution you then update based on the value in the database which might have changed since you loaded it.

If you change your db query to update to a set value it will reduce the problem.
$test->db->query("update test_user set money=money-'{$Land_Cost}' where uid='{$test->user['uid']}'");

should be
$newmoney = $test->user['money'] - $Land_Cost;
$test->db->query("update test_user set money=$newmoney where uid='{$test->user['uid']}'");

a second way which may not be possible depending on your setup is to change your database engine to innodb and use row locking to prevent the data being changed while you are modifying it.

[Opserty]

You should think about using switch() (http://php.net/switch) here instead of all these ifstatements:

if ($Location['p_legend'] == 7) { $Land_Cost = 2; $Land_Tier = 1; }
if ($Location['p_legend'] == 8) { $Land_Cost = 4; $Land_Tier = 2; }
if ($Location['p_legend'] == 9) { $Land_Cost = 6; $Land_Tier = 3; }
if ($Location['p_legend'] == 10) { $Land_Cost = 8; $Land_Tier = 4; }
if ($Location['p_legend'] == 11) { $Land_Cost = 10; $Land_Tier = 5; }
if ($Location['p_legend'] == 12) { $Land_Cost = 12; $Land_Tier = 6; }
if ($Location['p_legend'] == 14) { $Land_Cost = 14; $Land_Tier = 7; }



Start you off:

switch($Location['p_legend'])
{
case 7:
$Land_Cost = 2;
$Land_Tier = 1;
break;
}


Even if you don't, it would be better to replace the if statements with elseif's after the first if since $Location['p_legend'] can only have one value.

[Sarcoth]

well, a problem i had with a game i wrote was that i was comparing the money to a value loaded when the script started part way thru the scripts execution you then update based on the value in the database which might have changed since you loaded it.

If you change your db query to update to a set value it will reduce the problem.
$test->db->query("update test_user set money=money-'{$Land_Cost}' where uid='{$test->user['uid']}'");

should be
$newmoney = $test->user['money'] - $Land_Cost;
$test->db->query("update test_user set money=$newmoney where uid='{$test->user['uid']}'");

a second way which may not be possible depending on your setup is to change your database engine to innodb and use row locking to prevent the data being changed while you are modifying it.

Carnage, your $newmoney variable seems to have done the trick. I tested it over 20 times so far and it hasn't doubled the cost at all. I'll let you know if I notice a problem with it later on. Thank you.

Opserty, thanks for the recommendation. Even though my goal is accomplished with my current setup, I'll definitely change it over to what you posted. Thanks.

[Carnage]

my eventual solution to this was a very complicated form setup whereby each form was only submittable once.

You add an extra field 'formhash' to your forms, in the php you set a session variable $_SESSION['formhashes'] which is an array of all valid hashes. So you might have something like this:

$_SESSION['formhash'] = array('formhash1'=>1,'formhash2'=>0);

formhash 2 having been used is invalidated; when the script is finished processing you can delete it entrily from the array. I generated the hashes by simply MD5ing the timestamp . a random number . their userid

If a form with an invalid formhash is submitted you reject the data. I had this builtin to my input handling class so it was seamless on the forms it was used on.