Themabewertung:
  • 1 Bewertung(en) - 5 im Durchschnitt
  • 1
  • 2
  • 3
  • 4
  • 5
Reverse Engineering der NLT II
Nach wie vor bin ich immer wieder mal an meinem BrightEyes-Fork zu Gange. Meine Hauptmotivation ist das Reparieren von Bugs (von denen es leider furchtbar viele gibt). Gerne hätte ich auch die ganzen Bugfixes aus dem Patch von NRS mit drin. Ich stelle mir vor, nach Lust und Laune Listen von bekannten Bugs durchzugehen und zu reparieren; dabei ergibt sich dann auch gerne, dass der umliegende Code verstanden werden muss, den ich dann gleich kommentieren kann und Variablen umbenennen (falls noch nicht geschehen).

Teilweise betreffen die Bugs ja auch die Daten in der SCHICK.DAT. Momentan werden Daten "on the fly" im Quellcode repariert, z.B. diese Reparatur eines Rechtschreibfehlers.

Denkbar wäre es aber auch, sich einen Patch-Mechanismus für die SCHICK.DAT zu überlegen und das ganze als Diff-Dateien in BrightEyes zu verwalten, siehe z.B. diesen Beitrag und die darauf folgenden. Ich möchte hier also eine Diskussion darüber anstoßen, wie das ganze am bsten in BrightEyes umgesetzt werden sollte und was es dabei zu beachten gilt, damit man es hoffentlich gleich von Anfang an "richtig" macht

Persönlich tendiere ich zur gepatchten SCHICK.DAT, weil es sich sauberer anfühlt. Aber der Mechanismus muss erstmal aufgesetzt werden. Er dürfte grob aus den folgenden Teilen bestehen dürfte:
  • Entpacken und Zerlegen der SCHICK.DAT
  • Anwenden der Patches auf die Einzelteile
  • anschließendes Packen und Zusammenbauen zu der gepatchten SCHICK.DAT

Über die folgenden Punkte muss man sich wohl auch Gedanken machen:
  • Das Patchen muss abhängig von Präprozessor-Flags gestaltet sein, so dass die angewendeten Patches passend zu den im Quellcode aktivierten Modifikationen vorgenommen werden.
  • Momentan wird ja auf Textblöcke der SCHICK.DAT durch Befehle der Art get_tx(<nummer>) zugegriffen. Wenn das Reparieren eines Bugs das Erstellen weiterer Textblöcke erforderlich macht, dann verschiebt sich evtl. die <nummer>. Wie geht man damit am besten um? Fügt man die neuen Textblöcke immer hinten an, damit sich die Nummern davor nicht verschieben? (Aber was macht man, wenn durch Präprozessor-Flags nur eine Auswahl der neuen Textblöcke aktiviert wurde?) Gibt es eine Möglichkeit, hier eine vernünftige Zwischenebene einzufügen? Letztlich sind die <nummer>n ja auch wieder nur "magic numbers", die idR besser durch Konstanten-Bezeichner ersetzt werden sollten.
Und last but not least die Bitte: In diesen technischen Fummeleien habe ich wenig Erfahrung, kann mir da jemand helfen?

@gaor: Nachdem Henne ja nicht mehr mitmacht, geht meine Hoffnung zuerst in deine Richtung. Du hast ja auch das schlaue Tool schick-data-gui geschrieben, das ja auch die SCHICK.DAT ausliest.
Zitieren
Ich verstehe noch nicht, wann du genau die SCHICK.DAT patchen bzw. was genau du tun willst. Willst du ein Tool programmieren, das die SCHICK.DAT modifiziert (und ein Backup des Originals anlegt)? Oder willst du, dass die SCHICK.DAT on-the-fly beim Aufruf von Bright-Eyes gepatcht wird? Willst du ein Tool, das einen Patch auf die SCHICK.DAT anwendet, oder eines, das dabei hilft, neue Patches für die SCHICK.DAT zu generieren? Willst du einfach einen interaktiven Editor für die SCHICK.DAT? Wie genau kann ich dir bei irgendetwas davon helfen?

Vielleicht könntest du erstmal damit anfangen, in irgendeiner Art von Dokument alle Bestandteile der SCHICK.DAT zu sammeln, von denen dir bereits bekannt ist, dass sie Bugs verursachen und für einen Bug-Fix verändert werden müssten. Darauf aufbauend kann man besser beurteilen, welche Herangehensweise am effizientesten ist.

Das Tool schick-data-gui war mein Versuch, einen Reader für die SCHICK.DAT zu erstellen, um besser zu verstehen, was darin enthalten ist und in welchem Format. Aber es gibt noch unzählige Teile der SCHICK.DAT, die das Tool nicht "versteht". Wie genau würde dir das bei deinem Problem helfen?
Zitieren
Sehr schnelle Rückmeldung, wow!
(17.04.2021, 12:07)gaor schrieb: Ich verstehe noch nicht, wann du genau die SCHICK.DAT patchen bzw. was genau du tun willst. Willst du ein Tool programmieren, das die SCHICK.DAT modifiziert (und ein Backup des Originals anlegt)?

Ja, in dieser Richtung. Mir schwebt folgendes vor:

Die originale SCHICK.DAT muss sich jeder selber an einer vordefinierten Position im BrightEyes-Verzeichnisbaum ablegen (klar, wegen Copyright) und dann einmalig ein Skript starten, das daraus die Einzelteile (wie z.B. TEXT.LTX) generiert und wieder an einer gewissen Position ablegt. Also von der Logik her recht ähnlich zum einmaligen Erstellen der Binär-Segmente, wofür die schick.exe abgelegt wird und dann per Skript der disassembler drübergeht.

