Reverse Engineering der NLT II

[Obi-Wahn]

Genial!

[llm]

lohnt es sich für Tests das ganze durch andere DOS kompiler zu jagen - z.B. Borland 5.02 oder Watcom V2 - um zu schauen ob da unter DOS Fehler auftauchen, oder direkt auf Windows/Linux gehen mit aktuellen gcc?

[llm]

clang meckert hier

Code:

seg006.cpp:120:18: note: use '==' to turn this assignment into an equality comparison
  120 |        } while (list_i = list_i->next);
      |                        ^
      |                        ==

[llm]

Wenn die 20 Byte eliminiert wurden, wird der Code zu Rekonstruktionszwecken in ein anderes Verzeichnis kopiert.

macht es vielleicht sinn den Borland source mit defines auch unter gcc/clang/msvc kompilierbar zu machen - also ohne Verzeichnis-Kopie, oder sind einfach zu viele Änderungen nötig und das es unsauber wird?

[HenneNWH]

Wenn die 20 Byte eliminiert wurden, wird der Code zu Rekonstruktionszwecken in ein anderes Verzeichnis kopiert.

macht es vielleicht sinn den Borland source mit defines auch unter gcc/clang/msvc kompilierbar zu machen - also ohne Verzeichnis-Kopie, oder sind einfach zu viele Änderungen nötig und das es unsauber wird?

Warnungen sind behoben.

Die Verzeichniskopie ist notwendig damit, eventuell später, andere DOS-Versionen der SCHICKM.EXE rekonstruiert werden können.
Im Verzeichnis schick wird dann nach dem Kopieren alles so gehandhabt, wie im Verzeichnis gen.
D.h. Binäräquivalenz ist DANN nicht mehr notwendig und andere Compiler sind dann gern gesehen.

[llm]

scheint auf jeden Fall "gut" kompilierbar (kein linken) - mit aktuellen windows clang/msvc cl und watcom v2 wcl kompilieren die cpps schon mal ohne viele Warnungen - weiss aber nicht wie viel code nicht durch defines inaktiv ist

[llm]

noch ein paar Clang 21.1.4 Warnungen

Code:

seg002.cpp:4137:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
4137 |        va_start(arguments, icons);
      |                            ^
seg002.cpp:4123:34: note: parameter of type 'short' is declared here
4123 | void draw_loc_icons(signed short icons, ...)
      |                                  ^
1 warning generated.
seg024.cpp:63:17: warning: implicit conversion from '(unnamed enum at ./common.h:1126:1)' to 'signed char' changes value from 177 to -79 [-Wconstant-conversion]
  63 |        g_pp20_index = ARCHIVE_FILE_BUCH_DAT;
      |                      ~ ^~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg025.cpp:451:17: warning: implicit conversion from '(unnamed enum at ./common.h:1126:1)' to 'signed char' changes value from 177 to -79 [-Wconstant-conversion]
  451 |        g_pp20_index = ARCHIVE_FILE_BUCH_DAT;
      |                      ~ ^~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg028.cpp:189:4: warning: 'memcpy' will always overflow; destination buffer has size 96, but size argument is 192 [-Wfortify-source]
  189 |                        memcpy(gs_palette_buildings, g_buffer11_ptr, 0xc0);
      |                        ^
1 warning generated.
seg030.cpp:187:15: warning: cast to 'unsigned char *' from smaller integer type 'Bit32u' (aka 'unsigned int') [-Wint-to-pointer-cast]
  187 |        states_tab = (Bit8u*)partners_tab[tlk_informer].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg031.cpp:85:15: warning: cast to 'M302de::struct_dialog_state *' from smaller integer type 'Bit32u' (aka 'unsigned int') [-Wint-to-pointer-cast]
  85 |        states_tab = (struct struct_dialog_state*)partners_tab[informer_id].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
seg032.cpp:1279:36: warning: implicit conversion from 'int' to 'signed char' changes value from 254 to -2 [-Wconstant-conversion]
1279 |        g_pp20_index = (ARCHIVE_FILE_DNGS + 12);
      |                      ~  ~~~~~~~~~~~~~~~~~~^~~~
