Roughly 90% of the names are correct since they were taken from the symbols. It's basically down to renaming the missing bits, such as void __cdecl gendung_418D91()

Priorities (by importance):

1. Fix any remaining bugs caused by the decompilation process. Most of them are listed in the TODO.
2. Cross test the game against the original to make sure game-play is identical. Ensure dungeon generation, monster AI, pathing, item generation, etc. are all the same, using the same map seed as reference.
3. Document the remaining unknown bits of code. Such as function_432100 or dword_666166.
4. Perform a final cross-reference between the PSX/Debug symbols and the code. Ensure that functions and data have the correct names and signage. If not present in the symbols, use an educated guess.
5. Clean up the file and directory structure (lowercase?), ensure we have a concise solution for compiling under multiple setups.
6. Refactor header files, add description to each file, and remove unnecessary including of headers. Each file should properly list what is static and what is global, in accordance with the PSX symbol.
7. Begin refactoring code and naming local variables (v1/v2/v3). The PSX symbol lists the name, type, and size for nearly all of them. It also tells us how many lines the function should span, which helps us know if the refactored code checks out. The game should be tested every few functions to ensure no regressions.
8. Add comments and documentation to the more complex code, such as the engine and file formats. Add any missing enums and structs.

Side Projects:

1. Begin documenting and reverse engineering DiabloUI. Possibly small portions of Storm and/or Standard network runtime. We can probably replace this entirely, but just for good measure.
2. Document and cross reference code between Diablo and Hellfire. Create a separate code base for Hellfire.
3. Reference the API changes needed to convert DirectX to OpenGL, to help modders make things platform independent.
4. Celebrate with pizza and beer, we're finished!

from devilution.

Could you explain a little more how one would go about something like 4 and 7?

@ttdonovan Sure! Hope this may help a bit. Feel free to ask questions, and we'll try to get anyone interested up to speed!

1. Download IDA (the Interactive Disassembler). Hex-Rays offers a freeware version for download at: https://www.hex-rays.com/products/ida/support/download_freeware.shtml
2. Open diablo.exe (version 1.09b in IDA) and wait until it has finished analysis.
3. Download the IDC script from the notes repository: http://sanctuary.github.io/notes/notes.idc
4. Run the IDC script in IDA on the fresh IDB database to import names for variables and functions, type definitions, etc. (Note: run the IDC script only on new IDB databases as it removes all variable names before adding new ones.); for more info, see https://github.com/galaxyhaxz/devilution/pull/79#issuecomment-400536087
5. Open DiabloUI.dll in IDA and try to understand what it is doing. To get started have a look at https://github.com/sanctuary/notes/pull/251
6. Load diabpsx.bin executable in IDA and explore :) You may also want to extract the lump.bin archive to be able to load fmv.bin, frontend.bin, game.bin, or pregame.bin shared libraries. Note, only one of these libraries is loaded at any one time, the reason probably being that Playstation 1 had very limited memory. For notes on what each address refers to in the PSX release, see below.

I'm failing to understand where the PSX/Debug symbols are coming from.

The are originally from the Japanese release of Diablo on Playstation 1. The CD contained debug information in a .SYM file, the format of which has been reversed, so we can recover type information, variable names, etc, for the PSX release.

@galaxyhaxz will upload the .SYM files from each realese of Diablo 1 on Playstation, but for now, we can refer to https://github.com/sanctuary/psx which contains debug information recovered from the Diablo (Japan) [SLPS-01416] release.

For the brave out there, the tool used to parse the SYM files is availabe at https://github.com/sanctuary/exp/tree/master/cmd/sym_dump and can be installed by running go get -u github.com/sanctuary/exp/cmd/sym_dump. Still, sym_dump could definitely use some love <3

There is still much to do, so any help is warmly welcome. Reversing feels like solving a big puzzle. Once you get into it, it can become quite addictive. Consider yourself warned ;)

Edit: Oh, and more specifically for item 7. (naming of local variables). Debug info about local variables was also included in the SYM files, see for instance https://github.com/sanctuary/psx/blob/93e9bfee26f188db87daf2fc927f584ee9a0eb2c/funcs/source/automap.cpp#L34

// Path: C:\diabpsx\SOURCE\AUTOMAP.CPP
// Addr: 0x801620A0
void AmDrawPlayer__Fiiiii(int x0, int y0, int x1, int y1, int PNum) {
	int PNum;
	struct LINE_F2 *L2;
}

from devilution.

Maybe these priorities are beyond my capabilities and knowledge. Could you explain a little more how one would go about something like 4 and 7? I'm failing to understand where the PSX/Debug symbols are coming from. Or could you point me to pervious commit where some of these refactoring have been done.

from devilution.

Refactoring has not yet been started because bug fixing is more important. That's probably the last thing that will happen to the project. The symbol files will be uploaded soon into a separate repository.

from devilution.

@mewmew thanks for the guidance. I've started messing around with Hex-Rays and few questions about loading the Diablo.exe.

1. Do I want to load the file as "Portable executable ..." or "Binary file" - my guess is executable.
2. What processor type should I chose?
3. Are the default options OK? See screenshot.

I've attache the output - can you confirm it appears to have loaded and ran the notes.idc correctly?

  bytes   pages size description
--------- ----- ---- --------------------------------------------
  3088384   377 8192 allocating memory for b-tree...
  2981888   364 8192 allocating memory for virtual array...
   262144    32 8192 allocating memory for name pointers...
-----------------------------------------------------------------
  6332416            total memory allocated