Daneben gibt es dann noch Bearbeitungskopien aller in SCHICK.DAT enthaltenen Dateien, z.B. von TEXT.LTX, die natürlich nur lokal existieren (Copyright). In git werden nur Diff-Dateien verwaltet. Bei einem 'git pull' werden diese Bearbeitungskopien automatisch aus der lokalen Originaldatei + diff-Datei aus git erstellt. Bei 'git commit' werden die Diff-Dateien automatisch aus Bearbeitungskopie vs. Originaldatei erstellt und dann diese commitet.

Bei jedem Aufruf von 'make' sollte dann neben dem binary auch die dazu passende SCHICK.DAT erzeugt werden, d.h. die Bearbeitungskopien der Teile werden zu einer SCHICK.DAT zusammengebaut.

Jetzt ist noch die Frage, wo die Abhängigkeit von Präprozessor-Flags eingebracht werden kann. Das hatte ich mir zuvor zu einfach vorgestellt, glaube ich.
Hierzu fällt mir noch folgendes ein, was aber durchaus mit einigem Aufwand verbunden wäre: Man bräuchte -- zumindest für bestimmte Dateiformate wie LTX -- noch eine weitere Ebene, nämlich ein besser vom Menschen les- und bearbeitbares Dateiformat, z.B. was XML-artiges. Eine Grundform dieser Datei (auf die dann die diff-Dateien aufsetzen) -- nennen wir sie TEXT.LTX.xml -- muss sich per Skript aus TEXT.TLX erzeugen lassen und auch umgekehrt muss TEXT.LTX sich wieder per Skript aus TEXT.LTX.xml produzieren lassen. TEXT.LTX.xml bildet jetzt die lokale Bearbeitungskopie; die von git verwalteten Diff's setzen auf dieser Datei auf. Darin kann man jetzt bequem alles mögliche mit unterbringen, z.B. Kommentare, Abhängigkeiten von gewissen (Präprozessor-)Flags, oder zu jeder Textstelle auch einen zugehörigen Konstanten-Bezeichner, auf den der Quellcode dann Bezug nimmt (im Stil von get_tx(TX_ITEM_NAME_SWORD) anstelle von get_tx(23)). Die zugehörigen Definitionen "enum { ... TX_ITEM_NAME_SWORD = 23 ...};" müssten dann vor dem Compilieren automatisch aus der Datei TEXT.LTX.xml erzeugt werden.

Aber das ist nur das, was ich mir so zurechtgelegt habe. Vielleicht ist es aber auch unnötig kompliziert. Denkst du, dass das Vorgehen vernünftig ist, hast du andere Vorschläge?

(17.04.2021, 12:07)gaor schrieb: Vielleicht könntest du erstmal damit anfangen, in irgendeiner Art von Dokument alle Bestandteile der SCHICK.DAT zu sammeln, von denen dir bereits bekannt ist, dass sie Bugs verursachen und für einen Bug-Fix verändert werden müssten. Darauf aufbauend kann man besser beurteilen, welche Herangehensweise am effizientesten ist.

Mir geht es erstmal um die Text-Inhalte (Rechtschreibfehler, etc.), und darum, dass manche ID's von Gasthäusern im Original durcheinandergeraten sind (es gibt Gasthäuser in unterschiedlichen Orten mit derselben ID). Oder z.B. darum, dass es möglich ist den fahrenden Händler Kolberg in einem Haus in Clanegh anzusiedeln, so wie es im Patch von NRS auch passiert. Oder um den entgleisten Pixel bei den Hunden.

Vielleicht wäre es ein Anfang, sich erstmal auf ein sehr gut verstandenes Dateiformat wie .LTX zu konzentrieren.

Bei der technischen Umsetzung fühle ich mich unsicher. Für das Zerlegen und Zusammenbauen der Einzelteile gibt es wohl von euch schon passenden Code (nltpack). Dann die Frage, womit führt man das Patchen am besten durch, ist das standard diff-patch Paar dafür geeignet? Das ganze muss dann in das Makefile eingearbeitet werden und die ganzen für die Automatisierung nötigen Skripte erstellt werden usw.
Zitieren
(17.04.2021, 12:07)gaor schrieb: Das Tool schick-data-gui war mein Versuch, einen Reader für die SCHICK.DAT zu erstellen, um besser zu verstehen, was darin enthalten ist und in welchem Format. Aber es gibt noch unzählige Teile der SCHICK.DAT, die das Tool nicht "versteht". Wie genau würde dir das bei deinem Problem helfen?

