kantoks / skrupel

http://www.skrupel.de
19 stars 11 forks source link

Datenbankanbindung, Sql Abfragen, und weitere seltsame Sachen :-) #80

Open VapingSkull opened 1 week ago

VapingSkull commented 1 week ago

hi zusammen,

das ist zwar keine Melung eines Bugs, aber mir sind einige Dinge beim durchsehen der Skripte ins Auge gefallen. Als erstes mal Danke an den Ersteller des Skriptes und an das zur Verfügung stellen selbiges. Ich mochte das Spiel immer, und habe mich gewundert das es nicht weiterentwickelt wird.

Die Datenbank und dessen Anbindung ans Skript. ich arbeite seit einiger Zeit mit AdoDB und habe hier einige Sachen gefunden die ich nicht verstehe. Zum einen werden hier hunderte Abfragen an die DB gesendet beim Aufruf von inc.host.php (ist nur ein expemplarisches Beispiel) durchgeführt die man mit left Joins usw. zusammenfassen könnte. Dann dieses ewige mysql_data_seek dessen Funktion eigentlich dazu da war um eine bestimmte Zeile aus mehreren Zeilen herauszupicken. Skriptauszug :

$zeiger3 = mysql_query("SELECT * FROM skrupel_huellen where baid=$baid and spiel=$spiel and klasse=$schiffbau_klasse order by id");
                $huellenanzahl = mysql_num_rows($zeiger3);
                if($huellenanzahl>0){
                    $neueschiffe++;
                    $ok = mysql_data_seek($zeiger3,0);
                    $array3 = mysql_fetch_array($zeiger3);

ich erkenne den Sinn nicht dahinter. Soweit ich mich erinnere beeendet man das Statement nicht mit einem ; innerhalb des statements. HIer ist es nicht der Fall, aber bei sehr sehr vielen abfragen steht es so:

SELECT * FROM skrupel_huellen where baid=$baid and spiel=$spiel and klasse=$schiffbau_klasse order by id;

eigentlich sollte es ansatzweise so aussehen :

$sql = "SELECT * FROM skrupel_huellen where baid='".$baid."' and spiel='".$spiel."' and klasse=."'$schiffbau_klasse."' order by id";

Und erst dann führt man das SQL Statement aus. Dann wird in 98% der Fälle immer ein select * durchgeführt obwohl man nur die ID haben möchte. Warum ? Dann fehlen auf die Abgefragten Felder Indexe in der Datenbank. Es wird somit immer ein Fulltablescan ausgeführt. Das dauert viel zu lange.

Ich bin derzeit dabei das Skript umzuschreiben auf PHP 8.x. Datenbankanbing über AdoDB, und als TemplateEnging nutze ich Smarty 5.x

Was ich bereits gemacht habe, bzw. das ist noch in Arbeit , sämtlich Sprachdateien stehen bei mir nun in der Datenbank und diese lese ich per Function pro Seite aus. Es werden nur die genommen die benötigt werden. Dieses zusammenschnippeln von Sprachdateien fällt somit weg und beschleunigt das ganze. Das ganze werde ich auch noch mit Rassen usw. machen. Diese Dateiaktionen bremsen das Skript aus. Ebenso die Framesets, das wird wegfallen und durch Ajax ersetzt. Was ebenfalls wegfällt ist der Bilderpfad. Dieser steht nun als constante in der Config und wird nicht per get oder Post übertragen.

Diese Funktionen hier :

function int_post($key)
{
    if (isset($_POST[$key]) && is_numeric($_POST[$key])) {
        return intval($_POST[$key]);
    }

    return false;
}

function int_get($key)
{
    if (isset($_GET[$key]) && is_numeric($_GET[$key])) {
        return intval($_GET[$key]);
    }

    return false;
}

function str_post($key, $mode)
{
    if (isset($_POST[$key]) && !is_array($_POST[$key])) {
        $nl2br = $mode == 'SQLSAFE' && ($key == 'thema' || $key == 'beitrag' || $key == 'offenbarung');

        return safe_strval($_POST[$key], $mode, $nl2br);
    }

    return false;
}

function str_get($key, $mode)
{
    if (isset($_GET[$key]) && !is_array($_GET[$key])) {
        $nl2br = $mode == 'SQLSAFE' && ($key == 'thema' || $key == 'beitrag' || $key == 'offenbarung');

        return safe_strval($_GET[$key], $mode, $nl2br);
    }

    return false;
}

function safe_strval($value, $mode, $nl2br = false)
{
    switch ($mode){
        case 'NONE':
            return $value;
            break;

        case 'SQLSAFE':
            $retvar = stripslashes($value);
            if ($nl2br) {
                $retvar = nl2br($retvar);
            }
            //$retvar = strtr($retvar, array("\x00" => "\\x00", "\x1a" => "\\x1a", "\n" => "\\n", "\r" => "\\r", "\\" => "\\\\", "'" => "\'", "\"" => "\\\"")); // nur escapen
            $retvar = strtr($retvar, array("\x00" => '\\x00', "\x1a" => '\\x1a', "\n" => '\\n', "\r" => '\\r', '\\' => '', "'" => '', '"' => '')); // entfernt: " ' \
            return $retvar;
            break;

        case 'SHORTNAME':
            if (!preg_match('/[^0-9A-Za-z_]/', $value)) {
                return $value;
            }
            break;

        case 'PATHNAME':
            if (!preg_match('/[^0-9A-Za-z_\/\.:\-]/', $value)) {
                return $value;
            }
            break;

        default:
            if (!preg_match('/[^0-9A-Za-z_&:;\-]/', $value)) {
                return $value;
            }
    }

    return false;
}

fallen weg. Wenn ich zahlen aus der Datenbank hole, brauche ich beim zurückschreiben der gleichen Zahlen nicht prüfen ob es zahlen sind. Da die DB Anbindung über AdoDB stattfindet fallen diese Konstrukte wie SQLSAFE usw. weg.

Da ich eine Funktion habe die prüft ob get,post usw. genutzt wird, fällt dies ebenfalls weg.

Was ich ebenfalls noch nicht verstanden habe ist weshalb die DB Tabellen in der config stehen ? Folgendes habe ich was das angeht getan : Eine Constante in die Config geschrieben. Diese sieht so aus :

define ('table_prefix', 'skrupel_');

Diese bei allen DB Abfragen eingefügt.

$sql = "select id from " . table_prefix . "schiffe where id ='".$shid."'";
$meinSchiff = $db->getOne($sql);

Dient als Beispiel

Die Tabellenstrukturen sollte man definitiv überarbeiten. In manchen Tabellen steht für für das Datum varchar(10) als typ und es stehen nur zahlen drin. Datenbanken können besser mit Zahlen umgehen, behandeln aber Varchar Felder text. Also umgändert auf int. Und schon kann man vernünftig suchen bzw. sortieren .

Das sind nur einige KLeinigkeiten , aber diese sammeln sich.

Da ich eigentlich kein Git habe, und noch nicht weiß wie ich hier ein Projekt forken kann und dann weiter arbeiten kann, habe ich den ganzen KRam bei mir lokal über SVN laufen.

Vielleicht hat ja jemand Tips für mich. :-)

VapingSkull commented 6 days ago

Da das forken nicht funktioniert hat, habe ich ein Repo eröffnet und schreibe meinen Kram da rein :-D

secondtruth commented 3 days ago

Hi @VapingSkull,

sehr cool, dass du Skrupel modernisieren und mit PHP 8 nutzbar machen möchtest. Das hat das Spiel absolut verdient.

Ich würde jedoch Twig statt Smarty als Template Engine empfehlen, das ist etwas moderner. Oder alternativ Latte, denn PHP-Templates lassen sich sehr schnell in dessen Syntax migrieren.