Dev Question: How to patch SCI scripts

[Vhati]

Background

I've begun learning how to tinker with the ScummVM debugger (ctrl-shift-d) and SCI Companion. I can use breakpoints & backtrace in the former and decompile scripts with the latter. This week, I gleaned enough SCI to read a script and wield the debugger's send command to invoke methods on the fly (I gave myself inventory items in Quest for Glory 4).

This article was particularly helpful.
https://moral.net.au/writing/2017/09/23/sierra_bug/

My assembly skill is weak though; I can sort of read it, invert condition opcodes, hello-world level reversing. I've used CheatEngine's debugger to reverse engineer [rand() % x + y] arithmetic behind FasterThanLight's sector map generator algorithm, for use in a Java-based saved game editor. I've never had occasion to write assembly.

Question

What is the workflow for developing a patch (shimming with assembly signatures), and testing if I did it correctly? If I can identify a problematic line in a decompiled script, how do I turn that knowledge into patch signatures/data?
https://github.com/scummvm/scummvm/blob ... atches.cpp

If I wanted to merely substitute my own string in for narrator text, would that go here? I'm guessing I should abstract away hardcoded strings somehow for internationalization.
https://github.com/scummvm/scummvm/blob ... essage.cpp
.
.
I haven't set up a build environment yet. For now I'll assume the wiki is adequate (MinGW). Pretend I have one already.
https://wiki.scummvm.org/index.php?titl ... ng_ScummVM
.
.
These are the sort of bugs I'm thinking would be good exercises to start with (of course, if someone else fixes them first, I won't mind ).
Ticket: QFG4: Insect statue's alcove, msg presumes you know Chief
Ticket: QFG4: Grabbing the second bookshelf passage, msg doesn't reflect its closed/open state
Ticket: QFG4: Autosave SCI Script Deletes Manual Savegames

[Collector]

Most SCI games can be patched simply by dropping your modified recompiled script in the game's base folder or the path specified in the patchDir= in the RESOURCE.CFG. You can have SCI Companion compile it as a patch. For further help with Companion I would recommend asking here: http://sciprogramming.com/community/

For use in SVM this will not alter the hash of the file (usually the RESOURCE.MAP) it uses to ID a game, so it should still load. You can set the game to run from Companion in SVM for its debugger. you can also use a debugger build of DOSBox, but you will probably get more usable information from the SVM debugger. Note however that Companion's compiler is for SCI0 and SCI1.1 games. QfG4 is an SCI2 game.

If you are simply asking about how to patch only within SVM disregard my post.

[Vhati]

If you are simply asking about how to patch only within SVM disregard my post.

This, sorry. Your post was informative though.
I'm hoping to contribute to ScummVM, fixing QFG4's many bugs.

[Vhati]

That last ticket is scary. It deletes all but 20 of QFG4's saved games whenever it autosaves (disabling autosave prevents it, btw).

I guess I'll diagnose that here with the CD edition, and folks can chime in or watch me learn...

I filled my saved game dir with more than 20 files, loaded up a saved game, enabled autosave. I knew if I walked to another screen, my files would be deleted.

I summoned the debugger and ran the functions command (list kernel functions). Bunch of stuff came up. "FileIO" looked relevant.

Code: Select all

) bp_kernel FileIO
No kernel functions match FileIO.

Running bp_kernel without an arg gave its help, which suggested using an asterisk wildcard.

Code: Select all

) bp_kernel FileIO* break
) bp_kernel GetSaveFiles break
) bp_kernel MakeSaveFileName break

I set some breakpoints and triggered an autosave. The game paused and dropped down the debugger whenever it called those kernel functions. I dismissed the debugger by hitting escape until I found something interesting. Along the way, it was telling me the function names it found. To cut down on noise, I deleted the wildcard breakpoint, and added specific functions.

Code: Select all

) bp_del 0
) bp_kernel FileIOOpen break
) bp_kernel FileIOClose break
) bp_kernel FileIOUnlink break

.
.
At a breakpoint, the backtrace command prints breadcrumbs of where you are and what sent you here: script number, class, method.
.
.
Here's a suspicious bit, decompiled with SCI Companion (Script 0, Glory::save()).