Loading processor module /Applications/IDA Free/ida64.app/Contents/MacOS/procs/pc64.dylib for metapc...OK
Loading type libraries...
Autoanalysis subsystem has been initialized.
Database for file 'Diablo.exe' has been loaded.
69F0E8: can't rename byte as 'DS' because the name 'DS' is a register name.
Caching 'Functions window'... ok
Flushing buffers, please wait...ok
Unloading IDP module /Applications/IDA Free/ida64.app/Contents/MacOS/procs/pc64.dylib...
Possible file format: Portable executable for 80386 (PE) (/Applications/IDA Free/ida64.app/Contents/MacOS/loaders/pe64.dylib)

  bytes   pages size description
--------- ----- ---- --------------------------------------------
  3031040   370 8192 allocating memory for b-tree...
  3031040   370 8192 allocating memory for virtual array...
   262144    32 8192 allocating memory for name pointers...
-----------------------------------------------------------------
  6324224            total memory allocated

Possible file format: Portable executable for 80386 (PE) (/Applications/IDA Free/ida64.app/Contents/MacOS/loaders/pe64.dylib)

  bytes   pages size description
--------- ----- ---- --------------------------------------------
  3031040   370 8192 allocating memory for b-tree...
  3031040   370 8192 allocating memory for virtual array...
   262144    32 8192 allocating memory for name pointers...
-----------------------------------------------------------------
  6324224            total memory allocated

Loading file '/Users/tanner/Workspace/ttdonovan-github/devilution/junk/Diablo/Diablo.exe' into database...
Detected file format: Portable executable for 80386 (PE)
Loading processor module /Applications/IDA Free/ida64.app/Contents/MacOS/procs/pc64.dylib for metapc...OK
Autoanalysis subsystem has been initialized.
  0. Creating a new segment  (0000000000401000-0000000000479000) ... ... OK
  1. Creating a new segment  (0000000000479000-0000000000483000) ... ... OK
  2. Creating a new segment  (0000000000483000-00000000006AE000) ... ... OK
Reading imports directory...
File /Users/tanner/Workspace/ttdonovan-github/devilution/junk/Diablo/Storm.dll is used for module Storm...
  3. Creating a new segment  (0000000000479400-0000000000483000) ... ... OK
Plugin "eh_parse" not found
Assuming __cdecl calling convention by default
Type library 'mssdk' loaded. Applying types...
Types applied to 164 names.
Plan  FLIRT signature: SEH for vc7-14
Marking typical code sequences...
Flushing buffers, please wait...ok
File '/Users/tanner/Workspace/ttdonovan-github/devilution/junk/Diablo/Diablo.exe' has been successfully loaded into the database.
Type library 'vc6win' loaded. Applying types...
Types applied to 0 names.
Using FLIRT signature: SEH for vc7-14
Propagating type information...
Function argument information has been propagated
The initial autoanalysis has been finished.
69F0E8: can't rename byte as 'DS' because the name 'DS' is a register name.

After running the notes.idc script I was looking at the enum structures and noticed they have different ordering and names from Devilution, for example directions. Is this because of the .SYM that is yet to be uploaded? Is it that the Devilution code is more align to the original and Sanctuary Notes has been updated?

from devilution.

@mewmew thanks for the guidance. I've started messing around with Hex-Rays and few questions about loading the Diablo.exe.

1. Do I want to load the file as "Portable executable ..." or "Binary file" - my guess is executable.
2. What processor type should I chose?
3. Are the default options OK? See screenshot.

Yes, defaults are fine. Portable executable. Processor type i386 (80386). Both detected as based on the log you posted:

Possible file format: Portable executable for 80386 (PE) (/Applications/IDA Free/ida64.app/Contents/MacOS/loaders/pe64.dylib)

The error you probably got ("69F0E8: can't rename byte as 'DS' because the name 'DS' is a register name.") is an issue with IDA. It should work for other decompilers.

After running the notes.idc script I was looking at the enum structures and noticed they have different ordering and names from Devilution, for example directions. Is this because of the .SYM that is yet to be uploaded? Is it that the Devilution code is more align to the original and Sanctuary Notes has been updated?

Both Devilution and the notes repo have the intended aim to get as close as possible to document the original game. Devilution is closer in the sense that the same names have been used for functions as based on the SYM debug info. The notes repo has tried to use consistent naming for functions, e.g. prefix with source file name.

See for instance drlg_l1_load_dun, which is defined in drlg_l1.cpp. This function has the PSX signature void LoadL1Dungeon__FPcii(char *sFileName, int vx, int vy), but is documented in the notes repo as follows for consistency:

// drlg_l1_load_dun loads tile IDs, monsters and objects from the given dungeon file.
void drlg_l1_load_dun(char *dun_path, int view_x, int view_y);

I think in the future, the notes repo will be brought closer to that of Devilution, and vice versa. To keep closer to the original instead.

Related to enum defs, that may simply be an error. Either in the notes repo, or in Devilution, and if so, should be fixed.

Happy reversing!

Cheers,
/u

from devilution.

@ttdonovan The symbol files are now live here: https://github.com/diasurgical/scalpel
They are already dumped and ready to be viewed in a text editor. If you use Notepad++, a syntax file is provided to help highlight and color things.

from devilution.

We should consider summarizing a set of steps in CONTRIBUTE.md to get up to speed with contribution, based on the discussions of this issue.

from devilution.

Will do when I have time. I'll try to clean up the readme in the next few days.

from devilution.