Du weißt auf alle Fälle schon mal, wie man die SCHICK.DAT in die Einzelteile zerlegt. Und zumindest bei den gut verstandenen Formaten: Du hast es geschafft, den Inhalt auf den Bildschirm zu bringen! Da ist der Weg nicht weit, den Inhalt auch in ein (noch zu definierendes) XML-Format rauszuschreiben. Aber bitte verstehe mich nicht falsch: Das setzt natürlich voraus, dass du meinen Ansatz für vernünftig hältst, und dass du Lust hast, dich da einzubringen.
Zitieren
Ich möchte dich gerne unterstützen, weil ich's auch toll finde, dass du Zeit in Bright-Eyes investierst. Ich kann das angesprochene Thema aber noch nicht ganz überblicken. Für kleine Rechtschreibfehler wie den von dir verlinkten (https://github.com/siebenstreich/Bright-...0.cpp#L767) wäre ein Editor oder gar ein eigenes externes Format (XML) doch ein Overkill, oder nicht? Das ist doch verhältnismäßig elegant direkt im Code lösbar. Es geht ja nur um einzelne Bytes, da wäre es sogar denkbar, einfach direkt die Bytes in der SCHICK.DAT mit einem Hex-Editor zu manipulieren und mit xxd ein diff der Hexadezimal-Darstellungen zu erstellen, damit es auch andere einfach bei sich anwenden können. Genauso einfach wäre das Problem der Gasthäuser-IDs.

Welche Änderungen an der SCHICK.DAT für die übrigen von dir angesprochenen Probleme (Kolberg und entgleiste Pixel) nötig wären, kann ich nicht nachvollziehen, weil ich die Debatten nicht verfolgt habe. Deswegen meinte ich ja, es wäre gut, wenn es eine Übersicht gäbe, welche Arten von Änderungen benötigt werden, damit man besser beurteilen kann, welche Lösung sich anbietet.
Zitieren
(17.04.2021, 15:43)siebenstreich schrieb: Sehr schnelle Rückmeldung, wow!
[quote="gaor" pid='166743' dateline='1618657646']
Ja, in dieser Richtung. Mir schwebt folgendes vor:

Die originale SCHICK.DAT muss sich jeder selber an einer vordefinierten Position im BrightEyes-Verzeichnisbaum ablegen (klar, wegen Copyright) und dann einmalig ein Skript starten, das daraus die Einzelteile (wie z.B. TEXT.LTX) generiert und wieder an einer gewissen Position ablegt.

Hendriks nltpack kann schick.dat aus- und auch wieder verpacken. Mir ist noch nicht klar, worin der Vorteil des von Dir beschriebenen Verfahrens besteht.
Zitieren
@gaor: Ich überblicke auch noch nicht, wie komplex die an der SCHICK.DAT nötigen Änderungen werden. Aber ich denke, es sollte einheitlich sein. Kleinere Sachen per Binärpatch zu korrigieren und größere dann anders, das gefällt mir nicht so recht.

@gaor, Rabenaas:
Prinzipiell sehe ich die folgenden 3 Möglichkeiten, die ich mal versuche mit Vor- und Nachteilen aufzuzählen. Bitte schreibt mal eure Meinung dazu!

1. SCHICK.DAT bleibt gleich und man macht "Hot-Fixes" direkt im Code. Das passiert schon (sehr vereinzelt) in BrightEyes. Schnell und dirty, aber vielleicht sollte man es einfach so machen. Da bin ich irgendwie hin- und hergerissen. Unten ein Beispiel dazu. Eine potentieller Makel ergäbe sich, wenn es viele Code-Stellen gibt die auf denselben fehlerhaften Eintrag in der SCHICK.DAT zugreifen, denn dann muss man den Hot-Fix an jeder solchen Stelle vornehmen. Potentiell könnte das bei Rechtschreibfehlern in Gegenstandsnamen auftreten (es gibt da Basilikenzunge -> Basili*s*kenzunge, Wolfmesser -> Wolf*s*messer), hab ich mir noch nicht genauer angeschaut.

2. Man repariert die SCHICK.DAT per Hex-Editor oder sonstwie und verbreitet Binärpatches. Das gefällt mir nicht so gut, weil es intransparent ist und nicht recht zu dem offenen Ansatz von BrightEyes passt. Außerdem gibt es die Gefahr, dass man sich bei größeren oder vielen Änderungen verheddert.

3. Man bricht (zumindest die textlastigen) Daten der SCHICK.DAT auf ein besser lesbares Format herunter und macht dort die Änderungen. Dann kann man diffs von Textdateien machen und hat außerdem die Möglichkeit, Kommentare vorzunehmen und Änderungen fallweise zu aktivieren oder deaktivieren. Die Vorstellung gefällt mir schon gut. Der Nachteil ist, dass dafür einige Vorarbeit nötig ist und man in Gefahr läuft, sich damit zu verzetteln.


Hier noch ein konkretes Beispiel für einen Hot-Fix:
An zwei Stellen (Stelle 1, Stelle 2) wird in BrightEyes ein Textfenster angelegt, das es zuvor nicht gab.
Anders als bei den anderen Textfenstern kommt der Textinhalt dann also nicht aus der SCHICK.DAT, sondern direkt aus dem binary.
Die Frage ist jetzt, ob man das so gut findet, oder eher nicht. Es ist mindestens uneinheitlich. Aber man kann natürlich argumentieren, dass der Zweck die Mittel heiligt und das Problem damit kurz und bündig gelöst ist.
Zitieren
(17.04.2021, 20:58)siebenstreich schrieb: Ein potentieller Makel ergäbe sich, wenn es viele Code-Stellen gibt, die auf denselben fehlerhaften Eintrag in der SCHICK.DAT zugreifen, denn dann muss man den Hot-Fix an jeder solchen Stelle vornehmen.
Dann definiert man halt eine Funktion, die man an jeder solchen Stelle aufruft. Das halte ich jetzt für keinen besonders großen Aufwand.