Code: Select all

			(while
						(and
							(> temp9 0)
							(or
								(not (CheckFreeSpace (global29 data?)))
								(>= temp9 20)
							)
						)
						(localproc_21c5 (- temp9 1) newStr temp9 temp1)
						(= temp9
							(GetSaveFiles
								(global1 name?)
								(newStr data?)
								(temp1 data?)
							)
						)
					)

localproc_21c5() is deleting individual files (by calling FileIOUnlink). And it's in a loop with a >= 20 condition.
.
.
Next, I disassembled that method ("scummvm.log" captured the result) and looked for landmarks.

Code: Select all

) disasm Glory save bc

This is an excerpt from the (or ...) block above.

Code: Select all

0001:1fb1: 43 3f 02 00    callk	CheckFreeSpace[3f],	0002
0001:1fb5: 18             not  
0001:1fb6: 2f 05          bt   	05  [1fbd]
0001:1fb8: 8d 09          lst  	09
0001:1fba: 35 14          ldi  	14		; 20
0001:1fbc: 20             ge?  
0001:1fbd: 30 38 00       bnt  	0038  [1ff8]

Location: opcode_value arg [arg...] // opcode_name args_description

Comments in vm.cpp help explain what each opcode does.

• Call CheckFreeSpace().
• If it's true that there's inadequate space for a new save, branch to skip the rest of the OR condition and get to the loop content (deleting a file to free more space). Otherwise, keep going.
• Push a variable (temp9) onto the stack.
• Load a constant value into the accumulator register (0x14 hex, 20d decimal).
• Pop temp9 off the stack and compare to accumulator: is it greater-or-equal?
• If not true - and we got this far because the first condition wasn't true either - branch to the end of the loop. Done.

.
.
That's weird... My opcodes (e.g., bt = 0x2f, ldi = 0x35) didn't match the enums in "vm.h" !?