1 warning generated.
seg038.cpp:269:2: warning: 'memcpy' will always overflow; destination buffer has size 10, but size argument is 20 [-Wfortify-source]
  269 |        memcpy(g_fig_move_pathdir, path_table[best_dir], 20);
      |        ^
1 warning generated.
seg065.cpp:111:35: warning: implicit conversion from 'int' to 'signed char' changes value from 254 to -2 [-Wconstant-conversion]
  111 |        g_pp20_index = ARCHIVE_FILE_DNGS + 12;
      |                      ~ ~~~~~~~~~~~~~~~~~~^~~~
1 warning generated.
seg076.cpp:685:42: warning: format specifies type 'int' but the argument has type 'unsigned long long' [-Wformat]
  685 |                D1_INFO("sizeof(stair_struct) = %d\n", sizeof(stair_struct));
      |                                                ~~    ^~~~~~~~~~~~~~~~~~~~
      |                                                %llu
.\v302de.h:137:38: note: expanded from macro 'D1_INFO'
  137 | #define D1_INFO(...) fprintf(stderr, __VA_ARGS__)
      |                                      ^~~~~~~~~~~
seg076.cpp:834:42: warning: format specifies type 'int' but the argument has type 'unsigned long long' [-Wformat]
  834 |                D1_INFO("sizeof(fight_struct) = %d\n", sizeof(fight_struct));
      |                                                ~~    ^~~~~~~~~~~~~~~~~~~~
      |                                                %llu
.\v302de.h:137:38: note: expanded from macro 'D1_INFO'
  137 | #define D1_INFO(...) fprintf(stderr, __VA_ARGS__)
      |                                      ^~~~~~~~~~~
2 warnings generated.
seg090.cpp:451:60: error: hex escape sequence out of range
  451 |                                                "ALS %s MIT DER BARRIERE KOLLIDIERT, BRICHT SIE IN ST\x9aCKE.\x40"
      |                                                                                                      ^~~~~