(17.04.2021, 20:58)siebenstreich schrieb: Man [..] verbreitet Binärpatches.
Da stimme ich zu. Ich finde es auch sehr unbefriedigend, wenn man nur die Binär-Files rausgibt und nirgendwo nachvollziehbar und reproduzierbar dokumentiert ist, was und wie geändert wurde.


(17.04.2021, 20:58)siebenstreich schrieb: Man bricht (zumindest die textlastigen) Daten der SCHICK.DAT auf ein besser lesbares Format herunter und macht dort die Änderungen.
Wenn es nur um Text geht, braucht man kein lesbareres Format, weil der Text ja schon so lesbar ist. Du kannst dir ja mal die SCHICK.DAT mit einem Hex-Editor anschauen. Der Text ist bis auf ein paar Sonderzeichen direkt lesbar. Solche Patches könnte man mit diff und xxd erstellen. Gemessen an allen Beispielen, die du uns hier bisher vorgestellt hast, finde ich es aber immer noch am einfachsten, die Änderungen direkt im Quellcode vorzunehmen - so wie es bereits getan wird.


(17.04.2021, 20:58)siebenstreich schrieb: Die Frage ist jetzt, ob man das so gut findet, oder eher nicht.
Ich finde es gut so und sähe nur zwei Gründe, warum man es anders machen sollte: 1. Falls es wirklich sehr viele sehr große Änderungen an der SCHICK.DAT gibt, die sehr viele unübersichtliche zusätzliche Codezeilen benötigen. 2. Man will generell nicht nur Bugs fixen, sondern komplexere Mod-artige Änderungen vornehmen (sowas schwebte mir ursprünglich mal vor, als ich das schick-data-gui Tool begonnen habe, bin davon aber wieder abgekommen).

Am Rande bemerkt verstehe ich nicht, warum Henne an den von dir verlinkten Stellen so umständliche Char-Arrays anstatt String-Literale benutzt:
Code:
const unsigned char add_line[110] = { 0x40, 0x3c,'I','C','H',' ',
    'G','L','A','U','B','E',',',' ',
    'I','C','H',' ',
    'M','U','S','S',' ',
    'E','S',' ' ,'N','U','R',' ',
    'N','O','C','H',' ','E','I','N',' ',
    'E','I','N','Z','I','G','E','S',' ',
    'M','A','L',' ','V','E','R','S','U','C','H','E','N','!', 0x3e,' ',
    'M','U','R','M','E','L','T',' ',
    '%', 's',' ',
    'A','L','S',' ',
    '%', 's',' ',
    'W','I','E','D','E','R',' ',
    'A','U','F',' ',
    '%','s','E',' ',
    'F', 0x9a, 'S','S','E',' ',
    'K','O','M','M','T','.',
    '\0'
    };

Ich habe ja nicht so viel Ahnung von C und C++, aber ginge das nicht auch so?
Code:
const unsigned char add_line[110] = "\x40\x3cICH GLAUBE, ICH MUSS ES NUR NOCH "
                                    "EIN EINZIGES MAL VERSUCHEN!\x3e MURMELT %s "
                                    "ALS %s WIEDER AUF %sE F\x9aSSE KOMMT.";
Und muss man dafür überhaupt eine neue Variable definieren, oder kann man das Literal nicht direkt ins Funktionsargument reinschreiben?
Zitieren
Die Schreibweise mit mehrzeiligen Textkonstanten habe ich noch nie (bewusst) gesehen. Hab's gerade mit einem modernen gcc probiert. Damit passt alles.
Zitieren
(17.04.2021, 22:28)gaor schrieb:
(17.04.2021, 20:58)siebenstreich schrieb: Ein potentieller Makel ergäbe sich, wenn es viele Code-Stellen gibt, die auf denselben fehlerhaften Eintrag in der SCHICK.DAT zugreifen, denn dann muss man den Hot-Fix an jeder solchen Stelle vornehmen.
Dann definiert man halt eine Funktion, die man an jeder solchen Stelle aufruft. Das halte ich jetzt für keinen besonders großen Aufwand.

Hatte mir fast gedacht dass das kommt. :)
Ich will eigentlich mit dem Code möglichst nahe am Original bleiben, nachdem es sich ja nicht um was Eigenständiges handelt, sondern um einen Bugfix von bestehendem Code. Und da hab ich irgendwie eine Blockade, wegen einem fehlenden Buchstaben eine neue Funktion einzubauen. Aber letztlich ist das natürlich allemal besser als an 10 Stellen im Code synchron dieselbe Ergänzung einzufügen.
Und trotzdem werde ich das dumpfe Gefühl nicht ganz los: Konsequenterweise sollte das Übel an der Wurzel gepackt werden, und die liegt nunmal in der SCHICK.DAT und nicht im Code, der die (mit dem Rechtschreibfehler behafteten) Daten nur weiter verarbeitet.

(17.04.2021, 22:28)gaor schrieb: Wenn es nur um Text geht, braucht man kein lesbareres Format, weil der Text ja schon so lesbar ist. Du kannst dir ja mal die SCHICK.DAT mit einem Hex-Editor anschauen. Der Text ist bis auf ein paar Sonderzeichen direkt lesbar.

Stimmt natürlich. Was mich halt etwas stört ist, dass ich da nichts kommentieren kann und keine Präprozessor-Maschinerie zur Verfügung habe.

