- Rewrote kSetCursor to be a bit simpler to understand, and got rid of GF_SCI1_NEWSETCURSOR

- Removed the 3 mouse pointer view, loop and cell variables (and their 3 "save" versions) from the game state, as they're all actually not used anywhere
- Cleanup

svn-id: r40976

8 files changed, 55 insertions, 81 deletions

diff --git a/engines/sci/detection.cpp b/engines/sci/detection.cpp
index 07e1378e91..38095e446e 100644
--- a/engines/sci/detection.cpp
+++ b/engines/sci/detection.cpp
@@ -513,7 +513,7 @@ static const struct SciGameDescription SciGameDescriptions[] = {
 		{"resource.002", 0, "28fe9b4f0567e71feb198bc9f3a2c605", 1241816},
 		{"resource.003", 0, "f3146df0ad4297f5ce35aa8c4753bf6c", 586832},
 		{NULL, 0, NULL, 0}}, Common::EN_ANY, Common::kPlatformPC, 0},
-		GF_FOR_SCI1_510_OR_LATER | GF_SCI1_NEWSETCURSOR,
+		GF_FOR_SCI1_510_OR_LATER,
 		SCI_VERSION_AUTODETECT,
 		SCI_VERSION_1
 	},
@@ -527,7 +527,7 @@ static const struct SciGameDescription SciGameDescriptions[] = {
 		{"resource.002", 0, "323b3b12f43d53f27d259beb225f0aa7", 1129316},
 		{"resource.003", 0, "83ac03e4bddb2c1ac2d36d2a587d0536", 1145616},
 		{NULL, 0, NULL, 0}}, Common::EN_ANY, Common::kPlatformPC, 0},
-		GF_FOR_SCI1_510_OR_LATER | GF_SCI1_NEWSETCURSOR,
+		GF_FOR_SCI1_510_OR_LATER,
 		SCI_VERSION_AUTODETECT,
 		SCI_VERSION_1
 	},
@@ -541,7 +541,7 @@ static const struct SciGameDescription SciGameDescriptions[] = {
 		{"resource.002", 0, "02d7d0411f7903aacb3bc8b0f8ca8a9a", 1202581},
 		{"resource.003", 0, "84dd11b6825255671c703aee5ceff620", 1175835},
 		{NULL, 0, NULL, 0}}, Common::DE_DEU, Common::kPlatformPC, 0},
-		GF_FOR_SCI1_510_OR_LATER | GF_SCI1_NEWSETCURSOR,
+		GF_FOR_SCI1_510_OR_LATER,
 		SCI_VERSION_AUTODETECT,
 		SCI_VERSION_1
 	},
@@ -556,7 +556,7 @@ static const struct SciGameDescription SciGameDescriptions[] = {
 		{"resource.002", 0, "2d21a1d2dcbffa551552e3e0725d2284", 1186033},
 		{"resource.003", 0, "84dd11b6825255671c703aee5ceff620", 1174993},
 		{NULL, 0, NULL, 0}}, Common::ES_ESP, Common::kPlatformPC, 0},
-		GF_FOR_SCI1_510_OR_LATER | GF_SCI1_NEWSETCURSOR,
+		GF_FOR_SCI1_510_OR_LATER,
 		SCI_VERSION_AUTODETECT,
 		SCI_VERSION_1
 	},
@@ -1110,7 +1110,7 @@ static const struct SciGameDescription SciGameDescriptions[] = {
 		{"resource.000", 0, "449471bfd77be52f18a3773c7f7d843d", 571368},
 		{"resource.001", 0, "b45a581ff8751e052c7e364f58d3617f", 16800210},
 		{NULL, 0, NULL, 0}}, Common::EN_ANY, Common::kPlatformPC, 0},
-		GF_FOR_SCI1_510_OR_LATER | GF_SCI1_NEWSETCURSOR,
+		GF_FOR_SCI1_510_OR_LATER,
 		SCI_VERSION_AUTODETECT,
 		SCI_VERSION_1
 	},