Code: Select all

	op_bnot     = 0x00,	// 000
	op_add      = 0x01,	// 001
	op_sub      = 0x02,	// 002
	op_mul      = 0x03,	// 003
	op_div      = 0x04,	// 004
	op_mod      = 0x05,	// 005
	op_shr      = 0x06,	// 006
	op_shl      = 0x07,	// 007
	op_xor      = 0x08,	// 008
	op_and      = 0x09,	// 009
	op_or       = 0x0a,	// 010
	op_neg      = 0x0b,	// 011
	op_not      = 0x0c,	// 012
	op_eq_      = 0x0d,	// 013
	op_ne_      = 0x0e,	// 014
	op_gt_      = 0x0f,	// 015
	op_ge_      = 0x10,	// 016
	op_lt_      = 0x11,	// 017
	op_le_      = 0x12,	// 018
	op_ugt_     = 0x13,	// 019
	op_uge_     = 0x14,	// 020
	op_ult_     = 0x15,	// 021
	op_ule_     = 0x16,	// 022
	op_bt       = 0x17,	// 023
	op_bnt      = 0x18,	// 024
	op_jmp      = 0x19,	// 025
	op_ldi      = 0x1a,	// 026
	op_push     = 0x1b,	// 027
	op_pushi    = 0x1c,	// 028
	op_toss     = 0x1d,	// 029
	op_dup      = 0x1e,	// 030
	op_link     = 0x1f,	// 031
	op_call     = 0x20,	// 032
	op_callk    = 0x21,	// 033
	op_callb    = 0x22,	// 034
	op_calle    = 0x23,	// 035
	op_ret      = 0x24,	// 036
	op_send     = 0x25,	// 037
	op_info     = 0x26,	// 038
	op_superP   = 0x27,	// 039
	op_class    = 0x28,	// 040
	// dummy      0x29,	// 041
	op_self     = 0x2a,	// 042
	op_super    = 0x2b,	// 043
	op_rest     = 0x2c,	// 044
	op_lea      = 0x2d,	// 045
	op_selfID   = 0x2e,	// 046
	// dummy      0x2f	// 047
	op_pprev    = 0x30,	// 048
	op_pToa     = 0x31,	// 049
	op_aTop     = 0x32,	// 050
	op_pTos     = 0x33,	// 051
	op_sTop     = 0x34,	// 052
	op_ipToa    = 0x35,	// 053
	op_dpToa    = 0x36,	// 054
	op_ipTos    = 0x37,	// 055
	op_dpTos    = 0x38,	// 056
	op_lofsa    = 0x39,	// 057
	op_lofss    = 0x3a,	// 058
	op_push0    = 0x3b,	// 059
	op_push1    = 0x3c,	// 060
	op_push2    = 0x3d,	// 061
	op_pushSelf = 0x3e,	// 062
	op_line     = 0x3f,	// 063
	//
	op_lag      = 0x40,	// 064
	op_lal      = 0x41,	// 065
	op_lat      = 0x42,	// 066
	op_lap      = 0x43,	// 067
	op_lsg      = 0x44,	// 068
	op_lsl      = 0x45,	// 069
	op_lst      = 0x46,	// 070
	op_lsp      = 0x47,	// 071
	op_lagi     = 0x48,	// 072
	op_lali     = 0x49,	// 073
	op_lati     = 0x4a,	// 074
	op_lapi     = 0x4b,	// 075
	op_lsgi     = 0x4c,	// 076
	op_lsli     = 0x4d,	// 077
	op_lsti     = 0x4e,	// 078
	op_lspi     = 0x4f,	// 079
	//
	op_sag      = 0x50,	// 080
	op_sal      = 0x51,	// 081
	op_sat      = 0x52,	// 082
	op_sap      = 0x53,	// 083
	op_ssg      = 0x54,	// 084
	op_ssl      = 0x55,	// 085
	op_sst      = 0x56,	// 086
	op_ssp      = 0x57,	// 087
	op_sagi     = 0x58,	// 088
	op_sali     = 0x59,	// 089
	op_sati     = 0x5a,	// 090
	op_sapi     = 0x5b,	// 091
	op_ssgi     = 0x5c,	// 092
	op_ssli     = 0x5d,	// 093
	op_ssti     = 0x5e,	// 094
	op_sspi     = 0x5f,	// 095
	//
	op_plusag   = 0x60,	// 096
	op_plusal   = 0x61,	// 097
	op_plusat   = 0x62,	// 098
	op_plusap   = 0x63,	// 099
	op_plussg   = 0x64,	// 100
	op_plussl   = 0x65,	// 101
	op_plusst   = 0x66,	// 102
	op_plussp   = 0x67,	// 103
	op_plusagi  = 0x68,	// 104
	op_plusali  = 0x69,	// 105
	op_plusati  = 0x6a,	// 106
	op_plusapi  = 0x6b,	// 107
	op_plussgi  = 0x6c,	// 108
	op_plussli  = 0x6d,	// 109
	op_plussti  = 0x6e,	// 110
	op_plusspi  = 0x6f,	// 111
	//
	op_minusag  = 0x70,	// 112
	op_minusal  = 0x71,	// 113
	op_minusat  = 0x72,	// 114
	op_minusap  = 0x73,	// 115
	op_minussg  = 0x74,	// 116
	op_minussl  = 0x75,	// 117
	op_minusst  = 0x76,	// 118
	op_minussp  = 0x77,	// 119
	op_minusagi = 0x78,	// 120
	op_minusali = 0x79,	// 121
	op_minusati = 0x7a,	// 122
	op_minusapi = 0x7b,	// 123
	op_minussgi = 0x7c,	// 124
	op_minussli = 0x7d,	// 125
	op_minussti = 0x7e,	// 126
	op_minusspi = 0x7f	// 127

.
.
My opcodes did, however, match what everyone else was writing in "script_patches.cpp". I scraped a list from comments there, incomplete but known to work.

Code: Select all