(17.04.2021, 22:28)gaor schrieb: Solche Patches könnte man mit diff und xxd erstellen. Gemessen an allen Beispielen, die du uns hier bisher vorgestellt hast, finde ich es aber immer noch am einfachsten, die Änderungen direkt im Quellcode vorzunehmen - so wie es bereits getan wird.
[...]
Ich finde es gut so und sähe nur zwei Gründe, warum man es anders machen sollte: 1. Falls es wirklich sehr viele sehr große Änderungen an der SCHICK.DAT gibt, die sehr viele unübersichtliche zusätzliche Codezeilen benötigen.
ok, das ist eine klare Aussage, vielen Dank! Daran werde ich mich erstmal halten und schauen, wie weit ich komme.

(17.04.2021, 22:28)gaor schrieb: 2. Man will generell nicht nur Bugs fixen, sondern komplexere Mod-artige Änderungen vornehmen (sowas schwebte mir ursprünglich mal vor, als ich das schick-data-gui Tool begonnen habe, bin davon aber wieder abgekommen).

Nein, darum geht es mir nicht. Ich will lediglich "digitale Denkmalpflege" betreiben, um die sehr treffenden Worte von Henne (oder von dir?) zu benutzen.

Für vernünftiges Modding hielte ich es für zielführender, wenn man das Spiel neu und v.a. sauber programmiert und sich dabei die Logik aus BrightEyes abschaut. Vorher muss man sich sehr gründlich überlegen, wie man die Schnitstelle zu den Daten-Beschreibungsdateien möglichst flexibel gestalten will. Dieses Gefühl hatte ich von Anfang an und seitdem ich durch BrightEyes etwas in die Untiefen des Quellcodes geblickt habe, hat sich das nur deutlich verhärtet.

(17.04.2021, 22:28)gaor schrieb: Am Rande bemerkt verstehe ich nicht, warum Henne an den von dir verlinkten Stellen so umständliche Char-Arrays anstatt String-Literale benutzt [...]

Das hatte ich mich auch gefragt. Hat es vielleicht was mit BCC-Kompatibilität zu tun?
Zitieren
(18.04.2021, 00:01)siebenstreich schrieb: Das hatte ich mich auch gefragt. Hat es vielleicht was mit BCC-Kompatibilität zu tun?

Sicher nicht, der Abschnitt ist ja in ifdef M302de_ORIGINAL_BUGFIX eingeschlossen. Bei mir läuft der pre-commit hook auf jeden Fall auch ohne Probleme durch, wenn man die Variante mit String-Literalen verwendet. Das geht alles auch durch, wenn man nicht erst diese umständliche Variable add_line definiert:
Code:
strcpy((char*)Real2Host(ds_readfp(TEXT_OUTPUT_BUF)), (char*)get_tx(25));
strcat((char*)Real2Host(ds_readfp(TEXT_OUTPUT_BUF)),
        "\x40\x3cICH GLAUBE, ICH MUSS ES NUR NOCH "
        "EIN EINZIGES MAL VERSUCHEN!\x3e MURMELT %s "
        "ALS %s WIEDER AUF %sE F\x9aSSE KOMMT.");
Zitieren
(18.04.2021, 07:47)gaor schrieb: Sicher nicht, der Abschnitt ist ja in ifdef M302de_ORIGINAL_BUGFIX eingeschlossen. Bei mir läuft der pre-commit hook auf jeden Fall auch ohne Probleme durch, wenn man die Variante mit String-Literalen verwendet.

Der pre-commit hook (zumindest der BCC-Teil) dürfte davon auch nichts sehen, wegen dem #ifdef M302de_ORIGINAL_BUGFIX.

Ich habe deine Code-Bereinigung bei mir eingebaut (vorherige Version sicherheitshalber noch auskommentiert vorhanden, ich würde als Bestätigung die Stelle gerne mal "live" im Spiel sehen). Momentan wartet das noch zusammen mit einer länger werdenden Liste von Änderungen auf einen commit. Das kommt davon, wenn man sich nach dem Aufreißen einer Baustelle vom Hunderdsten ins Tausendste ablenken lässt...

Das blockweise Aufteilen von String-Konstanten kannte ich übrigens auch noch nicht. Wieder was gelernt!
Zitieren
(17.04.2021, 11:23)siebenstreich schrieb: Teilweise betreffen die Bugs ja auch die Daten in der SCHICK.DAT. Momentan werden Daten "on the fly" im Quellcode repariert, z.B. diese Reparatur eines Rechtschreibfehlers.
Die Reparatur ist aber nicht so gut realisiert, da nicht geprüft wird wie lang die von get_tx() zurückgegebene Zeichenkette ist, und somit möglicherweise Speicherstellen geändert werden könnten die nicht zum Text gehören.

Ich nutze um Dateien aus SCHICK.DAT zu identifizieren z.B. NVF-Dateien bei denen der Header fehlt oder keine Palette enthalten ist (um zu entscheiden welche Palette hinzugeladen werden muss) die Länge der Daten und die CRC32-Prüfsumme der Daten. Stimmt beides überein kann man relativ sicher sein, das man es wirklich mit den Daten zu tun hat die man erwartet.
Zitieren
An dieser Stelle passiert sogar etwas in dieser Richtung.