diff --git a/engines/sci/engine/game.cpp b/engines/sci/engine/game.cpp
index 3ae209c662..363549eacd 100644
--- a/engines/sci/engine/game.cpp
+++ b/engines/sci/engine/game.cpp
@@ -109,14 +109,8 @@ int _reset_graphics_input(EngineState *s) {
 	gfxop_fill_box(s->gfx_state, gfx_rect(0, 0, 320, 200), s->ega_colors[0]); // Fill screen black
 	gfxop_update(s->gfx_state);
 
-	s->mouse_pointer_view = s->mouse_pointer_loop = s->mouse_pointer_cel = -1; // No mouse pointer resource
-	s->save_mouse_pointer_view = s->save_mouse_pointer_loop = s->save_mouse_pointer_cel = -1; // No mouse pointer resource
 	gfxop_set_pointer_position(s->gfx_state, Common::Point(160, 150));
 
-	s->mouse_pointer_view = s->mouse_pointer_loop = s->mouse_pointer_cel = -1; // No mouse pointer resource
-	s->save_mouse_pointer_view = s->save_mouse_pointer_loop = s->save_mouse_pointer_cel = -1; // No mouse pointer resource
-
-
 	s->pic_is_new = 0;
 	s->pic_visible_map = GFX_MASK_NONE; // Other values only make sense for debugging
 	s->dyn_views = NULL; // no DynViews
diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index c4b017643e..a587f4eb87 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -302,53 +302,56 @@ static gfx_color_t graph_map_color(EngineState *s, int color, int priority, int
 
 reg_t kSetCursor(EngineState *s, int funct_nr, int argc, reg_t *argv) {
 	switch (argc) {
-	case 1 :
-		if (UKPV(0) == 0) {
-			s->save_mouse_pointer_view = s->mouse_pointer_view;
-			s->save_mouse_pointer_loop = s->mouse_pointer_loop;
-			s->save_mouse_pointer_cel = s->mouse_pointer_cel;
-			s->mouse_pointer_view = s->mouse_pointer_loop = s->mouse_pointer_cel = -1;
-			gfxop_set_pointer_cursor(s->gfx_state, GFXOP_NO_POINTER);
+	case 1 :	// set cursor according to the first parameter
+		GFX_ASSERT(gfxop_set_pointer_cursor(s->gfx_state, SKPV(0)));
+		break;
+	case 2 :
+		if (s->version < SCI_VERSION_1_1) {
+			// Pre-SCI1.1: set cursor according to the first parameter, and toggle its
+			// visibility based on the second parameter
+			// Some late SCI1 games actually use the SCI1.1 version of this call (EcoQuest 1
+			// and KQ5 CD, but I haven't seen this case happen), but we can determine the
+			// semantics from the second parameter passed.
+			// Rationale: with the older behavior, the second parameter can either be 0 
+			// (hide cursor) or 1/-1 (show cursor). This could be problematic if the engine
+			// tries to place the cursor at (x, 0) or (x, 1), but no SCI1 game does that, as
+			// this would open the menu on top. LSL5 is an exception, as the game can open
+			// the menu when the player presses a button during the intro, but the cursor is
+			// not placed on (x, 0) or (x, 1)
+			int param2 = SKPV(1);
+			if (param2 == 0 || param2 == 1 || param2 == -1) {
+				GFX_ASSERT(gfxop_set_pointer_cursor(s->gfx_state, 
+							param2 == 0 ? GFXOP_NO_POINTER : SKPV(0)));
+			} else {	// newer (SCI1.1) semantics: set pointer position
+				GFX_ASSERT(gfxop_set_pointer_position(s->gfx_state, 
+							Common::Point(UKPV(0), UKPV(1))));
+			}
 		} else {
-			s->mouse_pointer_view = s->save_mouse_pointer_view;
-			s->mouse_pointer_loop = s->save_mouse_pointer_loop;
-			s->mouse_pointer_cel = s->save_mouse_pointer_cel;
+			// SCI1.1 and newer: set pointer position
+			GFX_ASSERT(gfxop_set_pointer_position(s->gfx_state, 
+						Common::Point(UKPV(0), UKPV(1))));
 		}
-	case 2 :
+		break;
 	case 4 :
-		if (s->version >= SCI_VERSION_1_1 || (s->flags & GF_SCI1_NEWSETCURSOR)) {
-			GFX_ASSERT(gfxop_set_pointer_position(s->gfx_state, Common::Point(UKPV(0), UKPV(1))));
-		} else {
-			if (SKPV_OR_ALT(1, 1)) {
-				s->mouse_pointer_view = SKPV(0);
-			} else
-				s->mouse_pointer_view = GFXOP_NO_POINTER;
-
-			s->mouse_pointer_loop = s->mouse_pointer_cel = 0; // Not used with cursor-format pointers
-
-			GFX_ASSERT(gfxop_set_pointer_cursor(s->gfx_state, s->mouse_pointer_view));
+		GFX_ASSERT(gfxop_set_pointer_cursor(s->gfx_state, 
+					UKPV(0) == 0 ? GFXOP_NO_POINTER : SKPV(0)));
 
-			if (argc > 2) {
-				Common::Point newpos = Common::Point(SKPV(2) + s->port->_bounds.x, SKPV(3) + s->port->_bounds.y);
-				GFX_ASSERT(gfxop_set_pointer_position(s->gfx_state, newpos));
-			}
+		// Set pointer position, if requested
+		if (argc > 2) {
+			Common::Point newpos = Common::Point(SKPV(2) + s->port->_bounds.x, SKPV(3) + s->port->_bounds.y);
+			GFX_ASSERT(gfxop_set_pointer_position(s->gfx_state, newpos));
 		}
 		break;
-	case 3 : {
-		GFX_ASSERT(gfxop_set_pointer_view(s->gfx_state, UKPV(0), UKPV(1), UKPV(2), NULL));
-		s->mouse_pointer_view = UKPV(0);
-		s->mouse_pointer_loop = UKPV(1);
-		s->mouse_pointer_cel = UKPV(2);
-		break;
-	}
-	case 9 : {
-		Common::Point hotspot = Common::Point(SKPV(3), SKPV(4));
-
-//		sciprintf("Setting hotspot at %d/%d\n", hotspot.x, hotspot.y);
-
-		gfxop_set_pointer_view(s->gfx_state, UKPV(0), UKPV(1), UKPV(2), &hotspot);
+	case 3 :
+	case 5 :
+	case 9 :
+		if (argc > 3) {
+			Common::Point hotspot = Common::Point(SKPV(3), SKPV(4));
+			GFX_ASSERT(gfxop_set_pointer_view(s->gfx_state, UKPV(0), UKPV(1), UKPV(2), &hotspot));
+		} else {
+			GFX_ASSERT(gfxop_set_pointer_view(s->gfx_state, UKPV(0), UKPV(1), UKPV(2), NULL));
+		}
 		break;
-	}
 	default :
 		error("kSetCursor: Unhandled case: %d arguments given", argc);
 		break;
diff --git a/engines/sci/engine/state.cpp b/engines/sci/engine/state.cpp
index 70f5550722..ee0156aacf 100644
--- a/engines/sci/engine/state.cpp
+++ b/engines/sci/engine/state.cpp
@@ -56,13 +56,6 @@ EngineState::EngineState() : _dirseeker(this) {
 
 	game_time = 0;
 
-	mouse_pointer_view = 0;
-	mouse_pointer_loop = 0;
-	mouse_pointer_cel = 0;
-	save_mouse_pointer_view = 0;
-	save_mouse_pointer_loop = 0;
-	save_mouse_pointer_cel = 0;
-
 	port = 0;
 
 	memset(ega_colors, 0, sizeof(ega_colors));
diff --git a/engines/sci/engine/state.h b/engines/sci/engine/state.h
index 2db88c3288..c56a0e8e7b 100644
--- a/engines/sci/engine/state.h
+++ b/engines/sci/engine/state.h
@@ -151,13 +151,6 @@ public:
 
 	long game_time; /**< Counted at 60 ticks per second, reset during start time */
 
-	int mouse_pointer_view; /**< Mouse pointer resource, or -1 if disabled */
-	int mouse_pointer_loop; /**< Mouse pointer resource, or -1 if disabled */
-	int mouse_pointer_cel; /**< Mouse pointer resource, or -1 if disabled */
-	int save_mouse_pointer_view; /**< Temporary storage for mouse pointer resource, when the pointer is hidden */
-	int save_mouse_pointer_loop; /**< Temporary storage for mouse pointer resource, when the pointer is hidden */
-	int save_mouse_pointer_cel; /**< Temporary storage for mouse pointer resource, when the pointer is hidden */
-
 	GfxPort *port; /**< The currently active port */
 
 	gfx_color_t ega_colors[16]; /**< The 16 EGA colors- for SCI0(1) */
diff --git a/engines/sci/gfx/operations.cpp b/engines/sci/gfx/operations.cpp
index e2c33b54c7..2fd115bf01 100644
--- a/engines/sci/gfx/operations.cpp
+++ b/engines/sci/gfx/operations.cpp
@@ -1208,13 +1208,12 @@ int gfxop_set_pointer_cursor(GfxState *state, int nr) {
 }
 
 int gfxop_set_pointer_view(GfxState *state, int nr, int loop, int cel, Common::Point *hotspot) {
-	int real_loop = loop;
-	int real_cel = cel;
-	gfx_pixmap_t *new_pointer = NULL;
-
 	BASIC_CHECKS(GFX_FATAL);
 
-	new_pointer = _gfxr_get_cel(state, nr, &real_loop, &real_cel, 0); // FIXME: For now, don't palettize pointers
+	int real_loop = loop;
+	int real_cel = cel;
+	// FIXME: For now, don't palettize pointers
+	gfx_pixmap_t *new_pointer = _gfxr_get_cel(state, nr, &real_loop, &real_cel, 0);
 
 	if (!new_pointer) {
 		GFXWARN("Attempt to set invalid pointer #%d\n", nr);
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index d12a791c88..1e6384e684 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -195,8 +195,7 @@ Common::Error SciEngine::run() {
 		// SCI0/SCI01
 		if (gamestate->flags & GF_SCI1_EGA ||
 			gamestate->flags & GF_SCI1_LOFSABSOLUTE ||
-			gamestate->flags & GF_SCI1_NEWDOSOUND ||
-			gamestate->flags & GF_SCI1_NEWSETCURSOR) {
+			gamestate->flags & GF_SCI1_NEWDOSOUND) {
 			error("This game entry is erroneous. It's marked as SCI0/SCI01, but it has SCI1 flags set");
 		}
 	} else if (version >= SCI_VERSION_1_EARLY && version <= SCI_VERSION_1_LATE) {
@@ -210,8 +209,7 @@ Common::Error SciEngine::run() {
 	} else if (version == SCI_VERSION_1_1 || version == SCI_VERSION_32) {
 		if (gamestate->flags & GF_SCI1_EGA ||
 			gamestate->flags & GF_SCI1_LOFSABSOLUTE ||
-			gamestate->flags & GF_SCI1_NEWDOSOUND ||
-			gamestate->flags & GF_SCI1_NEWSETCURSOR) {
+			gamestate->flags & GF_SCI1_NEWDOSOUND) {
 			error("This game entry is erroneous. It's marked as SCI1.1/SCI32, but it has SCI1 flags set");
 		}
 
diff --git a/engines/sci/sci.h b/engines/sci/sci.h
index 096a1a3c7e..bc4be89b22 100644
--- a/engines/sci/sci.h
+++ b/engines/sci/sci.h
@@ -126,13 +126,7 @@ enum SciGameFlags {
 	/* Applies to all versions from 1.000.510 onwards
 	** kDoSound() is different than in earlier SCI1 versions.
 	*/
-	GF_SCI1_NEWDOSOUND		= (1 << 5),
-
-	/*
-	** Newer SCI1 games (like KQ5 CD and EcoQuest 1) use
-	** different semantics in kSetCursor(), like SCI1.1 games
-	*/
-	GF_SCI1_NEWSETCURSOR	= (1 << 6)
+	GF_SCI1_NEWDOSOUND		= (1 << 5)
 };
 
 class SciEngine : public Engine {