Ladder System Logical Errors

[garrensilverwing]

A friend of mine has been working on this code for a ladder/ranking system for this old school game we play. It works almost perfectly but there are some logical errors. I have been going through the code, tweaking what I can to try and get rid of the logical errors temporarily but it is not working. Some errors, for example, are people having a streak of 0, which should be impossible. Another problem is people will have a record of 4 wins and 4 losses but have only played 4 games. I was wondering if anyone wanted to help me find these logic errors and fix them. My head and eyes are starting to hurt looking at this code so help would be greatly appreciated!

<?php
     function Streak($Streak,$Result)
        {
            if($Streak > -1 && $Result == 1)$Streak = $Streak + 1;
            if($Streak < 1 && $Result == 0)$Streak = $Streak - 1;
            if($Streak < 1 && $Result == 1) $Streak = 1;
            if($Streak > -1 && $Result == 0) $Streak = -1;
            if($Streak == 0 && $Result == 1) $Streak = 1;
            if($Streak == 0 && $Result == 0) $Streak = -1;
            return $Streak;
        }
     function insertLadderGame($GameID, $map, $Winner, $Loser, $DateT)
     {
        $WResult = mysql_query("select * from standings where Player LIKE '$Winner'");
        $LResult = mysql_query("select * from standings where Player LIKE '$Loser'");
        if (mysql_num_rows($WResult) == 0 || mysql_num_rows($LResult) == 0)        //If either player is new
            {
                if (mysql_num_rows($WResult) == 0)        //If the winner is new
                    {
                        $New=1;
                        $result = mysql_query("SELECT Rank, Wins, Streak FROM standings order by Rank desc limit 1") or die(mysql_error());
                        $row = mysql_fetch_array($result);
                        $HighestRank = $row['Rank'];
                        $WOldRank = $WNewRank = $HighestRank + 1;
                        $Wins = 1;
                        $Losses = 0;
                        $Streak = 1;
                        $Series=0;
                        UpdatePlayer($New,$Winner,$WNewRank,$Wins,$Losses,$Streak,$WNewRank,$Series);
                    }
                 else        //If the winner is NOT new
                     {
                        $New = 0;
                        $Result = 1;
                        $row = mysql_fetch_array($WResult);
                        $WOldRank = $WNewRank = $row['Rank'];
                        $Wins = $row['Wins'] + 1;
                        $Losses = $row['Losses'];
                        $WStreak = $row['Streak'];
                        $WStreak = Streak($WStreak,$Result);
                        $HighRank = $row['HighRank'];
                        $Series = $row['series'];
                        if($WOldRank < $HighRank) $HighRank = $WOldRank;
                        UpdatePlayer($New,$Winner,$WNewRank,$Wins,$Losses,$WStreak,$HighRank,$Series);
                     }
                 if (mysql_num_rows($LResult) == 0)        //If the loser is new
                    {
                        $New = 1;
                        $result = mysql_query("SELECT Rank, Wins, Streak FROM standings order by Rank desc limit 1") or die(mysql_error());
                        $row = mysql_fetch_array($result);
                        $HighestRank = $row['Rank'];
                        $LOldRank = $LNewRank = $HighestRank + 1;
                        $Wins = 0;
                        $Losses = 1;
                        $Streak = -1;
                        $Series=0;
                        UpdatePlayer($New,$Loser,$LNewRank,$Wins,$Losses,$Streak,$LNewRank,$Series);
                    }
                 else        //If the loser is NOT new
                    {
                        $New = 0;
                        $Result = 0;
                        $row = mysql_fetch_array($LResult);
                        $LOldRank = $LNewRank = $row['Rank'];
                        $Wins = $row['Wins'];
                        $Losses = $row['Losses'] + 1;
                        $LStreak = $row['Streak'];
                        $LStreak = Streak($LStreak,$Result);
                        $HighRank = $row['HighRank'];
                        $Series = $row['series'];
                        UpdatePlayer($New,$Winner,$WNewRank,$Wins,$Losses,$WStreak,$HighRank,$Series);
                    }            
                 mysql_query("INSERT INTO checkedgames (GameID,Winner,Loser,WOldRank,WNewRank,LOldRank,LNewRank,Map,DateT)
                 VALUES('$GameID', '$Winner','$Loser','$WOldRank','$WNewRank','$LOldRank','$LNewRank','$map','$DateT' );") or die(mysql_error());
                 return 0;
            }
        else
            {
            // If neither Player is new
                $New = 0;
                
            //Winner
                $row = mysql_fetch_array($WResult);
                $Result = 1;
                $WHighRank = $row['HighRank'];
                $WOldRank = $row['Rank'];
                $WWins = $row['Wins'] + 1;
                $WLosses = $row['Losses'];
                $WStreak = Streak($row['Streak'],$Result);
                $WSeries = $row['series'];

            //Loser
                $row = mysql_fetch_array($LResult);
                $Result = 0;
                $LOldRank = $row['Rank'];
                $LHighRank = $row['HighRank'];
                $LWins = $row['Wins'];
                $LLosses = $row['Losses'] + 1;
                $LStreak = Streak($row['Streak'],$Result);
                $LSeries = $row['series'];

                if ($LOldRank + $WOldRank == 3) $WSeries = $WSeries + 1;
                if ($WSeries > 0)
                    {
                        if ($WSeries == 3)
                             {
                                mysql_query("UPDATE standings SET Rank=Rank-1 WHERE Rank BETWEEN 3 AND 10");
                                $SeriesLoser = $Loser;
                                $WSeries = 0;
                                $LSeries = 0;
                             }
                        else
                            {
                                $WNewRank = $WOldRank;
                                $LNewRank = $LOldRank;
                            }
                    }
                if ($WOldRank > $LOldRank)        //If the winner has a lower standing than the loser
                    {
                        if($WSeries == 0)
                            {
                            $WNewRank = (int)(($WOldRank + $LOldRank)/2);            
                            if ($WNewRank == 2)
                                {
                                    $LSeries = 0;
                                    mysql_query("UPDATE standings SET series='0' WHERE Rank=1") or die(mysql_error());
                                }
                                mysql_query("UPDATE standings SET Rank=Rank+1 WHERE Rank BETWEEN $WNewRank AND $WOldRank-1") or die(mysql_error());
                            }
                    }
                else 
                    {
                        $WNewRank = $WOldRank;
                        $LNewRank = $LOldRank;
                    }
                if ($WNewRank < $HighRank) $HighRank = $WNewRank;
                if ($WNewRank == $LOldRank) $LNewRank = $LOldRank + 1;
                else $LNewRank = $LOldRank;
                UpdatePlayer($New,$Winner,$WNewRank,$WWins,$WLosses,$WStreak,$WHighRank,$WSeries);
                UpdatePlayer($New,$Loser,$LNewRank,$LWins,$LLosses,$LStreak,$LHighRank,$LSeries);
                if($SeriesLoser)
                    {
                        $LNewRank = 10;
                        mysql_query("UPDATE standings SET Rank = $LNewRank WHERE Player LIKE '$SeriesLoser'") or die(mysql_error());
                    }
                $result = mysql_query("SELECT Rank, HighRank, Player FROM standings") or die(mysql_error());
                while($row = mysql_fetch_array($result))
                    {
                        $Rank = $row['Rank'];
                        $HighRank = $row['HighRank'];
                        $Player = $row['Player'];
                        if($Rank < $HighRank)
                            {
                                mysql_query("UPDATE standings SET HighRank=$Rank WHERE Player LIKE '$Player'") or die(mysql_error());
                            }
                    }    
                mysql_query("INSERT INTO checkedgames (GameID,Loser,Winner,WOldRank,WNewRank,LOldRank,LNewRank,map,DateT) VALUES('$GameID', '$Loser','$Winner','$WOldRank','$WNewRank','$LOldRank','$LNewRank','$map','$DateT');") or die(mysql_error());
                return 0;
            }
     }
    function UpdatePlayer($New,$Player,$Rank,$Wins,$Losses,$Streak,$HighRank,$Series)
        {
            if($New == 1)
                {
                    mysql_query("INSERT INTO standings (Player,Rank,Wins,Losses,Streak,HighRank,series) VALUES('$Player','$Rank','$Wins','$Losses','$Streak','$HighRank','$Series');") or die(mysql_error());
                }
            else
                {
                    mysql_query("UPDATE standings SET Rank='$Rank', Wins='$Wins', Losses='$Losses', Streak='$Streak', HighRank='$HighRank',  series='$Series' where Player LIKE '$Player'") or die(mysql_error());
                }
        }

 

[misson]

Some errors, for example, are people having a streak of 0, which should be impossible. Another problem is people will have a record of 4 wins and 4 losses but have only played 4 games. I was wondering if anyone wanted to help me find these logic errors and fix them.

Please post sample data that demonstrates this behavior. You could also add some scaffolding code to print intermediate results and inserted values so you can get a better idea of what's going wrong. Better than scaffolding would be to use a PHP debugger to observe variables.

My head and eyes are starting to hurt looking at this code so help would be greatly appreciated!

Keep breaking down the large function into tasks, and placing those tasks in separate functions/methods (should you switch to OOP). It will only get easier on the eyes.

Also, the blocks handling new players are still too long, and the player insert/update functions take too many parameters (4 would be pushing it, and you've got 8). Your UpdatePlayer function doesn't gain you much, though it is the beginning of a data access layer. There's a reason I implemented my versions in a certain way. If you don't want to use their return values as arrays, you can use the list construct:

function insertNewPlayer($player, $win) {
    $result = mysql_query("SELECT Rank FROM standings ORDER BY Rank DESC LIMIT 1") 
        or die(mysql_error());
    $row = mysql_fetch_array($result);
    ++$row['Rank'];
    $standing = array('oldRank' => $row['Rank'], 'newRank' => $row['Rank'], 'Player' => $player);
    if ($win) {
        $standing['Wins'] = $standing['Streak'] = 1;
        $standing['Losses'] = 0;
    } else {
        $standing['Streak'] = -1;
        $standing['Wins'] = 0;
        $standing['Losses'] = 1;
    }

    // TODO: switch to PDO and prepared queries
    if (! mysql_query("INSERT INTO standings (Player,Rank,Wins,Losses,HighRank,Streak) 
                 VALUES('$player','$standing[newRank]',$standing[Wins],$standing[Losses],'$standing[newRank]','$standing[Streak]');")) 
    {
        throw new DBException(mysql_error(), mysql_errno());
    }
    return $standing;
}
...
    list($WOldRank, $WNewRank, $Winner, $WStreak, $WWins, $WLosses) = insertWinner($Winner);

[php]
<?php
     function Streak($Streak,$Result)
        {
            if($Streak > -1 && $Result == 1)$Streak = $Streak + 1;
            if($Streak < 1 && $Result == 0)$Streak = $Streak - 1;
       ...

Make better use of "else" blocks. Also, "$Result" isn't a very descriptive name. Based on the name alone, it could hold an arbitrary number of values, such as -1 for loss, 1 for win and 0 for draw (not that you're recording the draws right now, but perhaps some other function does something with draw games). For another, data returned from a database is also called a "result" (for this reason, I suggest changing "$Result" to (e.g.) "$Outcome" throughout the code). $Won is a litter clearer, though it could conceivably hold the number of consecutive wins since the streak was recorded. The above is equivalent to:

function Streak($Streak,$Won) {
    if ($Won) {
        if ($Streak >= 0) ++$Streak;
        else $Streak = 1;
    } else {
        if ($Streak <= 0) --$Streak;
        else $Streak = -1;
    }
    return $Streak;
}

A third naming option is to call the parameter $Lost and swap the if and else blocks. Alternatively, we could use the win=1/loss=-1/draw=0 representation for outcomes and use the following function:

/* $Outcome is < 0 for losses, > 0 for wins; abs($Outcome) is the number of 
 * consecutive wins or losses. Doesn't handle draws.
 */
function Streak($Streak,$Outcome) {
    if ($Streak * $Outcome >= 0) $Streak += $Outcome;
    else $Streak = $Outcome;
    return $Streak;
}

        $result = mysql_query("SELECT Rank, HighRank, Player FROM standings") or die(mysql_error());
        while($row = mysql_fetch_array($result)) {
            $Rank = $row['Rank'];
            $HighRank = $row['HighRank'];
            $Player = $row['Player'];
            if($Rank < $HighRank) {
                mysql_query("UPDATE standings SET HighRank=$Rank WHERE Player LIKE '$Player'") or die(mysql_error());
            }
        }

I believe you want the test to be "$Rank > $HighRank". It doesn't matter anyway, because this while loop is equivalent to:

UPDATE standings SET HighRank=Rank WHERE HighRank < Rank

Even that doesn't matter, because there's only one player whose HighRank will need updating: the winner. You could take cake of the winner's highest ranking here, at the end of insertLadderGame, but the proper place to do it is in the code that updates a player's standings, the updatePlayer function, since ensuring HighRank >= Rank is a concern of the data access layer.

 

[garrensilverwing]

thanks misson, unfortunately i'm going out of town for like a week so i wont be able to implement your fixes for a little while, i think i know what you mean when you say scaffolding and whatever but i'll have to look it up, i can definitely try and find some examples to show you

 

[misson]

"Scaffolding" is any code you write as a development aid, which gets deactivated/taken out of the production version. The most common scaffolding are asserts and statements to print variables.