Aber ich bin nicht der Meinung, dass für eine "gute" Reparatur sowas zwingend abgeprüft werden muss. Wenn jemand in der SCHICK.DAT rumeditiert und dann BrightEyes damit ausführt, dann könnte auch an allen nicht fehlerbehafteten Stellen Mist passieren, wo ja auch nichts überprüft wird.
Sinnvoll wäre vielleicht das einmalige Überprüfen einer Hashsumme über die SCHICK.DAT beim Starten von BrightEyes. Ich notiere mir das mal (glaube aber nicht, dass ich das so schnell angehe...).
Zitieren
Du solltest Bugfixes und "Feature Mods" nicht im selben "Commit" mit anderen Dingen machen, sondern zumindest separate Commits, besser sogar verschiedene Branches. Ich habe extrem große Schwierigkeiten, in dem Wust aus Konstantenänderungen, Tabstoppänderungen und sonstigen Dingen die Patch-relevanten Sachen herauszudestillieren; der letzte Commit war besonders schlimm. Für die neuere Version des Patches konnte ich diese neueren Korrekturen deshalb nicht berücksichtigen.
Zitieren
Was die Änderungen der SCHICK.DAT anbelangt, existieren im Moment bei mir nur die geänderten Einzeldateien, die ich damals ganz pragmatisch mit Hex-Editor angegangen bin. Diese Änderungen kann man aber gewissermaßen mittels eines Änderungsskripts dokumentieren. Dazu müssten wir uns nur einig wären, wie das ganze aussehen soll.

Ausgangspunkt wäre die in Einzeldateien zerlegt SCHICK.DAT. Die Änderungen wären dreierlei:
  1. Einerseits die Textdateien, die aus nullterminierten Strings bestehen. Bei denen werden nicht nur einzelne Bytes geändert, sondern mitunter Buchstaben eingefügt und ganze Strings hinzugefügt. Hier könnte man ein kleines CLI-Tool erstellen, welches zwei Kommandos kennt: "replace" und "add". Das Änderungsskript würde dann beispielsweise (kein "echtes" Beispiel) enthalten:
    Code:
    ltxtool ORVIL.LTX replace 57 "HIER STEHT DER NEUE TEXT FÜR STRING 57, WELCHER ERSETZT WIRD."
    ltxtool SERSKE.LTX add "HIER STEHT DER TEXT FÜR DEN HINZUGEKOMMENEN STRING."
  2. Andererseits Binärdateien, bei denen einzelne Bytes geändert werden, ohne die Dateigröße zu verändern. Hier beispielsweise das Ändern von Item-Flags oder die hinzugekommene Markierung eines Hauses in Clanegh als Wohngebäude von Treborn Kolberg. Ich weiß nicht, ob es hierfür ein portables Tool schon gibt. Aber das Änderungsskript hätte dann solche Zeilen wie:
    Code:
    changeFile CLANEGH.DAT at 42A with 04 00 00
  3. Die FIGHT.LST ist ein Spezialfall: Hier sind zunächst die zusätzlichen Nachtlagerkämpfe an die bestehende Datei hintendran zu kopieren (das geht einfach mit COPY /B FIGHT.LST + FIGHT.NEW FIGHT.LST) und danach die Anzahl der Kämpfe in den ersten zwei Bytes mit "changeFile" oder wie immer das Tool heißt zu erhöhen. Die neuen Kämpfe kann man dann einfach als feststehende Binärdatei auf github hochladen. Alternative: Man schreibt ein kleines C-Programm, welches die neuen Kämpfe als gepackte structs beschreibt und dann speziell die FIGHT.LST patcht, das heißt hinten dran tackert und den Kampfzahlzähler am Dateibeginn erhöht. Dann wären die neuen Kämpfe besser "dokumentiert". Wäre allerdings extrem aufwändung und vielleicht trotzdem nicht so informativ.
Mit SCHICKM.EXE selbst hat das eigentlich nichts zu tun, weshalb das m.E. eigentlich aus BrightEyes ganz raus sollte. Allerdings bestehen Abhängigkeiten zwischen den Bugfixes in SCHICKM.EXE und SCHICK.DAT: eine ungepatchte SCHICKM.EXE funktioniert mit einer gepatchten SCHICK.DAT, aber nicht umgekehrt, denn wenn die neuen Nachtlagerkämpfe referenziert werden, müssen sie natürlich auch in SCHICK.DAT enthalten sein; dasselbe mit neuen Textstrings.
Zitieren
(26.04.2021, 08:52)NRS schrieb: Du solltest Bugfixes und "Feature Mods" nicht im selben "Commit" mit anderen Dingen machen, sondern zumindest separate Commits, besser sogar verschiedene Branches. Ich habe extrem große Schwierigkeiten, in dem Wust aus Konstantenänderungen, Tabstoppänderungen und sonstigen Dingen die Patch-relevanten Sachen herauszudestillieren; der letzte Commit war besonders schlimm. Für die neuere Version des Patches konnte ich diese neueren Korrekturen deshalb nicht berücksichtigen.

Ja, im letzten Commit ist einiges zusammengekommen. Hat sich halt so ergeben, ich hatte es ja auch angekündigt:

(20.04.2021, 10:15)siebenstreich schrieb: Momentan wartet das noch zusammen mit einer länger werdenden Liste von Änderungen auf einen commit. Das kommt davon, wenn man sich nach dem Aufreißen einer Baustelle vom Hunderdsten ins Tausendste ablenken lässt...

Es liegt einfach daran, dass ich gerne mal zu einer anderen Sache springe, bevor die erste abgeschlossen ist. Das dürfte mit meinem generell unsteten Naturell zu tun haben wie auch damit, dass man beim Bearbeiten oftmals änderungswürdige Sachen entdeckt, die dann schnell (oder auch weniger schnell) eingeschoben werden. Entweder, weil man denkt dass ein Vorziehen vernünftig ist, oder schlichtweg aus einer Laune heraus. Und nachdem ich ja praktisch alleine dran arbeite, hält sich meine Motivation auch in Genzen, mir hier mehr Disziplin aufzuerlegen.

