SCI: Move bpk/logkernel to main breakpoint infrastructure

This changes the syntax for bpk and logkernel:

Enable breakpoint on kernel call:

bpk FrameOut

Enable logging for kernel call:

bpk FrameOut log
For backward compatibility this has an alias: logkernel FrameOut

Removing a kernel call breakpoint is done with bp_del/bc now.

4 files changed, 42 insertions, 100 deletions

diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index efa10c337c..539475c464 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -598,7 +598,6 @@ void Kernel::mapFunctions() {
 		_kernelFuncs[id].workarounds = NULL;
 		_kernelFuncs[id].subFunctions = NULL;
 		_kernelFuncs[id].subFunctionCount = 0;
-		_kernelFuncs[id].debugLogging = false;
 		if (kernelName.empty()) {
 			// No name was given -> must be an unknown opcode
 			warning("Kernel function %x unknown", id);
@@ -715,85 +714,6 @@ void Kernel::mapFunctions() {
 	return;
 }
 
-bool Kernel::debugSetFunction(const char *kernelName, int logging, int breakpoint) {
-	if (strcmp(kernelName, "*")) {
-		for (uint id = 0; id < _kernelFuncs.size(); id++) {
-			if (_kernelFuncs[id].name) {
-				if (strcmp(kernelName, _kernelFuncs[id].name) == 0) {
-					if (_kernelFuncs[id].subFunctions) {
-						// sub-functions available and main name matched, in that case set logging of all sub-functions
-						KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-						uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-						for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-							if (kernelSubCall->function) {
-								if (logging != -1)
-									kernelSubCall->debugLogging = logging == 1 ? true : false;
-								if (breakpoint != -1)
-									kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-							}
-							kernelSubCall++;
-						}
-						return true;
-					}
-					// function name matched, set for this one and exit
-					if (logging != -1)
-						_kernelFuncs[id].debugLogging = logging == 1 ? true : false;
-					if (breakpoint != -1)
-						_kernelFuncs[id].debugBreakpoint = breakpoint == 1 ? true : false;
-					return true;
-				} else {
-					// main name was not matched
-					if (_kernelFuncs[id].subFunctions) {
-						// Sub-Functions available
-						KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-						uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-						for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-							if (kernelSubCall->function) {
-								if (strcmp(kernelName, kernelSubCall->name) == 0) {
-									// sub-function name matched, set for this one and exit
-									if (logging != -1)
-										kernelSubCall->debugLogging = logging == 1 ? true : false;
-									if (breakpoint != -1)
-										kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-									return true;
-								}
-							}
-							kernelSubCall++;
-						}
-					}
-				}
-			}
-		}
-		return false;
-	}
-	// Set debugLogging for all calls
-	for (uint id = 0; id < _kernelFuncs.size(); id++) {
-		if (_kernelFuncs[id].name) {
-			if (!_kernelFuncs[id].subFunctions) {
-				// No sub-functions, enable actual kernel function
-				if (logging != -1)
-					_kernelFuncs[id].debugLogging = logging == 1 ? true : false;
-				if (breakpoint != -1)
-					_kernelFuncs[id].debugBreakpoint = breakpoint == 1 ? true : false;
-			} else {
-				// Sub-Functions available, enable those too
-				KernelSubFunction *kernelSubCall = _kernelFuncs[id].subFunctions;
-				uint kernelSubCallCount = _kernelFuncs[id].subFunctionCount;
-				for (uint subId = 0; subId < kernelSubCallCount; subId++) {
-					if (kernelSubCall->function) {
-						if (logging != -1)
-							kernelSubCall->debugLogging = logging == 1 ? true : false;
-						if (breakpoint != -1)
-							kernelSubCall->debugBreakpoint = breakpoint == 1 ? true : false;
-					}
-					kernelSubCall++;
-				}
-			}
-		}
-	}
-	return true;
-}
-
 #ifdef ENABLE_SCI32
 enum {
 	kKernelEntriesSci2 = 0x8b,
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index bddfab531f..d1dce37e48 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -143,8 +143,6 @@ struct KernelFunction {
 	const SciWorkaroundEntry *workarounds;
 	KernelSubFunction *subFunctions;
 	uint16 subFunctionCount;
-	bool debugLogging;
-	bool debugBreakpoint;
 };
 
 class Kernel {
@@ -231,11 +229,6 @@ public:
 	 */
 	void loadKernelNames(GameFeatures *features);
 
-	/**
-	 * Sets debug flags for a kernel function
-	 */
-	bool debugSetFunction(const char *kernelName, int logging, int breakpoint);
-
 private:
 	/**
 	 * Loads the kernel selector names.
diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index e2522414e9..9f1b536fec 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -779,6 +779,44 @@ bool SciEngine::checkAddressBreakpoint(const reg32_t &address) {
 	return found;
 }
 
+bool SciEngine::checkKernelBreakpoint(const Common::String &name) {
+	bool found = false;
+
+	if (_debugState._activeBreakpointTypes & BREAK_KERNEL) {
+
+		Common::List<Breakpoint>::const_iterator bp;
+		for (bp = _debugState._breakpoints.begin(); bp != _debugState._breakpoints.end(); ++bp) {
+			if (bp->_action == BREAK_NONE)
+				continue;
+			if (bp->_type != BREAK_KERNEL)
+				continue;
+
+			bool wildcard = bp->_name.lastChar() == '*';
+			Common::String prefix = bp->_name;
+			if (wildcard)
+				prefix.deleteLastChar();
+			if (bp->_name == name || (wildcard && name.hasPrefix(prefix))) {
+				if (bp->_action == BREAK_BREAK) {
+					if (!found)
+						_console->debugPrintf("Break on k%s\n", name.c_str());
+					_debugState.debugging = true;
+					_debugState.breakpointWasHit = true;
+				} else if (bp->_action == BREAK_BACKTRACE) {
+					if (!found)
+						_console->debugPrintf("Break on k%s\n", name.c_str());
+					logBacktrace();
+				} else if (bp->_action == BREAK_INSPECT) {
+					// Ignoring this mode, to make it identical to BREAK_LOG
+				}
+				found = true;
+			}
+		}
+	}
+
+	return found;
+}
+
+
 void debugSelectorCall(reg_t send_obj, Selector selector, int argc, StackPtr argp, ObjVarRef &varp, reg_t funcp, SegManager *segMan, SelectorType selectorType) {
 	int activeBreakpointTypes = g_sci->_debugState._activeBreakpointTypes;
 	const char *objectName = segMan->getObjectName(send_obj);
diff --git a/engines/sci/engine/vm.cpp b/engines/sci/engine/vm.cpp
index 1e088d6d7c..b169e0ec11 100644
--- a/engines/sci/engine/vm.cpp
+++ b/engines/sci/engine/vm.cpp
@@ -296,7 +296,8 @@ ExecStack *send_selector(EngineState *s, reg_t send_obj, reg_t work_obj, StackPt
 			sp = CALL_SP_CARRY; // Destroy sp, as it will be carried over
 		}
 
-		if (activeBreakpointTypes || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
+		if ((activeBreakpointTypes & (BREAK_SELECTOREXEC | BREAK_SELECTORREAD | BREAK_SELECTORWRITE))
+		     || DebugMan.isDebugChannelEnabled(kDebugLevelScripts))
 			debugSelectorCall(send_obj, selector, argc, argp, varp, funcp, s->_segMan, selectorType);
 
 		assert(argp[0].toUint16() == argc); // The first argument is argc
@@ -375,13 +376,8 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
 		addKernelCallToExecStack(s, kernelCallNr, -1, argc, argv);
 		s->r_acc = kernelCall.function(s, argc, argv);
 
-		if (kernelCall.debugLogging)
+		if (g_sci->checkKernelBreakpoint(kernelCall.name))
 			logKernelCall(&kernelCall, NULL, s, argc, argv, s->r_acc);
-		if (kernelCall.debugBreakpoint) {
-			debugN("Break on k%s\n", kernelCall.name);
-			g_sci->_debugState.debugging = true;
-			g_sci->_debugState.breakpointWasHit = true;
-		}
 	} else {
 		// Sub-functions available, check signature and call that one directly
 		if (argc < 1)
@@ -447,13 +443,8 @@ static void callKernelFunc(EngineState *s, int kernelCallNr, int argc) {
 		addKernelCallToExecStack(s, kernelCallNr, subId, argc, argv);
 		s->r_acc = kernelSubCall.function(s, argc, argv);
 
-		if (kernelSubCall.debugLogging)
+		if (g_sci->checkKernelBreakpoint(kernelSubCall.name))
 			logKernelCall(&kernelCall, &kernelSubCall, s, argc, argv, s->r_acc);
-		if (kernelSubCall.debugBreakpoint) {
-			debugN("Break on k%s\n", kernelSubCall.name);
-			g_sci->_debugState.debugging = true;
-			g_sci->_debugState.breakpointWasHit = true;
-		}
 	}
 
 	// Remove callk stack frame again, if there's still an execution stack