Hey mein Lieber,
dachte ich nehme mir mal die Zeit und schaue über den Code, was mir dabei aufgefallen ist:
1) Prüfungen auf Ungleichheit mittels == false
- if (ch->IsGM() == false || ch->GetGMLevel() < GM_HIGH_WIZARD) {
Das ist redundant, schöner und kürzer wäre:
- if (!ch->IsGM() || ch->GetGMLevel() < GM_HIGH_WIZARD) {
2) Magic Numbers
Im Code gibt es häufig irgendwelche rumfliegenden Zahlen, wo man Variablen (Enums, Enum Classes) nutzen sollte.
- char escapePlayer[20 * 2 + 1];
sollte vermutlich sein:
- char escapePlayer[CHARACTER_NAME_MAX_LEN + 1];
3) Spieler Namen Query
Diese Query wäre nur dann erfolgreich, wenn es genau einen Spieler gibt, der sich (case insensitiv) gleich nennt.
Beispiel:
- Spieler A nennt sich 'Lead'
- Spieler B nennt sich 'lEad'
- Spieler C nennt sich 'lead'
Frage: Welchen Spieler selected die folgende Query? Bannt man Ausersehen den Falschen?
- SELECT player.account_id FROM player WHERE LOWER(name) like LOWER('%s') LIMIT 1
Es gibt hier nun verschiedene Lösungsansätze, z.B: über die Datenbank ünmöglich zu machen (über einen Constraint), dass die selben Namen genutzt werden (inkl. Groß und Kleinschreibungs-Check). Alternativ würde ich die Query auf Gleichheit prüfen und ohne LIMIT 1, von einem Usernamen wird es nur genau einen geben. Der GM muss dann auf Groß und Kleinschreibung achten.
- SELECT player.account_id FROM player WHERE name = '%s'
4) Guards für Anweisungen
- LPCHARACTER tch = d ? d->GetCharacter() : NULL;
- if (tch && tch != ch) {
- DESC_MANAGER::instance().DestroyDesc(d);
- ch->ChatPacket(CHAT_TYPE_INFO, "%s banned and kicked", escapePlayer);
- }
- else if (tch && tch == ch) {
- ch->ChatPacket(CHAT_TYPE_INFO, "cannot disconnect myself");
- return;
- }
- else {
- ch->ChatPacket(CHAT_TYPE_INFO, "%s banned", escapePlayer);
- }
Alles anzeigen
könnte man für lesbareren Code auch so schreiben (der Else-Pfad würde sowieso nie erreicht werden oder nur fälschlicherweise wenn tch = NULL wäre):
- // disconnect player
- LPDESC d = DESC_MANAGER::instance().FindByCharacterName(escapePlayer);
- LPCHARACTER tch = d ? d->GetCharacter() : NULL;
- if (!tch) {
- ch->ChatPacket(CHAT_TYPE_INFO, "a user by the name (%s) could not be found on this core!", escapePlayer);
- return;
- }
- if (tch == ch) {
- ch->ChatPacket(CHAT_TYPE_INFO, "cannot disconnect myself");
- return;
- }
-
- DESC_MANAGER::instance().DestroyDesc(d);
- ch->ChatPacket(CHAT_TYPE_INFO, "%s banned and kicked", escapePlayer);
Alles anzeigen
Ein paar finale Hinweise:
- Denk daran, dass das Bannen aktuell nicht Core-übergreifend funktioniert. Sollte der Spieler sich also rechtzeitig wegteleportieren oder ausloggen, hast du womöglich ein Problem.
- In der CInputMain::Analyze befindet sich direkt bei Eintritt folgende Prüfung:
- int32_t CInputMain::Analyze(LPDESC d, uint8_t bHeader, const char *c_pData)
- {
- LPCHARACTER ch;
- if (!(ch = d->GetCharacter()))
- {
- sys_err("no character on desc");
- d->SetPhase(PHASE_CLOSE);
- return (0);
- }
Du kannst also so Prüfungen wie "!ch" oder "!d->GetCharacter()" weglassen, außer du erwartest aus irgendwelchen Gründen, dass der User genau in dieser Milisekunde destroyed wird, in der dieser Code ausgeführt wird. Unmöglich ist nichts, aber dann haben wir größere Probleme als das 
- Nach einem 'strncpy' emphielt es sich, explizit die letzte Stelle des Arrays auf '\0' zu setzen, siehe hier:
- strncpy(szName, c_szName, GUILD_GRADE_NAME_MAX_LEN);
- szName[GUILD_GRADE_NAME_MAX_LEN] = '\0';
Weil diese Methode so funktioniert, dass wenn dein Buffer nicht groß genug ist, sie nur "so viele Bytes von src kopiert, wie möglich". Ergo, wenn dein Buffer nur 10 Stellen groß ist, du willst aber 11 kopieren, hört er bei 10 auf. Die 11. Stelle wäre aber das benötigte '\0' gewesen. Mit strlcpy hat man das Problem btw. nicht.
- Dir fehlt die Logik, wenn ein Spieler in einem Dungeon ist und der Gruppenleiter. Das kann dazu führen, dass die Gruppe den Dungeon nicht mehr beenden kann, je nachdem wie die Quest geschrieben ist.
Zum Abschluss:
Ich hoffe, dass es mehr Beiträge wie dieses gibt und das wir weiterhin gute Entwickler in der Szene haben, die so freundlich sind, uns ihre Arbeiten zur Verfügung zu stellen.
~Lead
Mit Items auf Truhe kann man sich alles nachträglich einbauen wenn man möchte. Und öffnen sollte man die Truhen nicht können alle 2-3sek da in meiner idee 2-3 Truhen maximal auf einer Map verteilt sind die zb alle halbe Stunde Respawnen nach öffnen. Daher kommt man garnicht zur möglichkeit mehrere auf einmal zu öffnen.
