The case of the storage buttons

Gravity ist nicht unbedingt berühmt dafür, stabilen Code zu produzieren. Die Wahl der richtigen Client Version ist daher in der Praxis für die meisten reines Glücksspiel.
Ich werde oft gefragt, welche Version man nutzen sollte. Da ich persönlich weder RO aktiv spiele, noch direkt mit Servern zusammen arbeite, kann an dieser Stelle lediglich auf Harmony Feedback zurück greifen. In der Praxis hat sich gezeigt, dass eine Version von Ende März am besten läuft.

Mit der Einführung des neuen Storage kamen einige Buttons hinzu, welche die einzelnen Kategorien in einem kleineren Fenster öffnen. Über die User Experience dieses Features lässt sich streiten, aber immerhin eine Verbesserung.

GUI Innovation made in Korea

In der erwähnten “stabilen” Version funktionieren diese Buttons nun leider nicht. Gravity wäre nicht Gravity, wenn das Problem zeitnah behoben worden wäre. Es dauerte über vier Monate, bis der Bug behoben wurde.

Mir blieben also zwei Optionen, um auch weiterhin guten Gewissens Empfehlungen geben zu können:

  • Andere Versionen untersuchen, deren Eigenheiten herausfinden und nach umfassender Analyse ggf. eine andere Version weiterempfehlen.
  • Das Problem beheben.

Eigentlich eine rhetorische Frage.

  • Ich bin faul.
  • Der RO Window Manager ist mir nicht unbekannt.
  • Reverse Engineering macht Spaß!

Klare Sache.

Was ist das Problem?
Um es im üblichen Umfang von RO Bug Reports zu formulieren:

Die Storage Buttons funktionieren nicht.

Aha. Um verwertbare Informationen über das Problem zu finden, haben sich einige Fragestellungen in der Vergangenheit als recht nützlich herausgestellt:

  • Welches Verhalten wird erwartet?
    Die sieben Buttons öffnen kleine Fenster im Stile eines einspaltigen Inventories. Die Fenstertitel entsprechen den Item-Kategorien, wie sie auch am linken Rand des Storage Fensters aufgelistet sind. Mehrfaches Drücken der Buttons blendet die Fenster ein oder aus.
  • Welches Verhalten wird beobachtet?
    Die Buttons bewirken entweder gar nichts, öffnen die falschen Fenster (Escape-Menü, korrektes Fenster mit falschem Titel) oder führen zum Absturz des Clients.

      Oft verschweigen Spieler Details! Warum sollte man die Message Box erwähnen, die 5 Sekunden vor dem Crash aufpoppte…?
  • Ist das Problem konstant reproduzierbar? Äußert es sich immer gleich?
    Ja.

      Ein entscheidender Faktor. Probleme, die nur sporadisch auftreten deuten auf Race Conditions oder (meistens) Memory Corruption hin. Diese Probleme sind weitaus komplexer, da oft keine direkte Verbindung zwischen Ursache und Wirkung besteht.
  • Tritt das Problem nur bei vereinzelten Personen auf?
    Nein.

      Im RO Bereich wird oft einen Ordner für eine Vielzahl verschiedener Server verwendet. Dass es damit zwangsläufig zu Problemen kommt, ist leider noch nicht in den Köpfen der Spieler angekommen… Standardantwort: Erneut in einen leeren Ordner installieren
  • Hat Gravity kürzlich Änderungen vorgenommen, die mit dem Problem in Verbindung stehen könnten?
    Nein.

      Oft entstehen Probleme daraus, dass Gravity interne Formate ändert und somit alte Exe Versionen mit dem aktuellen Content nicht mehr kompatibel sind. Einer der Hauptgründe, warum ich kRO Updates nicht den Spielern überlassen würde. Wer neuen Content braucht, kann ihn über eigene Patchserver verteilen.

Wie löst man das Problem?
Der entscheidende Hinweis ist, dass sich unter Umständen falsche Fenster öffnen.

In RO hat jedes Fenster eine konstante ID. Das Pet-Fenster hat beispielsweise die ID 88, das Storage die 33. Wenn falsche Fenster geöffnet werden, könnte das doch daran liegen, dass eine falsche ID übergeben wird…?
Zum Vergleich habe ich eine neuere Version heran gezogen, wo der Bug bereits behoben ist. Und tatsächlich – der Code offenbart Unterschiede:

Version mit Bug

Version ohne Bug

Ein Blick auf die Switch Table der MakeWindow Funktion bestätigt, dass in beiden Versionen der Bereich von 146 bis 152 verwendet wird.
Ganz klar! Passen wir die fehlerhafte Version doch einfach an, dass sie auch 146 addiert. Und siehe da: kein Crash mehr! :D

Aber wirklich funktioniert tut es noch immer nicht. Es passiert einfach gar nichts. Hmpf.

Schauen wir uns also mal an, was beim Öffnen der Fenster passiert:
(eax ist in beiden Snippets die ID des Fensters)

2010-03-24:

loc_505202: ; jumptable 00504456 cases 146-152
mov ebx, [esi+eax*4+12]
cmp ebx, edi
jnz loc_50934E

2010-07-21:

loc_50B2FC: ; jumptable 0050A556 cases 146-152
add eax, 0FFFFFF6Eh
mov [ebp+var_18], eax
lea eax, [esi+eax*4+270h]
mov [ebp+var_14], eax
mov ebx, [eax]
cmp ebx, edi
jnz loc_50F5E7

2010-07-21 (vereinfacht):

loc_50B2FC: ; jumptable 0050A556 cases 146-152
add eax, 0FFFFFF6Eh
mov ebx, [esi+eax*4+624]
cmp ebx, edi
jnz loc_50F5E7

Sieht ähnlich aus, aber nicht identisch. Nehmen wir den zweiten Codeblock mal auseinander:

add eax, 0FFFFFF6Eh

Auf die ID des Fensters wird ein Wert addiert. Ein verdammt großer Wert.
Die Intel Architektur unterscheidet beim Addieren nicht zwischen positiven und negativen Zahlen – man muss einfach eine Zahl addieren, die zu groß ist. Durch den entstehenden Überlauf verkleinert sich dann der Wert (siehe auch Zweierkomplement).
Formen wir mal ein wenig um:

add eax, -146

Könnte man da nicht…?

ebx = esi + (eax - 146) * 4 + 624
ebx = esi + eax * 4 - 146 * 4 + 624
ebx = esi + eax * 4 + 40

Schonmal interessant. Der Code ist fast identisch – der fehlerhafte Code addiert aber 28 “zu viel”. Wir haben aber eben die Fenster ID (mit der wir hier noch immer rechnen!) um 7 verkleinert:

ebx = esi + eax * 4 + 40
ebx = esi + (eax + 7) * 4 + 12

Das würde heißen: Es wird überall die erhöhte Fenster ID verwendet. Lediglich die Switch Table der MakeWindow Funktion erwartet einen kleineren Wert. Wie soll das zustande kommen?

Gravity verwendet für die Fenster IDs Enums:

enum WINDOWID {
WID_BASICINFOWND = 0,
WID_CHATWND,
WID_SELECTSERVERWND,
WID_LOGINWND,
WID_MAKECHARWND,
WID_SELECTCHARWND,
WID_WAITWND,
WID_LOADINGWND,
WID_ITEMWND,
WID_TOOLTIPWND,
...
WID_LAST
}

Meine Vermutung: An einer Stelle wurde für die Storage-Fenster statt des Enums der entsprechende Zahlwert verwendet. Solang sich innerhalb des Enums nichts ändert, funktioniert das auch. Aber sobald im Enum ein Wert hinzugefügt oder entfernt wird verschieben sich alle Werte innerhalb des Enums. Was lehrt uns das?
Ein Enum sollte ein Enum bleiben! Zahlwerte mit Enums zu verwenden führt früher oder später zu Fehlern.

Mein Lösungsweg: Vor dem Aufruf von MakeWindow wird die ID um 7 verringert. Die Switch Table wird dann so angepasst, dass erst ein kleiner Codeblock aufgerufen wird, der die ID wieder um 7 erhöht. Man hätte auch die Switch Table verschieben können, aber damit würde man unter Umständen andere Fenster überschreiben.

Ich will den entstandenen Fix natürlich niemandem vorenthalten: Click me

One Response to “The case of the storage buttons

  1. Nae says:

    cranker shit :3

Leave a Reply