0x00	neg; One example, typo?
0x02	add
0x04	sub
0x06	mul
0x08	div
0x0c	shr
0x12	and
0x14	or
0x18	not
0x1a	eq?
0x1c	ne?
0x1e	gt?
0x20	ge?
0x22	lt?
0x24	le?
0x28	uge?
0x2c	ult?
0x2e	bt (UINT16)
0x2f	bt
0x30	bnt (UINT16)
0x31	bnt
0x32	jmp (UINT16)
0x33	jmp
0x34	ldi (UINT16)
0x35	ldi
0x36	push
0x38	pushi (UINT16)
0x39	pushi
0x3a	toss
0x3c	dup
0x3e	link (UINT16)
0x3f	link
0x40	call (??? 2 bytes)
0x41	call (??? 3 bytes)
0x43	callk
0x45	callb
0x47	calle
0x48	ret
0x4a	send (UINT16)
0x4b	send
0x51	class
0x54	self
0x57	super
0x59	rest
0x5a	lea (UINT16) (UINT16)
0x5b	lea
0x62	pToa (UINT16)
0x63	pToa
0x64	aTop (UINT16)
0x65	aTop
0x66	pTos (UINT16)
0x67	pTos
0x72	lofsa
0x74	lofss
0x76	push0
0x78	push1
0x7a	push2
0x7c	pushSelf
0x7e	line
0x80	lag (UINT16); One comment said lsg, typo?
0x81	lag; One comment said lsg, typo?
0x82	lal (UINT16)
0x83	lal
0x85	lat
0x87	lap
0x89	lsg
0x8b	lsl
0x8d	lst
0x8f	lsp
0x90	lagi
0x93	lali
0x99	lsgi
0x9a	lsli (UINT16)
0x9b	lsli
0xa0	sag (UINT16)
0xa1	sag
0xa3	sal
0xa5	sat
0xa8	ssg (UINT16)
0xa9	ssg
0xab	ssl
0xb0	sagi
0xb3	sali
0xba	ssli
0xc1	+ag
0xe1	-ag

Okay, the real opcode values are double the enums, sometimes plus 1. "vm.h" had condensed the list of repeated names: wide/narrow arg variants of each op.
.
.
To be continued...

[Vhati]

I haven't set up a build environment yet. For now I'll assume the wiki is adequate (MinGW).

lol *sigh*
All the the MinGW links are out of date and broken.

On the plus side, MinGW+MSYS setup is easier nowadays with the mingw-get installer.

• Basic Setup (meta packages)
  • mingw-developer-toolkit
  • mingw32-base
  • mingw32-gcc-g++

Takes a while but otherwise painless. No fiddling with environment variables required.

MinGW's ready.

• The GUI pachage manager is at: "MinGW/libexec/mingw-get/guimain.exe".
• The CLI package manager is "mingw-get".
• The bash prompt is at: "MinGW/msys/1.0/msys.bat".

ScummVM's Dependencies

• The package manager above only knows about MinGW/MSYS stuff, for upgrades. Other libraries are still a scavenger hunt.
• Luckily the wiki's precompiled libraries bundle is still viable, although I skipped the SDL stuff in there.
• Got the latest SDL "development library" (not the "runtime library" that the wiki calls for). And it's SDL2 these days, not 1.2. Copied the i686 files.
• Per the wiki, I added SDL's bundled DirectX development headers and libs.

Compiling

• I went to GitHub, fetched a snapshot zip of ScummVM, and extracted it in "MinGW/msys/1.0/home/[MyUserName]/".
• Spawned the aforementioned bash prompt.
• cd "scummvm-master"
• ./configure --disable-all-engines --enable-engine=sci,sci32
• make
• strip scummvm.exe

Success!

Now that I know my build environment is good, I can fork the repository properly and maybe commit something (Developer Central). I won't go into the minutae of git here. Next post, writing assembly!
.
.
EDIT: The git that ships with that MSYS is so old that its HTTPS requests fail. I ran a separately installed git in a non-MSYS prompt.
EDIT: Get SDL2, not SDL 1.2.

[Vhati]

Correction: The MSYS I installed above does NOT ship with an old git. It doesn't bundle git at all. I have Git for Windows installed separately.

There were multiple MSYS environments on my hard drive, and I confused this one with one from another project, which did have an ancient git.

[Raziel]

You could, if you wanted to, update the MinGW relevant part of ScummVM's wiki, now that you know what has to be done