[Scummvm-git-logs] scummvm master -> d61ca31e5613231e5c97663e318cbe83e0a6686d

djsrv dservilla at gmail.com
Wed Jul 22 16:39:07 UTC 2020


This automated email contains information about 3 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
336576c538 DIRECTOR: LINGO: Enforce return values
3a694d7496 DIRECTOR: LINGO: Handle more ARGCNORETs
d61ca31e56 DIRECTOR: LINGO: Improve return value handling


Commit: 336576c538583a601c24ba8cc05862bd0b0e73bc
    https://github.com/scummvm/scummvm/commit/336576c538583a601c24ba8cc05862bd0b0e73bc
Author: djsrv (dservilla at gmail.com)
Date: 2020-07-22T12:38:01-04:00

Commit Message:
DIRECTOR: LINGO: Enforce return values

Changed paths:
    engines/director/lingo/lingo-builtins.cpp
    engines/director/lingo/lingo-bytecode.cpp
    engines/director/lingo/lingo-code.cpp
    engines/director/lingo/lingo-code.h
    engines/director/lingo/lingo-events.cpp
    engines/director/lingo/lingo.cpp
    engines/director/lingo/lingo.h


diff --git a/engines/director/lingo/lingo-builtins.cpp b/engines/director/lingo/lingo-builtins.cpp
index e04d32a536..5b681976e0 100644
--- a/engines/director/lingo/lingo-builtins.cpp
+++ b/engines/director/lingo/lingo-builtins.cpp
@@ -1569,7 +1569,7 @@ void LB::b_importFileInto(int nargs) {
 void menuCommandsCallback(int action, Common::String &text, void *data) {
 	Common::String name = Common::String::format("scummvmMenu%d", action);
 
-	LC::call(name, 0);
+	LC::call(name, 0, false);
 }
 
 void LB::b_installMenu(int nargs) {
diff --git a/engines/director/lingo/lingo-bytecode.cpp b/engines/director/lingo/lingo-bytecode.cpp
index c8ae8b979a..f345e88adf 100644
--- a/engines/director/lingo/lingo-bytecode.cpp
+++ b/engines/director/lingo/lingo-bytecode.cpp
@@ -507,7 +507,7 @@ void LC::cb_call() {
 
 	Datum nargs = g_lingo->pop();
 	if ((nargs.type == ARGC) || (nargs.type == ARGCNORET)) {
-		LC::call(name, nargs.u.i, nargs.type == ARGC ? FBLTIN : CBLTIN);
+		LC::call(name, nargs.u.i, nargs.type == ARGC);
 
 	} else {
 		warning("cb_call: first arg should be of type ARGC or ARGCNORET, not %s", nargs.type2str());
diff --git a/engines/director/lingo/lingo-code.cpp b/engines/director/lingo/lingo-code.cpp
index 1de43645fc..0ecc13e242 100644
--- a/engines/director/lingo/lingo-code.cpp
+++ b/engines/director/lingo/lingo-code.cpp
@@ -50,6 +50,7 @@
 #include "director/channel.h"
 #include "director/util.h"
 #include "director/lingo/lingo.h"
+#include "director/lingo/lingo-builtins.h"
 #include "director/lingo/lingo-code.h"
 #include "director/lingo/lingo-object.h"
 #include "director/lingo/lingo-the.h"
@@ -212,7 +213,7 @@ void LC::c_xpop() {
 	g_lingo->pop();
 }
 
-void Lingo::pushContext(const Symbol funcSym, bool preserveVarFrame) {
+void Lingo::pushContext(const Symbol funcSym, bool allowRetVal, Datum defaultRetVal) {
 	debugC(5, kDebugLingoExec, "Pushing frame %d", g_lingo->_callstack.size() + 1);
 	CFrame *fp = new CFrame;
 
@@ -223,6 +224,8 @@ void Lingo::pushContext(const Symbol funcSym, bool preserveVarFrame) {
 	fp->localvars = g_lingo->_localvars;
 	fp->retMe = g_lingo->_currentMe;
 	fp->sp = funcSym;
+	fp->allowRetVal = allowRetVal;
+	fp->defaultRetVal = defaultRetVal;
 
 	g_lingo->_currentScript = funcSym.u.defn;
 
@@ -234,9 +237,51 @@ void Lingo::pushContext(const Symbol funcSym, bool preserveVarFrame) {
 
 	g_lingo->_currentArchive = funcSym.archive;
 
-	// Execute anonymous functions within the current var frame.
-	if (!preserveVarFrame && !funcSym.anonymous)
-		g_lingo->_localvars = new DatumHash;
+	DatumHash *localvars = g_lingo->_localvars;
+	if (!funcSym.anonymous) {
+		// Execute anonymous functions within the current var frame.
+		localvars = new DatumHash;
+	}
+
+	if (funcSym.argNames) {
+		int symNArgs = funcSym.nargs;
+		if ((int)funcSym.argNames->size() < symNArgs) {
+			int dropSize = symNArgs - funcSym.argNames->size();
+			warning("%d arg names defined for %d args! Dropping the last %d values", funcSym.argNames->size(), symNArgs, dropSize);
+			for (int i = 0; i < dropSize; i++) {
+				g_lingo->pop();
+				symNArgs -= 1;
+			}
+		} else if ((int)funcSym.argNames->size() > symNArgs) {
+			warning("%d arg names defined for %d args! Ignoring the last %d names", funcSym.argNames->size(), symNArgs, funcSym.argNames->size() - symNArgs);
+		}
+		for (int i = symNArgs - 1; i >= 0; i--) {
+			Common::String name = (*funcSym.argNames)[i];
+			if (!localvars->contains(name)) {
+				g_lingo->varCreate(name, false, localvars);
+				Datum arg(name);
+				arg.type = VAR;
+				Datum value = g_lingo->pop();
+				g_lingo->varAssign(arg, value, false, localvars);
+			} else {
+				warning("Argument %s already defined", name.c_str());
+				g_lingo->pop();
+			}
+		}
+	}
+	if (funcSym.varNames) {
+		for (Common::Array<Common::String>::iterator it = funcSym.varNames->begin(); it != funcSym.varNames->end(); ++it) {
+			Common::String name = *it;
+			if (!localvars->contains(name)) {
+				(*localvars)[name] = Datum();
+			} else {
+				warning("Variable %s already defined", name.c_str());
+			}
+		}
+	}
+	g_lingo->_localvars = localvars;
+
+	fp->stackSizeBefore = _stack.size();
 
 	g_lingo->_callstack.push_back(fp);
 
@@ -250,6 +295,16 @@ void Lingo::popContext() {
 	CFrame *fp = g_lingo->_callstack.back();
 	g_lingo->_callstack.pop_back();
 
+	uint maxRetVals = fp->allowRetVal ? 1 : 0;
+	if (_stack.size() > fp->stackSizeBefore + maxRetVals) {
+		error("handler %s returned extra %d values", fp->sp.name->c_str(), _stack.size() - fp->stackSizeBefore);
+	} else if (_stack.size() < fp->stackSizeBefore) {
+		error("handler %s popped extra %d values", fp->sp.name->c_str(), fp->stackSizeBefore - _stack.size());
+	} else if (fp->allowRetVal && _stack.size() == fp->stackSizeBefore) {
+		warning("handler %s did not return value", fp->sp.name->c_str());
+		g_lingo->push(fp->defaultRetVal);
+	}
+
 	// Destroy anonymous function context
 	if (fp->sp.anonymous) {
 		delete g_lingo->_currentScriptContext;
@@ -1332,7 +1387,7 @@ void LC::c_callcmd() {
 
 	int nargs = g_lingo->readInt();
 
-	LC::call(name, nargs, CBLTIN);
+	LC::call(name, nargs, false);
 }
 
 void LC::c_callfunc() {
@@ -1340,10 +1395,10 @@ void LC::c_callfunc() {
 
 	int nargs = g_lingo->readInt();
 
-	LC::call(name, nargs, FBLTIN);
+	LC::call(name, nargs, true);
 }
 
-void LC::call(const Common::String &name, int nargs, SymbolType bltinType) {
+void LC::call(const Common::String &name, int nargs, bool allowRetVal) {
 	if (debugChannelSet(3, kDebugLingoExec))
 		g_lingo->printSTUBWithArglist(name.c_str(), nargs, "call:");
 
@@ -1372,7 +1427,7 @@ void LC::call(const Common::String &name, int nargs, SymbolType bltinType) {
 					g_lingo->_stack.remove_at(g_lingo->_stack.size() - nargs);
 					nargs -= 1;
 				}
-				call(funcSym, nargs);
+				call(funcSym, nargs, allowRetVal);
 				return;
 			}
 			firstArg = firstArg.eval();
@@ -1395,7 +1450,7 @@ void LC::call(const Common::String &name, int nargs, SymbolType bltinType) {
 					g_lingo->_stack.remove_at(g_lingo->_stack.size() - nargs);
 					nargs -= 1;
 				}
-				call(funcSym, nargs);
+				call(funcSym, nargs, allowRetVal);
 				return;
 			}
 		}
@@ -1405,27 +1460,20 @@ void LC::call(const Common::String &name, int nargs, SymbolType bltinType) {
 	funcSym = g_lingo->getHandler(name);
 
 	// Builtin
-	if (funcSym.type == VOIDSYM) {
-		switch (bltinType) {
-		case CBLTIN:
-			if (g_lingo->_builtinCmds.contains(name)) {
-				funcSym = g_lingo->_builtinCmds[name];
-			}
-			break;
-		case FBLTIN:
-			if (g_lingo->_builtinFuncs.contains(name)) {
-				funcSym = g_lingo->_builtinFuncs[name];
-			}
-			break;
-		default:
-			break;
+	if (allowRetVal) {
+		if (g_lingo->_builtinFuncs.contains(name)) {
+			funcSym = g_lingo->_builtinFuncs[name];
+		}
+	} else {
+		if (g_lingo->_builtinCmds.contains(name)) {
+			funcSym = g_lingo->_builtinCmds[name];
 		}
 	}
 
-	call(funcSym, nargs);
+	call(funcSym, nargs, allowRetVal);
 }
 
-void LC::call(const Symbol &funcSym, int nargs) {
+void LC::call(const Symbol &funcSym, int nargs, bool allowRetVal) {
 	bool dropArgs = false;
 
 	if (funcSym.type == VOIDSYM) {
@@ -1460,7 +1508,7 @@ void LC::call(const Symbol &funcSym, int nargs) {
 	}
 
 	if (funcSym.type != HANDLER) {
-		int stackSize = g_lingo->_stack.size() - nargs;
+		uint stackSizeBefore = g_lingo->_stack.size() - nargs;
 
 		if (funcSym.target) {
 			// Only need to update the me obj
@@ -1473,14 +1521,18 @@ void LC::call(const Symbol &funcSym, int nargs) {
 			(*funcSym.u.bltin)(nargs);
 		}
 
-		int stackNewSize = g_lingo->_stack.size();
-
-		if (funcSym.type != HANDLER && funcSym.type != CBLTIN) {
-			if (stackNewSize - stackSize != 1)
-				warning("built-in function %s did not return value", funcSym.name->c_str());
-		} else if (funcSym.type == CBLTIN) {
-			if (stackNewSize - stackSize != 0)
-				warning("built-in procedure %s returned extra %d values", funcSym.name->c_str(), stackNewSize - stackSize);
+		uint stackSize = g_lingo->_stack.size();
+
+		if (funcSym.u.bltin != LB::b_return && funcSym.u.bltin != LB::b_returnNumber && funcSym.u.bltin != LB::b_value) {
+			uint maxRetVals = allowRetVal ? 1 : 0;
+			if (stackSize > stackSizeBefore + maxRetVals) {
+				error("builtin %s returned extra %d values", funcSym.name->c_str(), stackSize - stackSizeBefore);
+			} else if (stackSize < stackSizeBefore) {
+				error("builtin %s popped extra %d values", funcSym.name->c_str(), stackSizeBefore - stackSize);
+			} else if (allowRetVal && stackSize == stackSizeBefore) {
+				error("builtin function %s did not return value", funcSym.name->c_str());
+				g_lingo->push(Datum());
+			}
 		}
 		return;
 	}
@@ -1493,53 +1545,12 @@ void LC::call(const Symbol &funcSym, int nargs) {
 		g_lingo->push(d);
 	}
 
-	g_lingo->pushContext(funcSym, true);
-
-	DatumHash *localvars = g_lingo->_localvars;
-	if (!funcSym.anonymous) {
-		// Create new set of local variables
-		// g_lingo->_localvars is not set until later so any lazy arguments
-		// can be evaluated within the current variable frame
-		localvars = new DatumHash;
+	Datum defaultRetVal;
+	if (funcSym.target && funcSym.target->getObjType() == kFactoryObj && funcSym.name->equalsIgnoreCase("mNew")) {
+		defaultRetVal = funcSym.target; // return me
 	}
 
-	if (funcSym.argNames) {
-		int symNArgs = funcSym.nargs;
-		if ((int)funcSym.argNames->size() < symNArgs) {
-			int dropSize = symNArgs - funcSym.argNames->size();
-			warning("%d arg names defined for %d args! Dropping the last %d values", funcSym.argNames->size(), symNArgs, dropSize);
-			for (int i = 0; i < dropSize; i++) {
-				g_lingo->pop();
-				symNArgs -= 1;
-			}
-		} else if ((int)funcSym.argNames->size() > symNArgs) {
-			warning("%d arg names defined for %d args! Ignoring the last %d names", funcSym.argNames->size(), symNArgs, funcSym.argNames->size() - symNArgs);
-		}
-		for (int i = symNArgs - 1; i >= 0; i--) {
-			Common::String name = (*funcSym.argNames)[i];
-			if (!localvars->contains(name)) {
-				g_lingo->varCreate(name, false, localvars);
-				Datum arg(name);
-				arg.type = VAR;
-				Datum value = g_lingo->pop();
-				g_lingo->varAssign(arg, value, false, localvars);
-			} else {
-				warning("Argument %s already defined", name.c_str());
-				g_lingo->pop();
-			}
-		}
-	}
-	if (funcSym.varNames) {
-		for (Common::Array<Common::String>::iterator it = funcSym.varNames->begin(); it != funcSym.varNames->end(); ++it) {
-			Common::String name = *it;
-			if (!localvars->contains(name)) {
-				(*localvars)[name] = Datum();
-			} else {
-				warning("Variable %s already defined", name.c_str());
-			}
-		}
-	}
-	g_lingo->_localvars = localvars;
+	g_lingo->pushContext(funcSym, allowRetVal, defaultRetVal);
 
 	g_lingo->_pc = 0;
 }
@@ -1551,12 +1562,6 @@ void LC::c_procret() {
 		return;
 	}
 
-	CFrame *fp = g_lingo->_callstack.back();
-	if (g_lingo->_currentMe.type == OBJECT && g_lingo->_currentMe.u.obj->getObjType() == kFactoryObj && fp->sp.name->equalsIgnoreCase("mNew")) {
-		// Return the newly created object after executing mNew
-		g_lingo->push(g_lingo->_currentMe);
-	}
-
 	g_lingo->popContext();
 
 	if (g_lingo->_callstack.size() == 0) {
diff --git a/engines/director/lingo/lingo-code.h b/engines/director/lingo/lingo-code.h
index de4dbb00f8..8e826606b2 100644
--- a/engines/director/lingo/lingo-code.h
+++ b/engines/director/lingo/lingo-code.h
@@ -116,8 +116,8 @@ namespace LC {
 	void c_callcmd();
 	void c_callfunc();
 
-	void call(const Symbol &targetSym, int nargs);
-	void call(const Common::String &name, int nargs, SymbolType bltinType = VOIDSYM);
+	void call(const Symbol &targetSym, int nargs, bool allowRetVal = true);
+	void call(const Common::String &name, int nargs, bool allowRetVal = true);
 
 	void c_procret();
 
diff --git a/engines/director/lingo/lingo-events.cpp b/engines/director/lingo/lingo-events.cpp
index fa9be0f171..b061862479 100644
--- a/engines/director/lingo/lingo-events.cpp
+++ b/engines/director/lingo/lingo-events.cpp
@@ -341,7 +341,7 @@ void Lingo::processEvent(LEvent event, ScriptType st, int scriptId, int channelI
 
 	if (script && script->_eventHandlers.contains(event)) {
 		debugC(1, kDebugEvents, "Lingo::processEvent(%s, %s, %d): executing event handler", _eventHandlerTypes[event], scriptType2str(st), scriptId);
-		LC::call(script->_eventHandlers[event], 0);
+		LC::call(script->_eventHandlers[event], 0, false);
 		execute(_pc);
 	} else {
 		debugC(9, kDebugEvents, "Lingo::processEvent(%s, %s, %d): no handler", _eventHandlerTypes[event], scriptType2str(st), scriptId);
diff --git a/engines/director/lingo/lingo.cpp b/engines/director/lingo/lingo.cpp
index 7600fe8414..61de4c6fab 100644
--- a/engines/director/lingo/lingo.cpp
+++ b/engines/director/lingo/lingo.cpp
@@ -630,7 +630,7 @@ void Lingo::executeScript(ScriptType type, uint16 id) {
 void Lingo::executeHandler(const Common::String &name) {
 	debugC(1, kDebugLingoExec, "Executing script handler : %s", name.c_str());
 	Symbol sym = getHandler(name);
-	LC::call(sym, 0);
+	LC::call(sym, 0, false);
 	execute(_pc);
 }
 
@@ -1146,7 +1146,7 @@ void Lingo::executePerFrameHook(int frame, int subframe) {
 			debugC(1, kDebugLingoExec, "Executing perFrameHook : <%s>(mAtFrame, %d, %d)", _perFrameHook.asString(true).c_str(), frame, subframe);
 			push(Datum(frame));
 			push(Datum(subframe));
-			LC::call(method, 2);
+			LC::call(method, 2, false);
 			execute(_pc);
 		}
 	}
diff --git a/engines/director/lingo/lingo.h b/engines/director/lingo/lingo.h
index e3d939f40e..0637a7fba1 100644
--- a/engines/director/lingo/lingo.h
+++ b/engines/director/lingo/lingo.h
@@ -179,6 +179,9 @@ struct CFrame {	/* proc/func call stack frame */
 	LingoArchive *retarchive;	/* which archive to use after return */
 	DatumHash *localvars;
 	Datum retMe; /* which me obj to use after return */
+	uint stackSizeBefore;
+	bool allowRetVal;			/* whether to allow a return value */
+	Datum defaultRetVal;		/* default return value */
 };
 
 struct LingoEvent {
@@ -285,7 +288,7 @@ public:
 
 public:
 	void execute(uint pc);
-	void pushContext(const Symbol funcSym, bool preserveVarFrame = false);
+	void pushContext(const Symbol funcSym, bool allowRetVal, Datum defaultRetVal);
 	void popContext();
 	void cleanLocalVars();
 	int castIdFetch(Datum &var);


Commit: 3a694d7496b83b910e24a0a7e3141246c91f769c
    https://github.com/scummvm/scummvm/commit/3a694d7496b83b910e24a0a7e3141246c91f769c
Author: djsrv (dservilla at gmail.com)
Date: 2020-07-22T12:38:01-04:00

Commit Message:
DIRECTOR: LINGO: Handle more ARGCNORETs

Changed paths:
    engines/director/lingo/lingo-bytecode.cpp


diff --git a/engines/director/lingo/lingo-bytecode.cpp b/engines/director/lingo/lingo-bytecode.cpp
index f345e88adf..fbd742ec35 100644
--- a/engines/director/lingo/lingo-bytecode.cpp
+++ b/engines/director/lingo/lingo-bytecode.cpp
@@ -307,7 +307,7 @@ void LC::cb_localcall() {
 		if (debugChannelSet(3, kDebugLingoExec))
 			g_lingo->printSTUBWithArglist(name.c_str(), nargs.u.i, "localcall:");
 
-		LC::call(name, nargs.u.i);
+		LC::call(name, nargs.u.i, nargs.type == ARGC);
 
 	} else {
 		warning("cb_localcall: first arg should be of type ARGC or ARGCNORET, not %s", nargs.type2str());
@@ -394,7 +394,7 @@ void LC::cb_objectcall() {
 				g_lingo->push(args.back());
 				args.pop_back();
 			}
-			LC::call(method, nargs.u.i);
+			LC::call(method, nargs.u.i, nargs.type == ARGC);
 			return;
 		}
 
@@ -412,7 +412,7 @@ void LC::cb_objectcall() {
 			args.pop_back();
 		}
 
-		LC::call(func, nargs.u.i);
+		LC::call(func, nargs.u.i, nargs.type == ARGC);
 	} else {
 		warning("cb_objectcall: could not find object or function with name %s", d.u.s->c_str());
 		// Push a VOID to the stack if function is supposed to return


Commit: d61ca31e5613231e5c97663e318cbe83e0a6686d
    https://github.com/scummvm/scummvm/commit/d61ca31e5613231e5c97663e318cbe83e0a6686d
Author: djsrv (dservilla at gmail.com)
Date: 2020-07-22T12:38:01-04:00

Commit Message:
DIRECTOR: LINGO: Improve return value handling

Changed paths:
    engines/director/lingo/lingo-code.cpp


diff --git a/engines/director/lingo/lingo-code.cpp b/engines/director/lingo/lingo-code.cpp
index 0ecc13e242..9d00f01ec3 100644
--- a/engines/director/lingo/lingo-code.cpp
+++ b/engines/director/lingo/lingo-code.cpp
@@ -295,14 +295,20 @@ void Lingo::popContext() {
 	CFrame *fp = g_lingo->_callstack.back();
 	g_lingo->_callstack.pop_back();
 
-	uint maxRetVals = fp->allowRetVal ? 1 : 0;
-	if (_stack.size() > fp->stackSizeBefore + maxRetVals) {
+	if (_stack.size() == fp->stackSizeBefore + 1) {
+		if (!fp->allowRetVal) {
+			warning("dropping return value");
+			g_lingo->pop();
+		}
+	} else if (_stack.size() == fp->stackSizeBefore) {
+		if (fp->allowRetVal) {
+			warning("handler %s did not return value", fp->sp.name->c_str());
+			g_lingo->push(fp->defaultRetVal);
+		}
+	} else if (_stack.size() > fp->stackSizeBefore) {
 		error("handler %s returned extra %d values", fp->sp.name->c_str(), _stack.size() - fp->stackSizeBefore);
-	} else if (_stack.size() < fp->stackSizeBefore) {
+	} else {
 		error("handler %s popped extra %d values", fp->sp.name->c_str(), fp->stackSizeBefore - _stack.size());
-	} else if (fp->allowRetVal && _stack.size() == fp->stackSizeBefore) {
-		warning("handler %s did not return value", fp->sp.name->c_str());
-		g_lingo->push(fp->defaultRetVal);
 	}
 
 	// Destroy anonymous function context
@@ -1524,14 +1530,18 @@ void LC::call(const Symbol &funcSym, int nargs, bool allowRetVal) {
 		uint stackSize = g_lingo->_stack.size();
 
 		if (funcSym.u.bltin != LB::b_return && funcSym.u.bltin != LB::b_returnNumber && funcSym.u.bltin != LB::b_value) {
-			uint maxRetVals = allowRetVal ? 1 : 0;
-			if (stackSize > stackSizeBefore + maxRetVals) {
+			if (stackSize == stackSizeBefore + 1) {
+				if (!allowRetVal) {
+					warning("dropping return value");
+					g_lingo->pop();
+				}
+			} else if (stackSize == stackSizeBefore) {
+				if (allowRetVal)
+					error("builtin function %s did not return value", funcSym.name->c_str());
+			} else if (stackSize > stackSizeBefore) {
 				error("builtin %s returned extra %d values", funcSym.name->c_str(), stackSize - stackSizeBefore);
-			} else if (stackSize < stackSizeBefore) {
+			} else {
 				error("builtin %s popped extra %d values", funcSym.name->c_str(), stackSizeBefore - stackSize);
-			} else if (allowRetVal && stackSize == stackSizeBefore) {
-				error("builtin function %s did not return value", funcSym.name->c_str());
-				g_lingo->push(Datum());
 			}
 		}
 		return;




More information about the Scummvm-git-logs mailing list