1 error generated.
seg093.cpp:808:15: warning: unsequenced modification and access to 'l_di' [-Wunsequenced]
  807 |                                                if (is_mouse_in_rect(l_di - 4, answer - 4,
      |                                                                      ~~~~
  808 |                                                                (l_di = g_town_positions[i].x) + 4,
      |                                                                      ^
seg093.cpp:809:17: warning: unsequenced modification and access to 'answer' [-Wunsequenced]
  807 |                                                if (is_mouse_in_rect(l_di - 4, answer - 4,
      |                                                                                ~~~~~~
  808 |                                                                (l_di = g_town_positions[i].x) + 4,
  809 |                                                                (answer = g_town_positions[i].y) + 4))
      |                                                                        ^
2 warnings generated.
seg097.cpp:639:23: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
  639 |                va_start(arguments, options);
      |                                    ^
seg097.cpp:546:81: note: parameter of type 'short' is declared here
  546 | signed short GUI_dialogbox(Bit8u* picture, char *name, char *text, signed short options, ...)
      |                                                                                ^
seg097.cpp:833:22: warning: passing an object that undergoes default argument promotion to 'va_start' has undefined behavior [-Wvarargs]
  833 |        va_start(arguments, options);
      |                            ^
seg097.cpp:783:48: note: parameter of type 'signed char' is declared here
  783 | signed short GUI_radio(char *text, signed char options, ...)
      |                                                ^
2 warnings generated.
seg109.cpp:957:28: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 255 to -1 [-Wconstant-conversion]
  957 |                g_gather_herbs_special = 255;
      |                                        ~ ^~~
1 warning generated.
seg112.cpp:156:29: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 130 to -126 [-Wconstant-conversion]
  156 |                        g_gather_herbs_special = 130;
      |                                                ~ ^~~
1 warning generated.
seg114.cpp:283:29: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 131 to -125 [-Wconstant-conversion]
  283 |                        g_gather_herbs_special = 131;
      |                                                ~ ^~~
1 warning generated.
seg116.cpp:190:28: warning: implicit conversion from 'int' to 'Bit8s' (aka 'signed char') changes value from 157 to -99 [-Wconstant-conversion]
  190 |                g_gather_herbs_special = 157;
      |                                        ~ ^~~
1 warning generated.

mein Test-Build script sieht so aus

Code:

@echo off

set clang_exe="c:\Program Files\LLVM\bin\clang.exe" -Wno-deprecated-declarations -Wno-deprecated-register -Wno-format-security

%clang_exe% datseg.cpp -c
%clang_exe% seg001.cpp -c
%clang_exe% seg002.cpp -c
%clang_exe% seg003.cpp -c
...

für den windows build clang/msvc und watcom musste ich nur den #include <unistd.h> durch io.h ersetzen

und noch unter Linux ein paar gcc 15.2.1 warnungen

Code:

seg028.cpp: In function ‘void M302de::load_area_description(short int)’:
seg028.cpp:340:53: warning: ‘ssize_t read(int, void*, size_t)’ writing 1000 bytes into a region of size 900 overflows the destination [-Wstringop-overflow=]
  340 |                        g_locations_tab_size = _read(fd, g_locations_tab, 1000);
In file included from v302de.h:76,
                from seg028.cpp:21:
datseg.h:1733:24: note: destination object ‘M302de::g_locations_tab’ of size 900
1733 | extern struct location g_locations_tab[150];    //ds:0xc025; seg028, seg64, seg066, seg74, seg094
      |                        ^~~~~~~~~~~~~~~
In file included from seg028.cpp:18:
/usr/include/unistd.h:371:16: note: in a call to function ‘ssize_t read(int, void*, size_t)’ declared with attribute ‘access (write_only, 2, 3)’
  371 | extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
      |                ^~~~
seg028.cpp:355:61: warning: ‘ssize_t read(int, void*, size_t)’ writing 1000 bytes into a region of size 900 overflows the destination [-Wstringop-overflow=]
  355 |                                g_locations_tab_size = _read(fd, g_locations_tab, 1000);
datseg.h:1733:24: note: destination object ‘M302de::g_locations_tab’ of size 900
1733 | extern struct location g_locations_tab[150];    //ds:0xc025; seg028, seg64, seg066, seg74, seg094
      |                        ^~~~~~~~~~~~~~~
/usr/include/unistd.h:371:16: note: in a call to function ‘ssize_t read(int, void*, size_t)’ declared with attribute ‘access (write_only, 2, 3)’
  371 | extern ssize_t read (int __fd, void *__buf, size_t __nbytes) __wur
      |                ^~~~
seg028.cpp: In function ‘void M302de::seg028_0224()’:
seg028.cpp:189:31: warning: ‘void* memcpy(void*, const void*, size_t)’ writing 192 bytes into a region of size 96 overflows the destination [-Wstringop-overflow=]
  189 |                        memcpy(gs_palette_buildings, g_buffer11_ptr, 0xc0);
      |                        ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
datseg.h:1126:15: note: destination object ‘M302de::gs_palette_buildings’ of size 96
1126 | extern Bit8u  gs_palette_buildings[3 * 32];    //ds:0x3eb3; seg002, seg028, seg066
      |              ^~~~~~~~~~~~~~~~~~~~
seg030.cpp: In function ‘void M302de::do_talk(short int, short int)’:
seg030.cpp:187:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  187 |        states_tab = (Bit8u*)partners_tab[tlk_informer].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
seg031.cpp: In function ‘void M302de::do_random_talk(short int, short int)’:
seg031.cpp:85:22: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
  85 |        states_tab = (struct struct_dialog_state*)partners_tab[informer_id].states_offset;
      |                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
seg038.cpp: In function ‘void M302de::FIG_find_path_to_target_backtrack(Bit8u*, short int, short int, short int, signed char, short int, short int, short int)’:
seg038.cpp:269:15: warning: ‘void* memcpy(void*, const void*, size_t)’ writing 20 bytes into a region of size 10 overflows the destination [-Wstringop-overflow=]
  269 |        memcpy(g_fig_move_pathdir, path_table[best_dir], 20);
      |        ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg038.cpp:14:
datseg.h:1647:20: note: destination object ‘M302de::g_fig_move_pathdir’ of size 10
1647 | extern signed char g_fig_move_pathdir[10];      // ds:0xd823; seg034, seg036, seg037, seg038
      |                    ^~~~~~~~~~~~~~~~~~
seg066.cpp: In function ‘void M302de::city_fade_and_colors()’:
seg066.cpp:1453:23: warning: ‘void* memcpy(void*, const void*, size_t)’ reading 288 bytes from a region of size 96 [-Wstringop-overread]
1453 |                memcpy(dst, gs_palette_floor, 0x120);
      |                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg066.cpp:12:
datseg.h:1125:15: note: source object ‘M302de::gs_palette_floor’ of size 96
1125 | extern Bit8u  gs_palette_floor[3 * 32];        //ds:0x3e53; seg002
      |              ^~~~~~~~~~~~~~~~
seg075.cpp: In function ‘void M302de::DNG_enter_dungeon(short int)’:
seg075.cpp:1343:23: warning: ‘void* memcpy(void*, const void*, size_t)’ reading 288 bytes from a region of size 96 [-Wstringop-overread]
1343 |                memcpy(g_renderbuf_ptr + 0x1f4, gs_palette_floor, 0x120);
      |                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from v302de.h:76,
                from seg075.cpp:13:
datseg.h:1125:15: note: source object ‘M302de::gs_palette_floor’ of size 96
1125 | extern Bit8u  gs_palette_floor[3 * 32];        //ds:0x3e53; seg002
      |              ^~~~~~~~~~~~~~~~
seg090.cpp: In function ‘short int M302de::DNG12_handler()’:
seg090.cpp:453:49: warning: hex escape sequence out of range
  453 |                                                "SCHADE DASS IHR DAS NICHT FR\x9aHER BEMERKT HABT.\0",
      |                                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

[HenneNWH]

Die Warnungen seh ich bei mir auch. Die müssen erstmal aus historischen Gründen bleiben.

[siebenstreich]

Keine Kritik, nur etwas ungefiltert nachgedacht:

Du hast das jetzt mit inline Assembler hingebogen. Irgendwie finde ich das immer noch etwas unbefriedigend, weil im Original wird an diesen Stellen ja ziemlich sicher C-Code gestanden haben. (Konsequent weitergedacht könnte man dann ja auch einfach das ganze Spiel in Assembler hinschreiben und sagen, man habe es reverse engineered...)

Vermutlich hast du an diesen Stellen vieles in C ausprobiert und nichts hat funktioniert. Trotzdem wäre es irgendwie cool, den richtigen C-Code noch zu finden.

Noch eine Sache: Wärst du dafür offen, vor dem "Fork" noch über gewisse Code-Bereinigungen (z.B. einheitlichere Variablenbenennung, etc.) zu diskutieren?

[HenneNWH]

@Siebenstreich: Das ist korrekt. Der ASM Code ist allerdings semantisch äquivalent zu dem C-Code welchen ich geschrieben hab.
Da in der Weiterentwicklung vorgesehen ist den ASM Code weitestgehend zu Entfernen bin ich damit zufrieden.

Die 20 Byte die noch offen sind betreffen nur technische Details mit Overlays und der DOS-EXE.
Diese Dinge spielen in der "neuen Welt" keine Rolle mehr.

Wie wäre es denn mit einem Meeting am Sonntag im Zeitraum vom (17:00 - 20:00 Uhr) für ca. 1 Stunde?
Deinen Vorschlag finde ich gut.

[NewProggie]

@Siebenstreich: Das ist korrekt. Der ASM Code ist allerdings semantisch äquivalent zu dem C-Code welchen ich geschrieben hab.
Da in der Weiterentwicklung vorgesehen ist den ASM Code weitestgehend zu Entfernen bin ich damit zufrieden.

Die 20 Byte die noch offen sind betreffen nur technische Details mit Overlays und der DOS-EXE.
Diese Dinge spielen in der "neuen Welt" keine Rolle mehr.

Wie wäre es denn mit einem Meeting am Sonntag im Zeitraum vom (17:00 - 20:00 Uhr) für ca. 1 Stunde?
Deinen Vorschlag finde ich gut.

Bei einem Meeting würde ich mich potentiell auch gerne mit einklinken