SCI:

- Split SCI2 from SCI2.1 signature checks
- Added signature checking for the subops of the SCI2.1 kernel call kList
- Added support for calls to kAddToFront and kAddToEnd with 3 parameters (unimplemented code uncovered by the signature checks)
- Added a FIXME for a potential incorrect subop (uncovered by the signature checks), which might be related to the crashes in the demo of Torin's Passage

svn-id: r50861

3 files changed, 78 insertions, 71 deletions

diff --git a/engines/sci/engine/kernel.cpp b/engines/sci/engine/kernel.cpp
index 9cfa6a2dab..1e97671613 100644
--- a/engines/sci/engine/kernel.cpp
+++ b/engines/sci/engine/kernel.cpp
@@ -292,7 +292,8 @@ struct SciKernelMapSubEntry {
 #define SIG_SCI1           SCI_VERSION_1_EGA, SCI_VERSION_1_LATE
 #define SIG_SCI11          SCI_VERSION_1_1, SCI_VERSION_1_1
 #define SIG_SCI16          SCI_VERSION_NONE, SCI_VERSION_1_1
-#define SIG_SCI32          SCI_VERSION_2, SCI_VERSION_NONE
+#define SIG_SCI2           SCI_VERSION_2, SCI_VERSION_NONE
+#define SIG_SCI21          SCI_VERSION_2_1, SCI_VERSION_NONE
 
 // SCI-Sound-Version
 #define SIG_SOUNDSCI0      SCI_VERSION_0_EARLY, SCI_VERSION_0_LATE
@@ -380,7 +381,7 @@ static const SciKernelMapSubEntry kDoSound_subops[] = {
 
 //    version,         subId, function-mapping,                    signature,              workarounds
 static const SciKernelMapSubEntry kGraph_subops[] = {
-    { SIG_SCI32,           1, MAP_CALL(StubNull),                  "",                     NULL }, // called by gk1 sci32 right at the start
+    { SIG_SCI2,            1, MAP_CALL(StubNull),                  "",                     NULL }, // called by gk1 sci32 right at the start
     { SIG_SCIALL,          2, MAP_CALL(GraphGetColorCount),        "",                     NULL },
     // 3 - set palette via resource
     { SIG_SCIALL,          4, MAP_CALL(GraphDrawLine),             "iiiii(i)(i)",          NULL },
@@ -409,7 +410,7 @@ static const SciKernelMapSubEntry kPalVary_subops[] = {
     { SIG_SCIALL,          4, MAP_CALL(PalVaryChangeTarget),       "i",                    NULL },
     { SIG_SCIALL,          5, MAP_CALL(PalVaryChangeTicks),        "i",                    NULL },
     { SIG_SCIALL,          6, MAP_CALL(PalVaryPauseResume),        "i",                    NULL },
-    { SIG_SCI32,           8, MAP_CALL(PalVaryUnknown),            "",                     NULL },
+    { SIG_SCI2,            8, MAP_CALL(PalVaryUnknown),            "",                     NULL },
 	SCI_SUBOPENTRY_TERMINATOR
 };
 
@@ -426,6 +427,41 @@ static const SciKernelMapSubEntry kPalette_subops[] = {
 	SCI_SUBOPENTRY_TERMINATOR
 };
 
+#ifdef ENABLE_SCI32
+//    version,         subId, function-mapping,                    signature,              workarounds
+static const SciKernelMapSubEntry kList_subops[] = {
+    { SIG_SCI21,           0, MAP_CALL(NewList),                   "",                     NULL },
+    { SIG_SCI21,           1, MAP_CALL(DisposeList),               "l",                    NULL },
+	{ SIG_SCI21,           2, MAP_CALL(NewNode),                   ".",                    NULL },
+	{ SIG_SCI21,           3, MAP_CALL(FirstNode),                 "[l0]",                 NULL },
+	{ SIG_SCI21,           4, MAP_CALL(LastNode),                  "l",                    NULL },
+	{ SIG_SCI21,           5, MAP_CALL(EmptyList),                 "l",                    NULL },
+	{ SIG_SCI21,           6, MAP_CALL(NextNode),                  "n",                    NULL },
+	{ SIG_SCI21,           7, MAP_CALL(PrevNode),                  "n",                    NULL },
+	{ SIG_SCI21,           8, MAP_CALL(NodeValue),                 "[n0]",                 NULL },
+	{ SIG_SCI21,           9, MAP_CALL(AddAfter),                  "lnn.",                 NULL },
+	{ SIG_SCI21,          10, MAP_CALL(AddToFront),                "ln.",                  NULL },
+	{ SIG_SCI21,          11, MAP_CALL(AddToEnd),                  "ln.",                  NULL },
+	{ SIG_SCI21,          12, MAP_CALL(AddBefore),                 "ln.",                  NULL },
+	{ SIG_SCI21,          13, MAP_CALL(MoveToFront),               "ln",                   NULL },
+	{ SIG_SCI21,          14, MAP_CALL(MoveToEnd),                 "ln",                   NULL },
+	{ SIG_SCI21,          15, MAP_CALL(FindKey),                   "l.",                   NULL },
+	{ SIG_SCI21,          16, MAP_CALL(DeleteKey),                 "l.",                   NULL },
+	{ SIG_SCI21,          17, MAP_CALL(ListAt),                    "li",                   NULL },
+	// FIXME: This doesn't seem to be ListIndexOf. In Torin demo, an index is
+	// passed as a second parameter instead of an object. Thus, it seems to
+	// be something like ListAt instead... If we swap the two subops though,
+	// Torin demo crashes complaining that it tried to send to a non-object,
+	// therefore the semantics might be different here
+	{ SIG_SCI21,          18, MAP_CALL(ListIndexOf),               "l[o0]",                NULL },
+	{ SIG_SCI21,          19, MAP_CALL(ListEachElementDo),         "li(.*)",               NULL },
+	{ SIG_SCI21,          20, MAP_CALL(ListFirstTrue),             "li(.*)",               NULL },
+	{ SIG_SCI21,          21, MAP_CALL(ListAllTrue),               "li(.*)",               NULL },
+	{ SIG_SCI21,          22, MAP_CALL(Sort),                      "ooo",                  NULL },
+	SCI_SUBOPENTRY_TERMINATOR
+};
+#endif
+
 struct SciKernelMapEntry {
 	const char *name;
 	KernelFunctionCall *function;
@@ -455,7 +491,7 @@ static SciKernelMapEntry s_kernelMap[] = {
     { MAP_CALL(CantBeHere),        SIG_EVERYWHERE,           "o(l)",                  NULL,            NULL },
     { MAP_CALL(CelHigh),           SIG_EVERYWHERE,           "ii(i)",                 NULL,            NULL },
     { MAP_CALL(CelWide),           SIG_EVERYWHERE,           "ii(i)",                 NULL,            NULL },
-    { MAP_CALL(CheckFreeSpace),    SIG_SCI32, SIGFOR_ALL,    "r.*",                   NULL,            NULL },
+    { MAP_CALL(CheckFreeSpace),    SIG_SCI2, SIGFOR_ALL,     "r.*",                   NULL,            NULL },
     { MAP_CALL(CheckFreeSpace),    SIG_EVERYWHERE,           "r",                     NULL,            NULL },
     { MAP_CALL(CheckSaveGame),     SIG_EVERYWHERE,           ".*",                    NULL,            NULL },
     { MAP_CALL(Clone),             SIG_EVERYWHERE,           "o",                     NULL,            NULL },
@@ -505,11 +541,11 @@ static SciKernelMapEntry s_kernelMap[] = {
     { MAP_CALL(GetMenu),           SIG_EVERYWHERE,           "i.",                    NULL,            NULL },
     { MAP_CALL(GetMessage),        SIG_EVERYWHERE,           "iiir",                  NULL,            NULL },
     { MAP_CALL(GetPort),           SIG_EVERYWHERE,           "",                      NULL,            NULL },
-    { MAP_CALL(GetSaveDir),        SIG_SCI32, SIGFOR_ALL,    "(r*)",                  NULL,            NULL },
+    { MAP_CALL(GetSaveDir),        SIG_SCI2, SIGFOR_ALL,     "(r*)",                  NULL,            NULL },
     { MAP_CALL(GetSaveDir),        SIG_EVERYWHERE,           "",                      NULL,            NULL },
     { MAP_CALL(GetSaveFiles),      SIG_EVERYWHERE,           "rrr",                   NULL,            NULL },
     { MAP_CALL(GetTime),           SIG_EVERYWHERE,           "(i)",                   NULL,            NULL },
-    { MAP_CALL(GlobalToLocal),     SIG_SCI32, SIGFOR_ALL,    "oo",                    NULL,            NULL },
+    { MAP_CALL(GlobalToLocal),     SIG_SCI2, SIGFOR_ALL,     "oo",                    NULL,            NULL },
     { MAP_CALL(GlobalToLocal),     SIG_EVERYWHERE,           "o",                     NULL,            NULL },
     { MAP_CALL(Graph),             SIG_EVERYWHERE,           NULL,                    kGraph_subops,   NULL },
     { MAP_CALL(HaveMouse),         SIG_EVERYWHERE,           "",                      NULL,            NULL },
@@ -521,7 +557,7 @@ static SciKernelMapEntry s_kernelMap[] = {
     { MAP_CALL(Joystick),          SIG_EVERYWHERE,           "i(.*)",                 NULL,            NULL }, // subop
     { MAP_CALL(LastNode),          SIG_EVERYWHERE,           "l",                     NULL,            NULL },
     { MAP_CALL(Load),              SIG_EVERYWHERE,           "ii(i*)",                NULL,            NULL },
-    { MAP_CALL(LocalToGlobal),     SIG_SCI32, SIGFOR_ALL,    "oo",                    NULL,            NULL },
+    { MAP_CALL(LocalToGlobal),     SIG_SCI2, SIGFOR_ALL,     "oo",                    NULL,            NULL },
     { MAP_CALL(LocalToGlobal),     SIG_EVERYWHERE,           "o",                     NULL,            NULL },
     { MAP_CALL(Lock),              SIG_EVERYWHERE,           "ii(i)",                 NULL,            NULL },
     { MAP_CALL(MapKeyToDir),       SIG_EVERYWHERE,           "o",                     NULL,            NULL },
@@ -620,7 +656,7 @@ static SciKernelMapEntry s_kernelMap[] = {
     // SCI2.1 Kernel Functions
     { MAP_CALL(CD),           	   SIG_EVERYWHERE,           "(.*)",                  NULL,            NULL },
     { MAP_CALL(IsOnMe),            SIG_EVERYWHERE,           "iio(.*)",               NULL,            NULL },
-    { MAP_CALL(List),              SIG_EVERYWHERE,           "(.*)",                  NULL,            NULL },
+    { MAP_CALL(List),              SIG_EVERYWHERE,           "(.*)",                  kList_subops,    NULL },
     { MAP_CALL(MulDiv),            SIG_EVERYWHERE,           "iii",                   NULL,            NULL },
     { MAP_CALL(PlayVMD),           SIG_EVERYWHERE,           "(.*)",                  NULL,            NULL },
     { MAP_CALL(Robot),             SIG_EVERYWHERE,           "(.*)",                  NULL,            NULL },
diff --git a/engines/sci/engine/kernel.h b/engines/sci/engine/kernel.h
index fa206e8053..d63eeaa468 100644
--- a/engines/sci/engine/kernel.h
+++ b/engines/sci/engine/kernel.h
@@ -461,6 +461,9 @@ reg_t kPlayVMD(EngineState *s, int argc, reg_t *argv);
 reg_t kIsOnMe(EngineState *s, int argc, reg_t *argv);
 reg_t kCD(EngineState *s, int argc, reg_t *argv);
 
+reg_t kAddBefore(EngineState *s, int argc, reg_t *argv);
+reg_t kMoveToFront(EngineState *s, int argc, reg_t *argv);
+reg_t kMoveToEnd(EngineState *s, int argc, reg_t *argv);
 #endif
 
 reg_t kDoSoundInit(EngineState *s, int argc, reg_t *argv);
diff --git a/engines/sci/engine/klists.cpp b/engines/sci/engine/klists.cpp
index 0701883a9b..06fc68b82b 100644
--- a/engines/sci/engine/klists.cpp
+++ b/engines/sci/engine/klists.cpp
@@ -251,6 +251,19 @@ reg_t kNodeValue(EngineState *s, int argc, reg_t *argv) {
 
 reg_t kAddToFront(EngineState *s, int argc, reg_t *argv) {
 	addToFront(s, argv[0], argv[1]);
+
+	if (argc == 3)
+		s->_segMan->lookupNode(argv[1])->key = argv[2];
+
+	return s->r_acc;
+}
+
+reg_t kAddToEnd(EngineState *s, int argc, reg_t *argv) {
+	addToEnd(s, argv[0], argv[1]);
+
+	if (argc == 3)
+		s->_segMan->lookupNode(argv[1])->key = argv[2];
+
 	return s->r_acc;
 }
 
@@ -294,11 +307,6 @@ reg_t kAddAfter(EngineState *s, int argc, reg_t *argv) {
 	return s->r_acc;
 }
 
-reg_t kAddToEnd(EngineState *s, int argc, reg_t *argv) {
-	addToEnd(s, argv[0], argv[1]);
-	return s->r_acc;
-}
-
 reg_t kFindKey(EngineState *s, int argc, reg_t *argv) {
 	reg_t node_pos;
 	reg_t key = argv[1];
@@ -580,65 +588,25 @@ reg_t kListAllTrue(EngineState *s, int argc, reg_t *argv) {
 	return s->r_acc;
 }
 
-// In SCI2.1, all the list functions were merged in one
 reg_t kList(EngineState *s, int argc, reg_t *argv) {
-	switch (argv[0].toUint16()) {
-	case 0:
-		return kNewList(s, argc - 1, argv + 1);
-	case 1:
-		return kDisposeList(s, argc - 1, argv + 1);
-	case 2:
-		return kNewNode(s, argc - 1, argv + 1);
-	case 3:
-		return kFirstNode(s, argc - 1, argv + 1);
-	case 4:
-		return kLastNode(s, argc - 1, argv + 1);
-	case 5:
-		return kEmptyList(s, argc - 1, argv + 1);
-	case 6:
-		return kNextNode(s, argc - 1, argv + 1);
-	case 7:
-		return kPrevNode(s, argc - 1, argv + 1);
-	case 8:
-		return kNodeValue(s, argc - 1, argv + 1);
-	case 9:
-		return kAddAfter(s, argc - 1, argv + 1);
-	case 10:
-		return kAddToFront(s, argc - 1, argv + 1);
-	case 11:
-		return kAddToEnd(s, argc - 1, argv + 1);
-	case 12:
-		error("kList: unimplemented subfunction kAddBefore");
-		//return kAddBefore(s, argc - 1, argv + 1);
-		return NULL_REG;
-	case 13:
-		error("kList: unimplemented subfunction kMoveToFront");
-		//return kMoveToFront(s, argc - 1, argv + 1);
-		return NULL_REG;
-	case 14:
-		error("kList: unimplemented subfunction kMoveToEnd");
-		//return kMoveToEnd(s, argc - 1, argv + 1);
-		return NULL_REG;
-	case 15:
-		return kFindKey(s, argc - 1, argv + 1);
-	case 16:
-		return kDeleteKey(s, argc - 1, argv + 1);
-	case 17:
-		return kListAt(s, argc - 1, argv + 1);
-	case 18:
-		return kListIndexOf(s, argc - 1, argv + 1);
-	case 19:
-		return kListEachElementDo(s, argc - 1, argv + 1);
-	case 20:
-		return kListFirstTrue(s, argc - 1, argv + 1);
-	case 21:
-		return kListAllTrue(s, argc - 1, argv + 1);
-	case 22:
-		return kSort(s, argc - 1, argv + 1);
-	default:
-		error("kList: Unhandled case %d", argv[0].toUint16());
-		return NULL_REG;
-	}
+	if (!s)
+		return make_reg(0, getSciVersion());
+	error("not supposed to call this");
+}
+
+reg_t kAddBefore(EngineState *s, int argc, reg_t *argv) {
+	error("Unimplemented function kAddBefore called");
+	return s->r_acc;
+}
+
+reg_t kMoveToFront(EngineState *s, int argc, reg_t *argv) {
+	error("Unimplemented function kMoveToFront called");
+	return s->r_acc;
+}
+
+reg_t kMoveToEnd(EngineState *s, int argc, reg_t *argv) {
+	error("Unimplemented function kMoveToEnd called");
+	return s->r_acc;
 }
 
 #endif