SCI: Handle selector read/write breakpoints from opcodes

3 files changed, 82 insertions, 3 deletions

diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index 6a2324e4ee..28c7b950de 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -804,6 +804,37 @@ void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr arg
 	}	// switch
 }
 
+void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curValue, reg_t newValue, SegManager *segMan, BreakpointType breakpointType) {
+	const Object *var_container = obj;
+	if (!obj->isClass() && getSciVersion() != SCI_VERSION_3)
+		var_container = segMan->getObject(obj->getSuperClassSelector());
+	if ((index >> 1) >= var_container->getVarCount()) {
+		// TODO: error, warning, debug?
+		return;
+	}
+	uint16 varSelector = var_container->getVarSelector(index >> 1);
+	if (g_sci->checkSelectorBreakpoint(breakpointType, objp, varSelector)) {
+		// checkSelectorBreakpoint has already triggered the breakpoint.
+		// We just output the relevant data here.
+
+		Console *con = g_sci->getSciDebugger();
+		const char *objectName = segMan->getObjectName(objp);
+		const char *selectorName = g_sci->getKernel()->getSelectorName(varSelector).c_str();
+		if (breakpointType == BREAK_SELECTORWRITE) {
+			con->debugPrintf("Write to selector (%s:%s): change %04x:%04x to %04x:%04x\n",
+								objectName, selectorName,
+								PRINT_REG(curValue), PRINT_REG(newValue));
+		} else if (breakpointType == BREAK_SELECTORREAD) {
+			con->debugPrintf("Read from selector (%s:%s): %04x:%04x\n",
+								objectName, selectorName,
+								PRINT_REG(curValue));
+
+		} else {
+			assert(false);
+		}
+	}
+}
+
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result) {
 	Kernel *kernel = g_sci->getKernel();
 	if (!kernelSubCall) {
diff --git a/engines/sci/engine/scriptdebug.h b/engines/sci/engine/scriptdebug.h
index 15e71343de..964b8a4b54 100644
--- a/engines/sci/engine/scriptdebug.h
+++ b/engines/sci/engine/scriptdebug.h
@@ -31,6 +31,8 @@ extern const char *opcodeNames[];
 
 void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType);
 
+void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curValue, reg_t newValue, SegManager *segMan, BreakpointType breakpointType);
+
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result);
 
 }
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index edffa60b09..1e088d6d7c 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -1129,29 +1129,60 @@ void run_vm(EngineState *s) {
 
 		case op_pToa: // 0x31 (49)
 			// Property To Accumulator
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    validate_property(s, obj, opparams[0]), NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
 			s->r_acc = validate_property(s, obj, opparams[0]);
 			break;
 
 		case op_aTop: // 0x32 (50)
+			{
 			// Accumulator To Property
-			validate_property(s, obj, opparams[0]) = s->r_acc;
+			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    opProperty, s->r_acc,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+
+			opProperty = s->r_acc;
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
+		}
 
 		case op_pTos: // 0x33 (51)
+			{
 			// Property To Stack
-			PUSH32(validate_property(s, obj, opparams[0]));
+			reg_t value = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    value, NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
+			PUSH32(value);
 			break;
+		}
 
 		case op_sTop: // 0x34 (52)
+			{
 			// Stack To Property
-			validate_property(s, obj, opparams[0]) = POP32();
+			reg_t newValue = POP32();
+			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    opProperty, newValue,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+			opProperty = newValue;
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif
 			break;
+		}
 
 		case op_ipToa: // 0x35 (53)
 		case op_dpToa: // 0x36 (54)
@@ -1161,10 +1192,25 @@ void run_vm(EngineState *s) {
 			// Increment/decrement a property and copy to accumulator,
 			// or push to stack
 			reg_t &opProperty = validate_property(s, obj, opparams[0]);
+			reg_t oldValue = opProperty;
+
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORREAD) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    oldValue, NULL_REG,
+				                    s->_segMan, BREAK_SELECTORREAD);
+			}
+
 			if (opcode & 1)
 				opProperty += 1;
 			else
 				opProperty -= 1;
+
+			if (g_sci->_debugState._activeBreakpointTypes & BREAK_SELECTORWRITE) {
+				debugPropertyAccess(obj, s->xs->objp, opparams[0],
+				                    oldValue, opProperty,
+				                    s->_segMan, BREAK_SELECTORWRITE);
+			}
+
 #ifdef ENABLE_SCI32
 			updateInfoFlagViewVisible(obj, opparams[0], true);
 #endif