Du kannst es aber ganz einfach so machen: Lade den Code herunter und suche nach "Original-Bug <nr>". An einem gewissen Punkt hatte ich angefangen, die reparierten Bugs durchzunummerieren. Wenn da eine neue Nummer dazukommt, dann handelt es sich wahrscheinlich um einen neuen Bugfix. Für den letzten Commit gibt es da:
  • Original-Bug 15: Komischer Spielzustand, wenn in der Versteinerungsfalle in der verfallenen Herberge alle Helden außer dem NPC versteinert werden.
  • Original-Bug 19: Werden in einem Dungeon zwei Gruppen vereinigt, so ist das Gruppe-Vereinigen-Icon danach nicht ausgegraut.
  • Original-Bug 20: Kann ignoriert werden. Ein weiterer Bug zum Thema Alchemie, den ich aber unwissentlich schon mitrepariert hatte.
Zitieren
Wenn ich das richtig verstehe, verwenden alle drei Korrekturen zwei neue Funktionen im Segment Nr. 2. Diese neuen Funktionen ermitteln die Anzahl der Helden insgesamt, beziehungsweise in der aktuellen Gruppe, ohne den NPC. Neue Funktionen einzubauen ist auf Binärebene mit das aufwändigste überhaupt. Es ist noch aufwändiger als das bloße Einfügen von Code in bestehende Funktionen. Es müssen dazu nicht nur alle Segmentbezüge aktualisiert werden -- dafür habe ich mir bereits vor langer Zeit ein Werkzeug gebastelt --, sondern es muss die Tabelle der öffentlichen Funktionen im Segment des Overlay-Managers ergänzt werden. Eine Heidenarbeit mit sehr wenig Ertrag. Ich muss also leider davon absehen, diese Korrekturen zu übernehmen. Mal abgesehen davon, dass ich den Patch jetzt abschließen möchte -- irgendein Kleinkram wird immer wieder noch auftauchen, aber die großen Fehler sollten jetzt eigentlich alle korrigiert sein.

siebenstreich schrieb:Und nachdem ich ja praktisch alleine dran arbeite, hält sich meine Motivation auch in Genzen, mir hier mehr Disziplin aufzuerlegen.
Das ist alles verständlich und nachvollziehbar. Nur gibt Dir git eigentlich das Werkzeug mit, durch das Umschalten auf verschiedene Branches entsprechend die einzelnen Änderungskategorien trotzdem schön sauber voneinander getrennt zu halten, auch wenn Du an drei Dingen gleichzeitig arbeitest.
Zitieren
(26.04.2021, 20:54)NRS schrieb: Wenn ich das richtig verstehe, verwenden alle drei Korrekturen zwei neue Funktionen im Segment Nr. 2.

Original-Bug 19 braucht diese Funktionen nicht, sondern nur eine Zeile zusätzlichen Code. Wobei der Bug wirklich alles andere als kritisch ist. (Ich hatte nicht konkret nach einem Fix gesucht, sondern mir ist die richtige Stelle beim Stöbern von selbst über den Weg gelaufen...)

(26.04.2021, 20:54)NRS schrieb: Diese neuen Funktionen ermitteln die Anzahl der Helden insgesamt, beziehungsweise in der aktuellen Gruppe, ohne den NPC. Neue Funktionen einzubauen ist auf Binärebene mit das aufwändigste überhaupt. Es ist noch aufwändiger als das bloße Einfügen von Code in bestehende Funktionen. Es müssen dazu nicht nur alle Segmentbezüge aktualisiert werden -- dafür habe ich mir bereits vor langer Zeit ein Werkzeug gebastelt --, sondern es muss die Tabelle der öffentlichen Funktionen im Segment des Overlay-Managers ergänzt werden.

Die zusätzlichen Funktionen sind für die Original-Bugs 12, 13, 14 und 15 notwendig. Eine sauberere Lösung wäre eigentlich, den vorhandenen Funktionen count_heroes_available und count_heroes_available_in_group ein zusätzliches Flag <ignore_npc> mit zu übergeben. Ist das vielleicht einfacher umzusetzen in Maschinencode? Ich habe mich für die neuen Funktionen entschieden, weil ich die Signatur der Original-Funktionen nicht verändern wollte.

Dann gäbe es noch die Alternative, den Test aus den vorhandenen Funktionen zusammenzustricken, wie es an manchen Stellen im Originalcode auch gemacht wird. Es sollte

Code:
count_heroes_available_ignore_npc()
dasselbe sein wie
Code:
check_hero(get_hero(6)) ? count_heroes_available() : (count_heroes_available()-1)
und es sollte
Code:
count_heroes_available_in_group_ignore_npc()
dasselbe sein wie
Code:
is_hero_available_in_group(get_hero(6)) ? count_heroes_available_in_group() : (count_heroes_available_in_group()-1)

(26.04.2021, 20:54)NRS schrieb: Mal abgesehen davon, dass ich den Patch jetzt abschließen möchte -- irgendein Kleinkram wird immer wieder noch auftauchen, aber die großen Fehler sollten jetzt eigentlich alle korrigiert sein.

