Ihr habe sicher schon vom Emulatorprojekt
[Only registered and activated users can see links. Click Here To Register...] gehört. Wer den Quell-Code etwas studiert, wird teilweise abstruse Stücke darin finden, die aufgrund ihrer unglaublichen Effizienz jeden 277 mHz-PC von 1992 zu einem Hochleistungs-Flyff-Server machen (in diesem Satz könnten Spuren von Ironie enthalten sein). Um das zu erkennen, muss man nicht einmal viel von C++ verstehen (den Rest erkläre ich zur Not). Hier ein par gute Beispiele:
1. Beispiel: Game Server, ItemBank.cpp, Zeile 15:
Code:
strcpy_s(item_name,strlen("")+1,"");
Hier soll offensichtlich der String (Zeichenkette, also eine Folge von Buchstaben) geleert werden. Die erste Frage ist natürlich, wie die Variable deklariert worden ist, und dies ist die Antwort:
Code:
char item_name[75];
Auf den ersten Blick plausibel, auf den zweiten jedoch totaler Mist aus 2 Gründen: 1. ist der String auf 74 (+ Endmarkierung) Zeichen beschränkt, und 2. verschwendet das zu viel Speicherplatz. Viel gewollt, nichts hinbekommen. So würde das richtig gemacht werden:
Code:
std::string item_name;
Für die nicht-Kenner von Programmiersprachen die Erklärung: std::string ist in C++ eine Klasse für Zeichenketten dynamischer Länge. Das heißt, es wird idealerweise nur so viel Arbeitsspeicher angefordert, wie auch für den enthaltenen Text gebraucht wird. Und wenn mal viel Platz nötig ist, wird einfach viel Speicher angefordert. Ich muss wohl nicht mehr den Vorteil gegenüber
Code:
char item_name[75];
erläutern.
Zurück zur Sache: Hier wird versucht, den String zu leeren, was man auch einfacher hätte haben können:
Oder (bei Verwendung von std::string):
2. Beispiel: Game Server, Server.cpp, Zeile 102:
Code:
char buf[0x1000];
*(char*)&buf[0] = 0x5e;
Diese Stelle befindet sich am Anfang der "sendWorldServerAuth"-Methode. Deren Aufgabe ist mir nicht ganz klar (kann mich jemand aufklären?).
Jedenfalls stolpert man gleich in der 1. Zeile über eine interessante Deklaration. Hier werden sage und schreibe 4096 Bytes (0x1000 dezimal) reserviert, von denen im besten Falle 200 benutzt werden. Wieder das gleich Problem wie bei Beispiel 1: Speicherplatzverschwendung + keine Flexibilität. Ich muss noch erwähnen, dass dieser Speicherplatz bei jedem Aufruf der Methode reserviert und wieder freigegeben wird, was doch recht erlahmend wirken kann. Danach wird in den frisch erzeugten "buf" das Anfangs-Byte eines jeden Flyff-Packets geschrieben. So weit in Ordnung (obwohl man hier in jedem Fall eine Klasse für Packets verwenden sollte).
Was dann kommt, ist umso lustiger:
Code:
*(int*)&buf[1] = 0;
*(int*)&buf[5] = strlen(worldname)+4+strlen(channelname)+14+4;
*(int*)&buf[9] = 0;
*(int*)&buf[13] = -2;
*(int*)&buf[17] = 1;
*(char*)&buf[21] = 1;//chans
*(char*)&buf[22] = MySQL::config.channelid;//id
*(int*)&buf[23] = strlen(worldname);
In der zweiten Zeile dieses Ausschnittes wird anscheinend die Größe des Packets eingetragen. Anstatt diese im Nachhinein zu berechnen und einzufügen wird sich jedoch schon abgehakt, noch ehe das letzte Byte des Packets geschrieben worden ist. Auch eine Laie wird erkennen, dass soetwas absolut unübersichtlich und unsinnig ist. Außerdem belasten die ständigen Aufrufe von strlen() die CPU. Es wird doch tatsächlich zweimal die Länge desselben Strings ermittelt ("strlen(worldname)"). Es wäre ja auch zu kompliziert, diese in einer Variablen zwischenzuspeichern. Und es wird noch besser:
Code:
for(unsigned i=0;i<strlen(worldname);i++)
buf[i+27] = worldname[i];
*(int*)&buf[27+strlen(worldname)] = strlen(channelname);
for(unsigned i=0;i<strlen(channelname);i++)
buf[31+strlen(worldname)+i] = channelname[i];
*(int*)&buf[31+strlen(worldname)+strlen(channelname)] = 1440; // channel size
Herzlichen Glückwunsch an den, der hier noch durchsteigt. Erstens ist das Verwenden eines alleinstehendes "unsigned" (statt "unsigned int") schlechter Stil, zweitens hätten ein par Leerzeichen zwischen den ganze Gewusel nicht geschadet. Das gravierendste sind jedoch die ständigen strlen()-Aufrufe. Ein wenig optimiert wäre dieses Stück Code mindestens 20 mal schneller. Anstatt mich jetzt noch mehr darüber aufzuregen, komme ich lieber zum nächsten Beispiel schlechter Programmier"Kunst".
3. Beispiel: Game Server, Movers.cpp, Zeile 95:
Code:
vector<Player*> players = Players::getPlayersWhoHaveThisMoverSpawned(mover);
for(unsigned i=0;i<players.size();i++){
if(players.at(i) == (Player*)mover)
continue;
mover->unSpawnTo(players.at(i));
}
players.clear();
Dies befindet sich in der Methode unSpawnMover(). Sie soll wohl ein Objekt aus der Welt entfernen und die Änderung allen Spielern zeigen.
1. Fehler: Es wird ein std::vector erzeugt und mit allerlei Werten gefüllt. Nämlich den Spielern, die in Sichtweite des Objektes liegen. Allein schon die Idee, für jeden Spieler zu speichern, was in seiner Reichweite liegt und dies ständig für sämtliche Spieler aktualisierung zu müssen, ist mir ein Graus. Denn genau dies führt zu den Laggs und dem ständigen Respawnen von Mobs und NPCs bei Caali-Servern. Aber wie wird es richtig gemacht? Eigentlich ganz einfach. Man unterteilt die Welt in quadratische Felder. Jedes dieser Felder besitzt eine Liste mit Objekten, die darin liegen. Verändert ein Objekt seine Position, muss dies nur in wenigen umliegenden Feldern aktualisiert werden. Wenn ein Objekt aus einem Feld entfernt wird, erhalten alle Spieler in dem Feld und dessen umliegenden ein Despawn-Packet. Die meisten Spieler in der Welt bleiben also unberührt (-> kein getPlayersWhoHaveThisMoverSpawned). Dieses System wird von der offiziellen Flyff-Server-Software benutzt und wird auch von Spheron² verwendet werden.
Weiteres zum Code: In der for-Schleife wird bei jedem Aufruf players.size() aufgerufen, eine nicht unerhebliche Rechenleistung. Es wäre effizienter, die Größe einmal abzufragen und in einer Variablen zu speichern.
Der folgende Vergleich soll wohl sicherstellen, dass ein Spieler nicht bei sich selbst despawned wird. Dabei ist mit jedoch schleierhaft, wieso ein Spieler sich selbst gespawned haben sollte, denn getPlayersWhoHaveThisMoverSpawned() gibt doch wohl nicht den Spieler selbst zurück.
Dann wird die nette Methode unSpawnTo() aufgerufen. Hierzu sollte man wissen, dass diese das Packet erzeugt, das auch sogleich an den Spieler gesendet wird. Da das Despawn-Packet eines Objektes jedoch bei allen Spielern gleich aussehen wird, ist es Unsinn, dieses bei jedem Durchlauf der for-Schleife wieder zu erzeugen.
Der letzte Unfug ist players.clear(). Denn direkt im Anschluss ist die Methode zuende und das Objekt "players" wird gelöscht. Vorher noch seine Inhalte zu "clearen" ist doppelte Arbeit.
4. Beispiel: MySQLM.cpp, Zeile 180:
Code:
void MySQL::setInt64(char* table, char* wht, int id, __int64 value){
setLastAccess(GetTickCount());
char query[255];
sprintf_s(query, 255, "update %s set %s='%u' where ID='%d';", table, wht, value, id);
mysql_real_query(&flyff_db, query, strlen(query));
}
Bei Dragon/Rhisis Flyff wird tatsächlich jeder Wert eines Datensatzes in der Datenbank einzeln aktualisiert. Hier werden jegliche Regeln des gesunden Verstandes gebrochen. Um einen Charakter komplett zu speichern, bräuchte man in etwa das:
Senden des Queries
Code:
UPDATE characters SET name='[GM]noob',level='-1337',hp='0',penya='99999999' [...] WHERE id='123';
Die machen jedoch das daraus:
Senden des Queries
Code:
UPDATE characters SET name='[GM]noob' WHERE id='123';
+ Senden des Queries
Code:
UPDATE characters SET level='-1337' WHERE id='123';
+ Senden des Queries
Code:
UPDATE characters SET hp='0' WHERE id='123';
+ Senden des Queries
Code:
UPDATE characters SET penya='99999999' WHERE id='123';
Als ob MySQL nicht schon langsam genug wäre, wird hier solch ein schlechtes Auslese- und Schreibsystem benutzt. Denn beim Lesen aus der Datenbank sieht das nicht anders aus.
Außerdem befindet sich im Code noch ein Fehler, der wahrscheinlich das korrekte Speichern der EXP auf höheren Leveln verhindert:
sprintf_s(query, 255, "update %s set %s='%u' where ID='%d';", table, wht, value, id);
So wird die __int64-Variable falsch in der String geschrieben, nämlich als unsigned long (dafür steht %u). Zur Erklärung: int64 hat 8 Bytes und long nur 4, es gehen also bei Werten jenseits von ca. 4 mrd Informationen verloren. Bei den EXP ist das fatal, denn für Leveln 120 braucht man ca. 22 mrd. EXP, von denen das meiste (wenn nicht alles) durch das falsche Speichern in der Datenbank verlorengehen kann.
Richtig wäre es so:
Code:
sprintf_s(query, 255, "update %s set %s='%I64d' where ID='%d';", table, wht, value, id);