In der Liste der neu gefundenen Bugs sind sehr viele völlig unkritische Kleinigkeiten dabei, das stimmt. Zwei Bugs halte ich schon noch für reparierenswert, aber ich habe mich nicht damit auseinandergesetzt bisher. Vielleicht hast du ja Lust, dem noch nachzugehen.
  • Speicherzugriffsfehler nach dem Finalisieren eines lange dauernden (>=1 Tag) Rezepts, für den das brauende Held von der Gruppe abgetrennt wurde. [Genauer: Einen Helden ein lange dauerndes (>= 1 Tag) Rezept in einem Gasthaus brauen lassen und von der Gruppe trennen. Mit der verkleinerten Gruppe irgendwas machen, bis die Zeit verstrichen ist. Dann durch Weiterschalten der aktiven Gruppe auf den Alchemie-Helden im Gasthaus wechseln. Es wird die Meldung über den Erfolg oder Misserfolg der Brauaktion angezeigt. Danach befindet man sich mit diesem einen Helden in der Herberge. In der Statuszeile wird anstelle des Namens der Herberge Datenmüll oder fremde Textteile angezeigt, je nachdem, was die andere Gruppe währenddessen gemacht hat. Z.B. habe ich einfach eine leere Zeile dort gesehen, den Namen einer anderen Herberge (nachdem die anderen Helden in genau dieser Herberge übernachtet hatten um die Zeit abzusitzen), oder die Ortsbeschreibung von Manrin (die natürlich viel zu lang ist und dann in die Heldenbilder hineinhängt).]
  • AT/PT-Basiswerte bleiben unverändert bei Anlegen des Kraftgürtels (vmtl. auch Gulmond, KK-Elixier, KK-Steigern-Zauber etc.)
Zitieren
(26.04.2021, 18:38)NRS schrieb: Was die Änderungen der SCHICK.DAT anbelangt, existieren im Moment bei mir nur die geänderten Einzeldateien, die ich damals ganz pragmatisch mit Hex-Editor angegangen bin. Diese Änderungen kann man aber gewissermaßen mittels eines Änderungsskripts dokumentieren. Dazu müssten wir uns nur einig wären, wie das ganze aussehen soll.

Ausgangspunkt wäre die in Einzeldateien zerlegt SCHICK.DAT. Die Änderungen wären dreierlei:
  1. Einerseits die Textdateien, die aus nullterminierten Strings bestehen. Bei denen werden nicht nur einzelne Bytes geändert, sondern mitunter Buchstaben eingefügt und ganze Strings hinzugefügt. Hier könnte man ein kleines CLI-Tool erstellen, welches zwei Kommandos kennt: "replace" und "add". Das Änderungsskript würde dann beispielsweise (kein "echtes" Beispiel) enthalten:
    Code:
    ltxtool ORVIL.LTX replace 57 "HIER STEHT DER NEUE TEXT FÜR STRING 57, WELCHER ERSETZT WIRD."
    ltxtool SERSKE.LTX add "HIER STEHT DER TEXT FÜR DEN HINZUGEKOMMENEN STRING."
  2. Andererseits Binärdateien, bei denen einzelne Bytes geändert werden, ohne die Dateigröße zu verändern. Hier beispielsweise das Ändern von Item-Flags oder die hinzugekommene Markierung eines Hauses in Clanegh als Wohngebäude von Treborn Kolberg. Ich weiß nicht, ob es hierfür ein portables Tool schon gibt. Aber das Änderungsskript hätte dann solche Zeilen wie:
    Code:
    changeFile CLANEGH.DAT at 42A with 04 00 00
  3. Die FIGHT.LST ist ein Spezialfall: Hier sind zunächst die zusätzlichen Nachtlagerkämpfe an die bestehende Datei hintendran zu kopieren (das geht einfach mit COPY /B FIGHT.LST + FIGHT.NEW FIGHT.LST) und danach die Anzahl der Kämpfe in den ersten zwei Bytes mit "changeFile" oder wie immer das Tool heißt zu erhöhen. Die neuen Kämpfe kann man dann einfach als feststehende Binärdatei auf github hochladen. Alternative: Man schreibt ein kleines C-Programm, welches die neuen Kämpfe als gepackte structs beschreibt und dann speziell die FIGHT.LST patcht, das heißt hinten dran tackert und den Kampfzahlzähler am Dateibeginn erhöht. Dann wären die neuen Kämpfe besser "dokumentiert". Wäre allerdings extrem aufwändung und vielleicht trotzdem nicht so informativ.
Mit SCHICKM.EXE selbst hat das eigentlich nichts zu tun, weshalb das m.E. eigentlich aus BrightEyes ganz raus sollte. Allerdings bestehen Abhängigkeiten zwischen den Bugfixes in SCHICKM.EXE und SCHICK.DAT: eine ungepatchte SCHICKM.EXE funktioniert mit einer gepatchten SCHICK.DAT, aber nicht umgekehrt, denn wenn die neuen Nachtlagerkämpfe referenziert werden, müssen sie natürlich auch in SCHICK.DAT enthalten sein; dasselbe mit neuen Textstrings.

Vielen Dank für diese Rückmeldung, interessant! Mal schauen, ob und wann ich was mache.

Würde es dir etwas ausmachen, mir deine Änderungen (zumindest unter 1. und 2.) in dem angesprochenen Format zur Verfügung zu stellen?
Zitieren




Benutzer, die gerade dieses Thema anschauen: 1 Gast/Gäste