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

csnover csnover at users.noreply.github.com
Thu Sep 28 03:29:13 CEST 2017


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

Summary:
7feccaaa98 SCI32: Implement SCI3-variant volume handling
c88d5519c2 SCI: Add uninitialized read workaround for shoplifting in SQ4CD
9a8070da3c SCI: Do some clean-up of event handling system
4bd31dae9b SCI: Fix use-after-free when kernel call debugging is active during a save game restore
7a41b6f023 SCI: Add support for keyup events
a4eb6a5ca5 SCI: Remove old SCI32 view scaling code from SCI16 graphics code


Commit: 7feccaaa98378eb586501c9de8c2d4ca1bb00aa4
    https://github.com/scummvm/scummvm/commit/7feccaaa98378eb586501c9de8c2d4ca1bb00aa4
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-27T20:27:33-05:00

Commit Message:
SCI32: Implement SCI3-variant volume handling

Trying to find differences in Lighthouse's audio sample playback,
I discovered that SCI3 had its own variant of handling volumes and
sending this volume information back to game scripts. It is not
known if this fixes any sound bug.

Changed paths:
    engines/sci/sound/audio32.cpp
    engines/sci/sound/soundcmd.cpp


diff --git a/engines/sci/sound/audio32.cpp b/engines/sci/sound/audio32.cpp
index 2f7338e..59ebc86 100644
--- a/engines/sci/sound/audio32.cpp
+++ b/engines/sci/sound/audio32.cpp
@@ -1061,10 +1061,23 @@ void Audio32::setLoop(const int16 channelIndex, const bool loop) {
 #pragma mark Effects
 
 int16 Audio32::getVolume(const int16 channelIndex) const {
-	if (channelIndex < 0 || channelIndex >= _numActiveChannels) {
+	bool getGlobalVolume = false;
+	switch (getSciVersion()) {
+	case SCI_VERSION_3:
+		getGlobalVolume = (channelIndex == kAllChannels);
+		break;
+	default:
+		getGlobalVolume = (channelIndex < 0 || channelIndex >= _numActiveChannels);
+	}
+
+	if (getGlobalVolume) {
 		return (_mixer->getVolumeForSoundType(Audio::Mixer::kSFXSoundType) + 1) * kMaxVolume / Audio::Mixer::kMaxMixerVolume;
 	}
 
+	if (channelIndex < 0 || channelIndex >= _numActiveChannels) {
+		return -1;
+	}
+
 	Common::StackLock lock(_mutex);
 	return getChannel(channelIndex).volume;
 }
@@ -1266,7 +1279,13 @@ reg_t Audio32::kernelVolume(const int argc, const reg_t *const argv) {
 	Common::StackLock lock(_mutex);
 
 	const int16 volume = argc > 0 ? argv[0].toSint16() : -1;
-	const int16 channelIndex = findChannelByArgs(argc, argv, 1, argc > 2 ? argv[2] : NULL_REG);
+	int16 channelIndex;
+
+	if (getSciVersion() == SCI_VERSION_3 && argc < 2) {
+		channelIndex = kAllChannels;
+	} else {
+		channelIndex = findChannelByArgs(argc, argv, 1, argc > 2 ? argv[2] : NULL_REG);
+	}
 
 	if (volume != -1) {
 		setVolume(channelIndex, volume);
diff --git a/engines/sci/sound/soundcmd.cpp b/engines/sci/sound/soundcmd.cpp
index e8fd20d..0bb295d 100644
--- a/engines/sci/sound/soundcmd.cpp
+++ b/engines/sci/sound/soundcmd.cpp
@@ -514,8 +514,19 @@ void SoundCommandParser::processUpdateCues(reg_t obj) {
 	if (musicSlot->isSample) {
 #ifdef ENABLE_SCI32
 		if (_soundVersion >= SCI_VERSION_2) {
-			const int position = g_sci->_audio32->getPosition(ResourceId(kResourceTypeAudio, musicSlot->resourceId), musicSlot->soundObj);
+			const ResourceId audioId = ResourceId(kResourceTypeAudio, musicSlot->resourceId);
+
+			if (getSciVersion() == SCI_VERSION_3) {
+				// In SSCI the volume is first set to -1 and then reset later if
+				// a sample is playing in the audio player, but since our audio
+				// code returns -1 for not-found samples, the extra check is not
+				// needed and we can just always set it to the return value of
+				// the getVolume call
+				const int16 volume = g_sci->_audio32->getVolume(audioId, musicSlot->soundObj);
+				writeSelectorValue(_segMan, musicSlot->soundObj, SELECTOR(vol), volume);
+			}
 
+			const int16 position = g_sci->_audio32->getPosition(audioId, musicSlot->soundObj);
 			if (position == -1) {
 				processStopSound(musicSlot->soundObj, true);
 			}


Commit: c88d5519c2e2672ce7faabfa52f36af4a8706cba
    https://github.com/scummvm/scummvm/commit/c88d5519c2e2672ce7faabfa52f36af4a8706cba
Author: David Fioramonti (dafioram at gmail.com)
Date: 2017-09-27T20:27:33-05:00

Commit Message:
SCI: Add uninitialized read workaround for shoplifting in SQ4CD

When in Galaxy Galleria, going into the software store and trying
to shoplift the SQ4 Hintbook will crash the game after you leave
and are electrocuted.

Fixes Trac#10229. Closes gh-1031.

Changed paths:
    engines/sci/engine/workarounds.cpp


diff --git a/engines/sci/engine/workarounds.cpp b/engines/sci/engine/workarounds.cpp
index 0254ead..9c271db 100644
--- a/engines/sci/engine/workarounds.cpp
+++ b/engines/sci/engine/workarounds.cpp
@@ -434,6 +434,7 @@ const SciWorkaroundEntry uninitializedReadWorkarounds[] = {
 	{ GID_SQ1,            -1,   703,  0,         "firePulsar", "changeState",     sig_uninitread_sq1_1,     0, { WORKAROUND_FAKE,   0 } }, // export 1, but called locally (when shooting at aliens)
 	{ GID_SQ4,            -1,   398,  0,            "showBox", "changeState",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // CD: called when rummaging in Software Excess bargain bin
 	{ GID_SQ4,            -1,   928, -1,           "Narrator", "startText",                       NULL,  1000, { WORKAROUND_FAKE,   1 } }, // CD: happens in the options dialog and in-game when speech and subtitles are used simultaneously
+	{ GID_SQ4,           395,   395, -1,    "fromStoreScript", "changeState",                     NULL,     0, { WORKAROUND_FAKE,   0 } }, // CD: happens when shoplifting in Galaxy Galleria - bug #10229
 	{ GID_SQ4,            -1,   708, -1,            "exitBut", "doVerb",                          NULL,     0, { WORKAROUND_FAKE,   0 } }, // Floppy: happens, when looking at the "close" button in the sq4 hintbook - bug #6447
 	{ GID_SQ4,            -1,   708, -1,                   "", "doVerb",                          NULL,     0, { WORKAROUND_FAKE,   0 } }, // Floppy: happens, when looking at the "close" button... in Russian version - bug #5573
 	{ GID_SQ4,            -1,   708, -1,            "prevBut", "doVerb",                          NULL,     0, { WORKAROUND_FAKE,   0 } }, // Floppy: happens, when looking at the "previous" button in the sq4 hintbook - bug #6447


Commit: 9a8070da3c533dd4885e8044051a5e1a9caac9bb
    https://github.com/scummvm/scummvm/commit/9a8070da3c533dd4885e8044051a5e1a9caac9bb
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-27T20:27:33-05:00

Commit Message:
SCI: Do some clean-up of event handling system

Convert macros and vars to enums, rename keyboard events in
preparation for adding key up events, clean up unnecessary nested
conditionals, add TODOs for potential future work.

Changed paths:
    engines/sci/engine/kevent.cpp
    engines/sci/engine/kgraphics.cpp
    engines/sci/event.cpp
    engines/sci/event.h
    engines/sci/graphics/controls16.cpp
    engines/sci/graphics/controls32.cpp
    engines/sci/graphics/cursor.cpp
    engines/sci/graphics/frameout.h
    engines/sci/graphics/maciconbar.cpp
    engines/sci/graphics/menu.cpp
    engines/sci/graphics/portrait.cpp
    engines/sci/graphics/video32.cpp
    engines/sci/sci.cpp


diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 735382c..0f77b47 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -42,10 +42,9 @@
 namespace Sci {
 
 reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
-	int mask = argv[0].toUint16();
+	SciEventType mask = (SciEventType)argv[0].toUint16();
 	reg_t obj = argv[1];
 	SciEvent curEvent;
-	int modifier_mask = getSciVersion() <= SCI_VERSION_01 ? SCI_KEYMOD_ALL : SCI_KEYMOD_NO_FOOLOCK;
 	uint16 modifiers = 0;
 	SegManager *segMan = s->_segMan;
 	Common::Point mousePos;
@@ -59,16 +58,16 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 
 	// If there's a simkey pending, and the game wants a keyboard event, use the
 	// simkey instead of a normal event
-	if (g_debug_simulated_key && (mask & SCI_EVENT_KEYBOARD)) {
+	if (g_debug_simulated_key && (mask & kSciEventKeyDown)) {
 		// In case we use a simulated event we query the current mouse position
 		mousePos = g_sci->_gfxCursor->getPosition();
 
 		// Limit the mouse cursor position, if necessary
 		g_sci->_gfxCursor->refreshPosition();
 
-		writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_KEYBOARD); // Keyboard event
+		writeSelectorValue(segMan, obj, SELECTOR(type), kSciEventKeyDown);
 		writeSelectorValue(segMan, obj, SELECTOR(message), g_debug_simulated_key);
-		writeSelectorValue(segMan, obj, SELECTOR(modifiers), SCI_KEYMOD_NUMLOCK); // Numlock on
+		writeSelectorValue(segMan, obj, SELECTOR(modifiers), kSciKeyModNumLock);
 		writeSelectorValue(segMan, obj, SELECTOR(x), mousePos.x);
 		writeSelectorValue(segMan, obj, SELECTOR(y), mousePos.y);
 		g_debug_simulated_key = 0;
@@ -153,8 +152,9 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 	writeSelectorValue(segMan, obj, SELECTOR(y), mousePos.y);
 
 	// Get current keyboard modifiers, only keep relevant bits
-	modifiers = curEvent.modifiers & modifier_mask;
-	if (g_sci->getPlatform() == Common::kPlatformDOS) {
+	const int modifierMask = getSciVersion() <= SCI_VERSION_01 ? kSciKeyModAll : kSciKeyModNonSticky;
+	modifiers = curEvent.modifiers & modifierMask;
+	if (g_sci->getPlatform() == Common::kPlatformDOS && getSciVersion() <= SCI_VERSION_01) {
 		// We are supposed to emulate SCI running in DOS
 
 		// We set the higher byte of the modifiers to 02h
@@ -169,30 +169,28 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 		// SCI32 also resets the upper byte.
 
 		// This was verified in SSCI itself by creating a SCI game and checking behavior.
-		if (getSciVersion() <= SCI_VERSION_01) {
-			modifiers |= 0x0200;
-		}
+		modifiers |= 0x0200;
 	}
 
 	switch (curEvent.type) {
-	case SCI_EVENT_QUIT:
+	case kSciEventQuit:
 		s->abortScriptProcessing = kAbortQuitGame; // Terminate VM
 		g_sci->_debugState.seeking = kDebugSeekNothing;
 		g_sci->_debugState.runningStep = 0;
 		break;
 
-	case SCI_EVENT_KEYBOARD:
-		writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_KEYBOARD); // Keyboard event
+	case kSciEventKeyDown:
+		writeSelectorValue(segMan, obj, SELECTOR(type), curEvent.type);
 		writeSelectorValue(segMan, obj, SELECTOR(message), curEvent.character);
 		// We only care about the translated character
 		writeSelectorValue(segMan, obj, SELECTOR(modifiers), modifiers);
 		s->r_acc = TRUE_REG;
 		break;
 
-	case SCI_EVENT_MOUSE_RELEASE:
-	case SCI_EVENT_MOUSE_PRESS:
+	case kSciEventMouseRelease:
+	case kSciEventMousePress:
 		// track left buttton clicks, if requested
-		if (curEvent.type == SCI_EVENT_MOUSE_PRESS && curEvent.modifiers == 0 && g_debug_track_mouse_clicks) {
+		if (curEvent.type == kSciEventMousePress && curEvent.modifiers == 0 && g_debug_track_mouse_clicks) {
 			g_sci->getSciDebugger()->debugPrintf("Mouse clicked at %d, %d\n",
 						mousePos.x, mousePos.y);
 		}
@@ -206,7 +204,7 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 		break;
 
 #ifdef ENABLE_SCI32
-	case SCI_EVENT_HOT_RECTANGLE:
+	case kSciEventHotRectangle:
 		writeSelectorValue(segMan, obj, SELECTOR(type), curEvent.type);
 		writeSelectorValue(segMan, obj, SELECTOR(message), curEvent.hotRectangleIndex);
 		s->r_acc = TRUE_REG;
@@ -215,7 +213,7 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 
 	default:
 		// Return a null event
-		writeSelectorValue(segMan, obj, SELECTOR(type), SCI_EVENT_NONE);
+		writeSelectorValue(segMan, obj, SELECTOR(type), kSciEventNone);
 		writeSelectorValue(segMan, obj, SELECTOR(message), 0);
 		writeSelectorValue(segMan, obj, SELECTOR(modifiers), modifiers);
 		s->r_acc = NULL_REG;
@@ -228,14 +226,14 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 		Console *con = g_sci->getSciDebugger();
 		con->debugPrintf("SCI event occurred: ");
 		switch (curEvent.type) {
-		case SCI_EVENT_QUIT:
+		case kSciEventQuit:
 			con->debugPrintf("quit event\n");
 			break;
-		case SCI_EVENT_KEYBOARD:
+		case kSciEventKeyDown:
 			con->debugPrintf("keyboard event\n");
 			break;
-		case SCI_EVENT_MOUSE_RELEASE:
-		case SCI_EVENT_MOUSE_PRESS:
+		case kSciEventMousePress:
+		case kSciEventMouseRelease:
 			con->debugPrintf("mouse click event\n");
 			break;
 		default:
@@ -270,31 +268,31 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 }
 
 struct KeyDirMapping {
-	uint16 key;
+	SciKeyCode key;
 	uint16 direction;
 };
 
 const KeyDirMapping keyToDirMap[] = {
-	{ SCI_KEY_HOME,   8 }, { SCI_KEY_UP,     1 }, { SCI_KEY_PGUP,   2 },
-	{ SCI_KEY_LEFT,   7 }, { SCI_KEY_CENTER, 0 }, { SCI_KEY_RIGHT,  3 },
-	{ SCI_KEY_END,    6 }, { SCI_KEY_DOWN,   5 }, { SCI_KEY_PGDOWN, 4 },
+	{ kSciKeyHome, 8 }, { kSciKeyUp,     1 }, { kSciKeyPageUp,   2 },
+	{ kSciKeyLeft, 7 }, { kSciKeyCenter, 0 }, { kSciKeyRight,    3 },
+	{ kSciKeyEnd,  6 }, { kSciKeyDown,   5 }, { kSciKeyPageDown, 4 },
 };
 
 reg_t kMapKeyToDir(EngineState *s, int argc, reg_t *argv) {
 	reg_t obj = argv[0];
 	SegManager *segMan = s->_segMan;
 
-	if (readSelectorValue(segMan, obj, SELECTOR(type)) == SCI_EVENT_KEYBOARD) { // Keyboard
+	if (readSelectorValue(segMan, obj, SELECTOR(type)) == kSciEventKeyDown) {
 		uint16 message = readSelectorValue(segMan, obj, SELECTOR(message));
-		uint16 eventType = SCI_EVENT_DIRECTION;
-		// It seems with SCI1 Sierra started to add the SCI_EVENT_DIRECTION bit instead of setting it directly.
+		SciEventType eventType = kSciEventDirection;
+		// It seems with SCI1 Sierra started to add the kSciEventDirection bit instead of setting it directly.
 		// It was done inside the keyboard driver and is required for the PseudoMouse functionality and class
 		// to work (script 933).
 		if (g_sci->_features->detectPseudoMouseAbility() == kPseudoMouseAbilityTrue) {
-			eventType |= SCI_EVENT_KEYBOARD;
+			eventType |= kSciEventKeyDown;
 		}
 
-		for (int i = 0; i < 9; i++) {
+		for (int i = 0; i < ARRAYSIZE(keyToDirMap); i++) {
 			if (keyToDirMap[i].key == message) {
 				writeSelectorValue(segMan, obj, SELECTOR(type), eventType);
 				writeSelectorValue(segMan, obj, SELECTOR(message), keyToDirMap[i].direction);
diff --git a/engines/sci/engine/kgraphics.cpp b/engines/sci/engine/kgraphics.cpp
index f2111ca..7e63c35 100644
--- a/engines/sci/engine/kgraphics.cpp
+++ b/engines/sci/engine/kgraphics.cpp
@@ -1165,7 +1165,7 @@ reg_t kAnimate(EngineState *s, int argc, reg_t *argv) {
 	// keep ScummVM responsive. Fixes ScummVM "freezing" during the credits,
 	// bug #3101846
 	if (g_sci->getGameId() == GID_ECOQUEST && s->currentRoomNumber() == 680)
-		g_sci->getEventManager()->getSciEvent(SCI_EVENT_PEEK);
+		g_sci->getEventManager()->getSciEvent(kSciEventPeek);
 
 	return s->r_acc;
 }
diff --git a/engines/sci/event.cpp b/engines/sci/event.cpp
index b1be46a..2967b55 100644
--- a/engines/sci/event.cpp
+++ b/engines/sci/event.cpp
@@ -66,47 +66,46 @@ struct SciKeyConversion {
 };
 
 static const SciKeyConversion keyMappings[] = {
-	{ Common::KEYCODE_UP          , SCI_KEY_UP     , SCI_KEY_UP     },
-	{ Common::KEYCODE_DOWN        , SCI_KEY_DOWN   , SCI_KEY_DOWN   },
-	{ Common::KEYCODE_RIGHT       , SCI_KEY_RIGHT  , SCI_KEY_RIGHT  },
-	{ Common::KEYCODE_LEFT        , SCI_KEY_LEFT   , SCI_KEY_LEFT   },
-	{ Common::KEYCODE_INSERT      , SCI_KEY_INSERT , SCI_KEY_INSERT },
-	{ Common::KEYCODE_HOME        , SCI_KEY_HOME   , SCI_KEY_HOME   },
-	{ Common::KEYCODE_END         , SCI_KEY_END    , SCI_KEY_END    },
-	{ Common::KEYCODE_PAGEUP      , SCI_KEY_PGUP   , SCI_KEY_PGUP   },
-	{ Common::KEYCODE_PAGEDOWN    , SCI_KEY_PGDOWN , SCI_KEY_PGDOWN },
-	{ Common::KEYCODE_DELETE      , SCI_KEY_DELETE , SCI_KEY_DELETE },
-	// Keypad
-	{ Common::KEYCODE_KP0         , SCI_KEY_INSERT , '0'            },
-	{ Common::KEYCODE_KP1         , SCI_KEY_END    , '1'            },
-	{ Common::KEYCODE_KP2         , SCI_KEY_DOWN   , '2'            },
-	{ Common::KEYCODE_KP3         , SCI_KEY_PGDOWN , '3'            },
-	{ Common::KEYCODE_KP4         , SCI_KEY_LEFT   , '4'            },
-	{ Common::KEYCODE_KP5         , SCI_KEY_CENTER , '5'            },
-	{ Common::KEYCODE_KP6         , SCI_KEY_RIGHT  , '6'            },
-	{ Common::KEYCODE_KP7         , SCI_KEY_HOME   , '7'            },
-	{ Common::KEYCODE_KP8         , SCI_KEY_UP     , '8'            },
-	{ Common::KEYCODE_KP9         , SCI_KEY_PGUP   , '9'            },
-	{ Common::KEYCODE_KP_PERIOD   , SCI_KEY_DELETE , '.'            },
-	{ Common::KEYCODE_KP_ENTER    , SCI_KEY_ENTER  , SCI_KEY_ENTER  },
-	{ Common::KEYCODE_KP_PLUS     , '+'            , '+'            },
-	{ Common::KEYCODE_KP_MINUS    , '-'            , '-'            },
-	{ Common::KEYCODE_KP_MULTIPLY , '*'            , '*'            },
-	{ Common::KEYCODE_KP_DIVIDE   , '/'            , '/'            },
+	{ Common::KEYCODE_UP          , kSciKeyUp       , kSciKeyUp       },
+	{ Common::KEYCODE_DOWN        , kSciKeyDown     , kSciKeyDown     },
+	{ Common::KEYCODE_RIGHT       , kSciKeyRight    , kSciKeyRight    },
+	{ Common::KEYCODE_LEFT        , kSciKeyLeft     , kSciKeyLeft     },
+	{ Common::KEYCODE_INSERT      , kSciKeyInsert   , kSciKeyInsert   },
+	{ Common::KEYCODE_HOME        , kSciKeyHome     , kSciKeyHome     },
+	{ Common::KEYCODE_END         , kSciKeyEnd      , kSciKeyEnd      },
+	{ Common::KEYCODE_PAGEUP      , kSciKeyPageUp   , kSciKeyPageUp   },
+	{ Common::KEYCODE_PAGEDOWN    , kSciKeyPageDown , kSciKeyPageDown },
+	{ Common::KEYCODE_DELETE      , kSciKeyDelete   , kSciKeyDelete   },
+	{ Common::KEYCODE_KP0         , kSciKeyInsert   , '0'             },
+	{ Common::KEYCODE_KP1         , kSciKeyEnd      , '1'             },
+	{ Common::KEYCODE_KP2         , kSciKeyDown     , '2'             },
+	{ Common::KEYCODE_KP3         , kSciKeyPageDown , '3'             },
+	{ Common::KEYCODE_KP4         , kSciKeyLeft     , '4'             },
+	{ Common::KEYCODE_KP5         , kSciKeyCenter   , '5'             },
+	{ Common::KEYCODE_KP6         , kSciKeyRight    , '6'             },
+	{ Common::KEYCODE_KP7         , kSciKeyHome     , '7'             },
+	{ Common::KEYCODE_KP8         , kSciKeyUp       , '8'             },
+	{ Common::KEYCODE_KP9         , kSciKeyPageUp   , '9'             },
+	{ Common::KEYCODE_KP_PERIOD   , kSciKeyDelete   , '.'             },
+	{ Common::KEYCODE_KP_ENTER    , kSciKeyEnter    , kSciKeyEnter    },
+	{ Common::KEYCODE_KP_PLUS     , '+'             , '+'             },
+	{ Common::KEYCODE_KP_MINUS    , '-'             , '-'             },
+	{ Common::KEYCODE_KP_MULTIPLY , '*'             , '*'             },
+	{ Common::KEYCODE_KP_DIVIDE   , '/'             , '/'             }
 };
 
 struct MouseEventConversion {
 	Common::EventType commonType;
-	short sciType;
+	SciEventType sciType;
 };
 
 static const MouseEventConversion mouseEventMappings[] = {
-	{ Common::EVENT_LBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS },
-	{ Common::EVENT_RBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS },
-	{ Common::EVENT_MBUTTONDOWN,   SCI_EVENT_MOUSE_PRESS },
-	{   Common::EVENT_LBUTTONUP, SCI_EVENT_MOUSE_RELEASE },
-	{   Common::EVENT_RBUTTONUP, SCI_EVENT_MOUSE_RELEASE },
-	{   Common::EVENT_MBUTTONUP, SCI_EVENT_MOUSE_RELEASE }
+	{ Common::EVENT_LBUTTONDOWN , kSciEventMousePress   },
+	{ Common::EVENT_RBUTTONDOWN , kSciEventMousePress   },
+	{ Common::EVENT_MBUTTONDOWN , kSciEventMousePress   },
+	{ Common::EVENT_LBUTTONUP   , kSciEventMouseRelease },
+	{ Common::EVENT_RBUTTONUP   , kSciEventMouseRelease },
+	{ Common::EVENT_MBUTTONUP   , kSciEventMouseRelease }
 };
 
 EventManager::EventManager(bool fontIsExtended) :
@@ -119,12 +118,13 @@ EventManager::EventManager(bool fontIsExtended) :
 EventManager::~EventManager() {
 }
 
-static int altify(int ch) {
-	// Calculates a PC keyboard scancode from a character */
-	int row;
-	int c = toupper((char)ch);
+/**
+ * Calculates the IBM keyboard alt-key scancode of a printable character.
+ */
+static int altify(char ch) {
+	const char c = toupper(ch);
 
-	for (row = 0; row < ARRAYSIZE(scancodeAltifyRows); row++) {
+	for (int row = 0; row < ARRAYSIZE(scancodeAltifyRows); ++row) {
 		const char *keys = scancodeAltifyRows[row].keys;
 		int offset = scancodeAltifyRows[row].offset;
 
@@ -132,8 +132,8 @@ static int altify(int ch) {
 			if (*keys == c)
 				return offset << 8;
 
-			offset++;
-			keys++;
+			++offset;
+			++keys;
 		}
 	}
 
@@ -142,31 +142,24 @@ static int altify(int ch) {
 
 SciEvent EventManager::getScummVMEvent() {
 #ifdef ENABLE_SCI32
-	SciEvent input = { SCI_EVENT_NONE, 0, 0, Common::Point(), Common::Point(), -1 };
-	SciEvent noEvent = { SCI_EVENT_NONE, 0, 0, Common::Point(), Common::Point(), -1 };
+	SciEvent input   = { kSciEventNone, kSciKeyModNone, 0, Common::Point(), Common::Point(), -1 };
+	SciEvent noEvent = { kSciEventNone, kSciKeyModNone, 0, Common::Point(), Common::Point(), -1 };
 #else
-	SciEvent input = { SCI_EVENT_NONE, 0, 0, Common::Point() };
-	SciEvent noEvent = { SCI_EVENT_NONE, 0, 0, Common::Point() };
+	SciEvent input   = { kSciEventNone, kSciKeyModNone, 0, Common::Point() };
+	SciEvent noEvent = { kSciEventNone, kSciKeyModNone, 0, Common::Point() };
 #endif
 
 	Common::EventManager *em = g_system->getEventManager();
 	Common::Event ev;
 
-	bool found = em->pollEvent(ev);
-
-	// Don't generate events for mouse movement
-	while (found && ev.type == Common::EVENT_MOUSEMOVE)
+	// SCI does not generate separate events for mouse movement (it puts the
+	// current mouse position on every event, including non-mouse events), so
+	// skip past all mousemove events in the event queue
+	bool found;
+	do {
 		found = em->pollEvent(ev);
+	} while (found && ev.type == Common::EVENT_MOUSEMOVE);
 
-	// Save the mouse position
-	//
-	// We call getMousePos of the event manager here, since we also want to
-	// store the mouse position in case of keyboard events, which do not feature
-	// any mouse position information itself.
-	// This should be safe, since the mouse position in the event manager should
-	// only be updated when a mouse related event has been taken from the queue
-	// via pollEvent.
-	// We also adjust the position based on the scaling of the screen.
 	Common::Point mousePos = em->getMousePos();
 
 #if ENABLE_SCI32
@@ -196,15 +189,17 @@ SciEvent EventManager::getScummVMEvent() {
 
 	if (!found || ev.type == Common::EVENT_MOUSEMOVE) {
 		int modifiers = em->getModifierState();
-		noEvent.modifiers =
-			((modifiers & Common::KBD_ALT) ? SCI_KEYMOD_ALT : 0) |
-			((modifiers & Common::KBD_CTRL) ? SCI_KEYMOD_CTRL : 0) |
-			((modifiers & Common::KBD_SHIFT) ? SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT : 0);
+		if (modifiers & Common::KBD_ALT)
+			noEvent.modifiers |= kSciKeyModAlt;
+		if (modifiers & Common::KBD_CTRL)
+			noEvent.modifiers |= kSciKeyModCtrl;
+		if (modifiers & Common::KBD_SHIFT)
+			noEvent.modifiers |= kSciKeyModShift;
 
 		return noEvent;
 	}
 	if (ev.type == Common::EVENT_QUIT || ev.type == Common::EVENT_RTL) {
-		input.type = SCI_EVENT_QUIT;
+		input.type = kSciEventQuit;
 		return input;
 	}
 
@@ -222,14 +217,16 @@ SciEvent EventManager::getScummVMEvent() {
 		break;
 	}
 
-	input.modifiers =
-		((scummVMKeyFlags & Common::KBD_ALT) ? SCI_KEYMOD_ALT : 0) |
-		((scummVMKeyFlags & Common::KBD_CTRL) ? SCI_KEYMOD_CTRL : 0) |
-		((scummVMKeyFlags & Common::KBD_SHIFT) ? SCI_KEYMOD_LSHIFT | SCI_KEYMOD_RSHIFT : 0);
-		// Caps lock and Scroll lock have been removed, cause we already handle upper
-		// case keys and Scroll lock doesn't seem to be used anywhere
-		//((ourModifiers & Common::KBD_CAPS) ? SCI_KEYMOD_CAPSLOCK : 0) |
-		//((ourModifiers & Common::KBD_SCRL) ? SCI_KEYMOD_SCRLOCK : 0) |
+	// Caps lock and scroll lock are not handled here because we already
+	// handle upper case keys elsewhere, and scroll lock doesn't seem to
+	// ever be used
+	input.modifiers = kSciKeyModNone;
+	if (scummVMKeyFlags & Common::KBD_ALT)
+		input.modifiers |= kSciKeyModAlt;
+	if (scummVMKeyFlags & Common::KBD_CTRL)
+		input.modifiers |= kSciKeyModCtrl;
+	if (scummVMKeyFlags & Common::KBD_SHIFT)
+		input.modifiers |= kSciKeyModShift;
 
 	// Handle mouse events
 	for (int i = 0; i < ARRAYSIZE(mouseEventMappings); i++) {
@@ -243,11 +240,11 @@ SciEvent EventManager::getScummVMEvent() {
 			switch (ev.type) {
 			case Common::EVENT_RBUTTONDOWN: // right button
 			case Common::EVENT_RBUTTONUP:
-				input.modifiers |= (SCI_KEYMOD_RSHIFT | SCI_KEYMOD_LSHIFT); // this value was hardcoded in the mouse interrupt handler
+				input.modifiers |= kSciKeyModShift; // this value was hardcoded in the mouse interrupt handler
 				break;
 			case Common::EVENT_MBUTTONDOWN: // middle button
 			case Common::EVENT_MBUTTONUP:
-				input.modifiers |= SCI_KEYMOD_CTRL; // this value was hardcoded in the mouse interrupt handler
+				input.modifiers |= kSciKeyModCtrl; // this value was hardcoded in the mouse interrupt handler
 				break;
 			default:
 				break;
@@ -261,21 +258,18 @@ SciEvent EventManager::getScummVMEvent() {
 		return noEvent;
 
 	// Check for Control-Shift-D (debug console)
-	if (ev.kbd.hasFlags(Common::KBD_CTRL | Common::KBD_SHIFT) && ev.kbd.keycode == Common::KEYCODE_d) {
+	if (ev.type == Common::EVENT_KEYDOWN && ev.kbd.hasFlags(Common::KBD_CTRL | Common::KBD_SHIFT) && ev.kbd.keycode == Common::KEYCODE_d) {
 		// Open debug console
 		Console *con = g_sci->getSciDebugger();
 		con->attach();
 		return noEvent;
 	}
 
-	// Process keyboard events
-
-	bool numlockOn = (ev.kbd.flags & Common::KBD_NUM);
 
-	Common::KeyCode scummVMKeycode = ev.kbd.keycode;
+	const Common::KeyCode scummVMKeycode = ev.kbd.keycode;
 
 	input.character = ev.kbd.ascii;
-	input.type = SCI_EVENT_KEYBOARD;
+	input.type = kSciEventKeyDown;
 
 	if (scummVMKeycode >= Common::KEYCODE_KP0 && scummVMKeycode <= Common::KEYCODE_KP9) {
 		if (!(scummVMKeyFlags & Common::KBD_NUM)) {
@@ -286,60 +280,59 @@ SciEvent EventManager::getScummVMEvent() {
 		}
 	}
 
-	if ((input.character) && (input.character <= 0xFF)) {
-		// Directly accept most common keys without conversion
-		if ((input.character >= 0x80) && (input.character <= 0xFF)) {
-			// If there is no extended font, we will just clear the
-			// current event.
-			// Sierra SCI actually accepted those characters, but
-			// didn't display them inside text edit controls because
-			// the characters were missing inside the font(s).
-			// We filter them out for non-multilingual games because
-			// of that.
-			if (!_fontIsExtended)
+	if (input.character && input.character <= 0xFF) {
+		// Extended characters need to be converted to the old to DOS CP850/437
+		// character sets for multilingual games
+		if (input.character >= 0x80 && input.character <= 0xFF) {
+			// SSCI accepted all input scan codes, regardless of locale, and
+			// just didn't display any characters that were missing from fonts
+			// used by text input controls. We intentionally filter them out
+			// entirely for non-multilingual games here instead, so we can have
+			// better error detection for bugs in the text controls
+			if (!_fontIsExtended) {
 				return noEvent;
-			// Convert 8859-1 characters to DOS (cp850/437) for
-			// multilingual SCI01 games
+			}
+
 			input.character = codePageMap88591ToDOS[input.character & 0x7f];
 		}
+
 		if (scummVMKeycode == Common::KEYCODE_TAB) {
-			input.character = SCI_KEY_TAB;
+			input.character = kSciKeyTab;
 			if (scummVMKeyFlags & Common::KBD_SHIFT)
-				input.character = SCI_KEY_SHIFT_TAB;
+				input.character = kSciKeyShiftTab;
 		}
+
 		if (scummVMKeycode == Common::KEYCODE_DELETE)
-			input.character = SCI_KEY_DELETE;
-	} else if ((scummVMKeycode >= Common::KEYCODE_F1) && scummVMKeycode <= Common::KEYCODE_F10) {
-		// SCI_K_F1 == 59 << 8
-		// SCI_K_SHIFT_F1 == 84 << 8
-		if (!(scummVMKeyFlags & Common::KBD_SHIFT))
-			input.character = SCI_KEY_F1 + ((scummVMKeycode - Common::KEYCODE_F1)<<8);
+			input.character = kSciKeyDelete;
+	} else if (scummVMKeycode >= Common::KEYCODE_F1 && scummVMKeycode <= Common::KEYCODE_F10) {
+		if (scummVMKeyFlags & Common::KBD_SHIFT)
+			input.character = kSciKeyShiftF1 + ((scummVMKeycode - Common::KEYCODE_F1) << 8);
 		else
-			input.character = SCI_KEY_SHIFT_F1 + ((scummVMKeycode - Common::KEYCODE_F1)<<8);
+			input.character = kSciKeyF1 + ((scummVMKeycode - Common::KEYCODE_F1) << 8);
 	} else {
-		// Special keys that need conversion
+		// Arrow keys, numpad keys, etc.
 		for (int i = 0; i < ARRAYSIZE(keyMappings); i++) {
 			if (keyMappings[i].scummVMKey == scummVMKeycode) {
+				const bool numlockOn = (ev.kbd.flags & Common::KBD_NUM);
 				input.character = numlockOn ? keyMappings[i].sciKeyNumlockOn : keyMappings[i].sciKeyNumlockOff;
 				break;
 			}
 		}
 	}
 
-	// When Ctrl AND Alt are pressed together with a regular key, Linux will give us control-key, Windows will give
-	//  us the actual key. My opinion is that windows is right, because under DOS the keys worked the same, anyway
-	//  we support the other case as well
+	// TODO: Leaky abstractions from SDL should not be handled in game engines!
+	// When Ctrl and Alt are pressed together with a printable key, SDL1 on
+	// Linux will give us a control character instead of the printable
+	// character we need to convert to an alt scancode
 	if ((scummVMKeyFlags & Common::KBD_ALT) && input.character > 0 && input.character < 27)
 		input.character += 96; // 0x01 -> 'a'
 
-	// Scancodify if appropriate
-	if (scummVMKeyFlags & Common::KBD_ALT)
-		input.character = altify(input.character);
-
-	// In SSCI, Ctrl+<key> generates ASCII control characters, but the backends
-	// usually give us a latin character + Ctrl flag, so convert this combo back
-	// into what is expected by game scripts
-	if ((scummVMKeyFlags & Common::KBD_NON_STICKY) == Common::KBD_CTRL && input.character >= 'a' && input.character <= 'z') {
+	if (scummVMKeyFlags & Common::KBD_ALT) {
+		input.character = altify(input.character & 0xFF);
+	} else if ((scummVMKeyFlags & Common::KBD_NON_STICKY) == Common::KBD_CTRL && input.character >= 'a' && input.character <= 'z') {
+		// In SSCI, Ctrl+<key> generates ASCII control characters, but the
+		// backends usually give us a printable character + Ctrl flag, so
+		// convert this combo back into what is expected by game scripts
 		input.character -= 96;
 	}
 
@@ -368,11 +361,11 @@ void EventManager::updateScreen() {
 	}
 }
 
-SciEvent EventManager::getSciEvent(uint32 mask) {
+SciEvent EventManager::getSciEvent(SciEventType mask) {
 #ifdef ENABLE_SCI32
-	SciEvent event = { SCI_EVENT_NONE, 0, 0, Common::Point(), Common::Point(), -1 };
+	SciEvent event = { kSciEventNone, kSciKeyModNone, 0, Common::Point(), Common::Point(), -1 };
 #else
-	SciEvent event = { SCI_EVENT_NONE, 0, 0, Common::Point() };
+	SciEvent event = { kSciEventNone, kSciKeyModNone, 0, Common::Point() };
 #endif
 
 	if (getSciVersion() < SCI_VERSION_2) {
@@ -382,9 +375,9 @@ SciEvent EventManager::getSciEvent(uint32 mask) {
 	// Get all queued events from graphics driver
 	do {
 		event = getScummVMEvent();
-		if (event.type != SCI_EVENT_NONE)
+		if (event.type != kSciEventNone)
 			_events.push_back(event);
-	} while (event.type != SCI_EVENT_NONE);
+	} while (event.type != kSciEventNone);
 
 	// Search for matching event in queue
 	Common::List<SciEvent>::iterator iter = _events.begin();
@@ -396,12 +389,12 @@ SciEvent EventManager::getSciEvent(uint32 mask) {
 		event = *iter;
 
 		// If not peeking at the queue, remove the event
-		if (!(mask & SCI_EVENT_PEEK))
+		if (!(mask & kSciEventPeek))
 			_events.erase(iter);
 	} else {
-		// No event found: we must return a SCI_EVT_NONE event.
+		// No event found: we must return a kSciEventNone event.
 
-		// Because event.type is SCI_EVT_NONE already here,
+		// Because event.type is kSciEventNone already here,
 		// there is no need to change it.
 	}
 
@@ -434,7 +427,7 @@ void EventManager::checkHotRectangles(const Common::Point &mousePosition) {
 			_activeRectIndex = i;
 			if (i != lastActiveRectIndex) {
 				SciEvent hotRectEvent;
-				hotRectEvent.type = SCI_EVENT_HOT_RECTANGLE;
+				hotRectEvent.type = kSciEventHotRectangle;
 				hotRectEvent.hotRectangleIndex = i;
 				_events.push_front(hotRectEvent);
 				break;
@@ -447,7 +440,7 @@ void EventManager::checkHotRectangles(const Common::Point &mousePosition) {
 	if (lastActiveRectIndex != _activeRectIndex && lastActiveRectIndex != -1) {
 		_activeRectIndex = -1;
 		SciEvent hotRectEvent;
-		hotRectEvent.type = SCI_EVENT_HOT_RECTANGLE;
+		hotRectEvent.type = kSciEventHotRectangle;
 		hotRectEvent.hotRectangleIndex = -1;
 		_events.push_front(hotRectEvent);
 	}
diff --git a/engines/sci/event.h b/engines/sci/event.h
index 614a5a6..9d6e51f 100644
--- a/engines/sci/event.h
+++ b/engines/sci/event.h
@@ -28,9 +28,98 @@
 
 namespace Sci {
 
+enum SciEventType {
+	kSciEventNone         = 0,
+	kSciEventMousePress   = 1,
+	kSciEventMouseRelease = 1 << 1,
+	kSciEventMouse        = kSciEventMousePress | kSciEventMouseRelease,
+	kSciEventKeyDown      = 1 << 2,
+	kSciEventKeyUp        = 1 << 3,
+	kSciEventKey          = kSciEventKeyDown | kSciEventKeyUp,
+	kSciEventDirection    = 1 << 6,
+	kSciEventSaid         = 1 << 7,
+#ifdef ENABLE_SCI32
+	kSciEventHotRectangle = 1 << 10,
+#endif
+	kSciEventQuit         = 1 << 11,
+	kSciEventPeek         = 1 << 15,
+
+	kSciEventAny          = ~kSciEventPeek
+};
+
+inline SciEventType operator|(const SciEventType a, const SciEventType b) {
+	return static_cast<SciEventType>((int)a | (int)b);
+}
+
+inline SciEventType &operator|=(SciEventType &a, const SciEventType b) {
+	return a = static_cast<SciEventType>((int)a | (int)b);
+}
+
+enum SciKeyCode {
+	kSciKeyEtx       = 3,
+	kSciKeyBackspace = 8,
+	kSciKeyTab       = '\t',
+	kSciKeyEnter     = 13,
+	kSciKeyEsc       = 27,
+	kSciKeyShiftTab  = 15 << 8,
+
+	kSciKeyHome      = 71 << 8, // numpad 7
+	kSciKeyUp        = 72 << 8, // numpad 8
+	kSciKeyPageUp    = 73 << 8, // numpad 9
+	kSciKeyLeft      = 75 << 8, // numpad 4
+	kSciKeyCenter    = 76 << 8, // numpad 5
+	kSciKeyRight     = 77 << 8, // numpad 6
+	kSciKeyEnd       = 79 << 8, // numpad 1
+	kSciKeyDown      = 80 << 8, // numpad 2
+	kSciKeyPageDown  = 81 << 8, // numpad 3
+	kSciKeyInsert    = 82 << 8, // numpad 0
+	kSciKeyDelete    = 83 << 8, // numpad .
+
+	kSciKeyF1        = 59 << 8,
+	kSciKeyF2        = 60 << 8,
+	kSciKeyF3        = 61 << 8,
+	kSciKeyF4        = 62 << 8,
+	kSciKeyF5        = 63 << 8,
+	kSciKeyF6        = 64 << 8,
+	kSciKeyF7        = 65 << 8,
+	kSciKeyF8        = 66 << 8,
+	kSciKeyF9        = 67 << 8,
+	kSciKeyF10       = 68 << 8,
+
+	kSciKeyShiftF1   = 84 << 8,
+	kSciKeyShiftF2   = 85 << 8,
+	kSciKeyShiftF3   = 86 << 8,
+	kSciKeyShiftF4   = 87 << 8,
+	kSciKeyShiftF5   = 88 << 8,
+	kSciKeyShiftF6   = 89 << 8,
+	kSciKeyShiftF7   = 90 << 8,
+	kSciKeyShiftF8   = 91 << 8,
+	kSciKeyShiftF9   = 92 << 8,
+	kSciKeyShiftF10  = 93 << 8
+};
+
+enum SciKeyModifiers {
+	kSciKeyModNone      = 0,
+	kSciKeyModRShift    = 1,
+	kSciKeyModLShift    = 1 << 1,
+	kSciKeyModShift     = kSciKeyModRShift | kSciKeyModLShift,
+	kSciKeyModCtrl      = 1 << 2,
+	kSciKeyModAlt       = 1 << 3,
+	kSciKeyModScrLock   = 1 << 4,
+	kSciKeyModNumLock   = 1 << 5,
+	kSciKeyModCapsLock  = 1 << 6,
+	kSciKeyModInsert    = 1 << 7,
+	kSciKeyModNonSticky = kSciKeyModRShift | kSciKeyModLShift | kSciKeyModCtrl | kSciKeyModAlt,
+	kSciKeyModAll       = ~kSciKeyModNone
+};
+
+inline SciKeyModifiers &operator|=(SciKeyModifiers &a, SciKeyModifiers b) {
+	return a = static_cast<SciKeyModifiers>((int)a | (int)b);
+}
+
 struct SciEvent {
-	uint16 type;
-	uint16 modifiers;
+	SciEventType type;
+	SciKeyModifiers modifiers;
 	/**
 	 * For keyboard events: the actual character of the key that was pressed
 	 * For 'Alt', characters are interpreted by their
@@ -39,105 +128,33 @@ struct SciEvent {
 	uint16 character;
 
 	/**
-	 * The mouse position at the time the event was created,
-	 * in display coordinates.
+	 * The mouse position at the time the event was created, in script
+	 * coordinates (SCI16) or display coordinates (SCI32).
 	 */
 	Common::Point mousePos;
 
 #ifdef ENABLE_SCI32
 	/**
-	 * The mouse position at the time the event was created,
-	 * in script coordinates.
+	 * The mouse position at the time the event was created, in script
+	 * coordinates. Used only by SCI32.
 	 */
 	Common::Point mousePosSci;
 
+	/**
+	 * The currently active hot rectangle, or -1 if no hot rectangle is active.
+	 * Used only by the chase scene in Phantasmagoria 1.
+	 */
 	int16 hotRectangleIndex;
 #endif
 };
 
-/*Values for type*/
-#define SCI_EVENT_NONE            0
-#define SCI_EVENT_MOUSE_PRESS     (1 << 0)
-#define SCI_EVENT_MOUSE_RELEASE   (1 << 1)
-#define SCI_EVENT_KEYBOARD        (1 << 2)
-#define SCI_EVENT_DIRECTION       (1 << 6)
-#define SCI_EVENT_SAID            (1 << 7)
-#ifdef ENABLE_SCI32
-#define SCI_EVENT_HOT_RECTANGLE   (1 << 10)
-#endif
-/*Fake values for other events*/
-#define SCI_EVENT_QUIT            (1 << 11)
-#define SCI_EVENT_PEEK            (1 << 15)
-#define SCI_EVENT_ANY             0x7fff
-
-/* Keycodes of special keys: */
-#ifdef ENABLE_SCI32
-#define SCI_KEY_ETX           3
-#endif
-#define SCI_KEY_ESC          27
-#define SCI_KEY_BACKSPACE     8
-#define SCI_KEY_ENTER        13
-#define SCI_KEY_TAB        '\t'
-#define SCI_KEY_SHIFT_TAB  (0xf << 8)
-
-#define SCI_KEY_HOME   (71 << 8)	// 7
-#define SCI_KEY_UP     (72 << 8)	// 8
-#define SCI_KEY_PGUP   (73 << 8)	// 9
-//
-#define SCI_KEY_LEFT   (75 << 8)	// 4
-#define SCI_KEY_CENTER (76 << 8)	// 5
-#define SCI_KEY_RIGHT  (77 << 8)	// 6
-//
-#define SCI_KEY_END    (79 << 8)	// 1
-#define SCI_KEY_DOWN   (80 << 8)	// 2
-#define SCI_KEY_PGDOWN (81 << 8)	// 3
-//
-#define SCI_KEY_INSERT (82 << 8)	// 0
-#define SCI_KEY_DELETE (83 << 8)	// .
-
-#define SCI_KEY_F1   (59 << 8)
-#define SCI_KEY_F2   (60 << 8)
-#define SCI_KEY_F3   (61 << 8)
-#define SCI_KEY_F4   (62 << 8)
-#define SCI_KEY_F5   (63 << 8)
-#define SCI_KEY_F6   (64 << 8)
-#define SCI_KEY_F7   (65 << 8)
-#define SCI_KEY_F8   (66 << 8)
-#define SCI_KEY_F9   (67 << 8)
-#define SCI_KEY_F10  (68 << 8)
-
-#define SCI_KEY_SHIFT_F1   (84 << 8)
-#define SCI_KEY_SHIFT_F2   (85 << 8)
-#define SCI_KEY_SHIFT_F3   (86 << 8)
-#define SCI_KEY_SHIFT_F4   (87 << 8)
-#define SCI_KEY_SHIFT_F5   (88 << 8)
-#define SCI_KEY_SHIFT_F6   (89 << 8)
-#define SCI_KEY_SHIFT_F7   (90 << 8)
-#define SCI_KEY_SHIFT_F8   (91 << 8)
-#define SCI_KEY_SHIFT_F9   (92 << 8)
-#define SCI_KEY_SHIFT_F10  (93 << 8)
-
-/*Values for buckybits */
-#define SCI_KEYMOD_RSHIFT    (1 << 0)
-#define SCI_KEYMOD_LSHIFT    (1 << 1)
-#define SCI_KEYMOD_CTRL      (1 << 2)
-#define SCI_KEYMOD_ALT       (1 << 3)
-#define SCI_KEYMOD_SCRLOCK   (1 << 4)
-#define SCI_KEYMOD_NUMLOCK   (1 << 5)
-#define SCI_KEYMOD_CAPSLOCK  (1 << 6)
-#define SCI_KEYMOD_INSERT    (1 << 7)
-
-#define SCI_KEYMOD_NON_STICKY      (SCI_KEYMOD_RSHIFT | SCI_KEYMOD_LSHIFT | SCI_KEYMOD_CTRL | SCI_KEYMOD_ALT)
-#define SCI_KEYMOD_NO_FOOLOCK      (~(SCI_KEYMOD_SCRLOCK | SCI_KEYMOD_NUMLOCK | SCI_KEYMOD_CAPSLOCK | SCI_KEYMOD_INSERT))
-#define SCI_KEYMOD_ALL             0xFF
-
 class EventManager {
 public:
 	EventManager(bool fontIsExtended);
 	~EventManager();
 
 	void updateScreen();
-	SciEvent getSciEvent(uint32 mask);
+	SciEvent getSciEvent(SciEventType mask);
 	void flushEvents();
 
 private:
diff --git a/engines/sci/graphics/controls16.cpp b/engines/sci/graphics/controls16.cpp
index 479044a..138f49e 100644
--- a/engines/sci/graphics/controls16.cpp
+++ b/engines/sci/graphics/controls16.cpp
@@ -158,50 +158,50 @@ void GfxControls16::kernelTexteditChange(reg_t controlObject, reg_t eventObject)
 		eventType = readSelectorValue(_segMan, eventObject, SELECTOR(type));
 
 		switch (eventType) {
-		case SCI_EVENT_MOUSE_PRESS:
+		case kSciEventMousePress:
 			// TODO: Implement mouse support for cursor change
 			break;
-		case SCI_EVENT_KEYBOARD:
+		case kSciEventKeyDown:
 			eventKey = readSelectorValue(_segMan, eventObject, SELECTOR(message));
 			modifiers = readSelectorValue(_segMan, eventObject, SELECTOR(modifiers));
 			switch (eventKey) {
-			case SCI_KEY_BACKSPACE:
+			case kSciKeyBackspace:
 				if (cursorPos > 0) {
 					cursorPos--; text.deleteChar(cursorPos);
 					textChanged = true;
 				}
 				break;
-			case SCI_KEY_DELETE:
+			case kSciKeyDelete:
 				if (cursorPos < textSize) {
 					text.deleteChar(cursorPos);
 					textChanged = true;
 				}
 				break;
-			case SCI_KEY_HOME: // HOME
+			case kSciKeyHome:
 				cursorPos = 0; textChanged = true;
 				break;
-			case SCI_KEY_END: // END
+			case kSciKeyEnd:
 				cursorPos = textSize; textChanged = true;
 				break;
-			case SCI_KEY_LEFT: // LEFT
+			case kSciKeyLeft:
 				if (cursorPos > 0) {
 					cursorPos--; textChanged = true;
 				}
 				break;
-			case SCI_KEY_RIGHT: // RIGHT
+			case kSciKeyRight:
 				if (cursorPos + 1 <= textSize) {
 					cursorPos++; textChanged = true;
 				}
 				break;
-			case 3:	// returned in SCI1 late and newer when Control - C is pressed
-				if (modifiers & SCI_KEYMOD_CTRL) {
+			case kSciKeyEtx:
+				if (modifiers & kSciKeyModCtrl) {
 					// Control-C erases the whole line
 					cursorPos = 0; text.clear();
 					textChanged = true;
 				}
 				break;
 			default:
-				if ((modifiers & SCI_KEYMOD_CTRL) && eventKey == 99) {
+				if ((modifiers & kSciKeyModCtrl) && eventKey == 99) {
 					// Control-C in earlier SCI games (SCI0 - SCI1 middle)
 					// Control-C erases the whole line
 					cursorPos = 0; text.clear();
diff --git a/engines/sci/graphics/controls32.cpp b/engines/sci/graphics/controls32.cpp
index 77dfdc2..52a9dfa 100644
--- a/engines/sci/graphics/controls32.cpp
+++ b/engines/sci/graphics/controls32.cpp
@@ -154,22 +154,22 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 		// the last event just gets posted back to the event manager for
 		// reprocessing, but instead, we only remove the event from the
 		// queue *after* we have determined it is not a defocusing event
-		const SciEvent event = eventManager->getSciEvent(SCI_EVENT_ANY | SCI_EVENT_PEEK);
+		const SciEvent event = eventManager->getSciEvent(kSciEventAny | kSciEventPeek);
 
 		bool focused = true;
 		// Original engine did not have a QUIT event but we have to handle it
-		if (event.type == SCI_EVENT_QUIT) {
+		if (event.type == kSciEventQuit) {
 			focused = false;
-		} else if (event.type == SCI_EVENT_MOUSE_PRESS && !editorPlaneRect.contains(event.mousePosSci)) {
+		} else if (event.type == kSciEventMousePress && !editorPlaneRect.contains(event.mousePosSci)) {
 			focused = false;
-		} else if (event.type == SCI_EVENT_KEYBOARD) {
+		} else if (event.type == kSciEventKeyDown) {
 			switch (event.character) {
-			case SCI_KEY_ESC:
-			case SCI_KEY_UP:
-			case SCI_KEY_DOWN:
-			case SCI_KEY_TAB:
-			case SCI_KEY_SHIFT_TAB:
-			case SCI_KEY_ENTER:
+			case kSciKeyEsc:
+			case kSciKeyUp:
+			case kSciKeyDown:
+			case kSciKeyTab:
+			case kSciKeyShiftTab:
+			case kSciKeyEnter:
 				focused = false;
 				break;
 			}
@@ -181,8 +181,8 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 
 		// Consume the event now that we know it is not one of the
 		// defocusing events above
-		if (event.type != SCI_EVENT_NONE)
-			eventManager->getSciEvent(SCI_EVENT_ANY);
+		if (event.type != kSciEventNone)
+			eventManager->getSciEvent(kSciEventAny);
 
 		// NOTE: In the original engine, the font and bitmap were
 		// reset here on each iteration through the loop, but it
@@ -195,33 +195,33 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 		bool shouldDeleteChar = false;
 		bool shouldRedrawText = false;
 		uint16 lastCursorPosition = editor.cursorCharPosition;
- 		if (event.type == SCI_EVENT_KEYBOARD) {
+		if (event.type == kSciEventKeyDown) {
 			switch (event.character) {
-			case SCI_KEY_LEFT:
+			case kSciKeyLeft:
 				clearTextOnInput = false;
 				if (editor.cursorCharPosition > 0) {
 					--editor.cursorCharPosition;
 				}
 				break;
 
-			case SCI_KEY_RIGHT:
+			case kSciKeyRight:
 				clearTextOnInput = false;
 				if (editor.cursorCharPosition < editor.text.size()) {
 					++editor.cursorCharPosition;
 				}
 				break;
 
-			case SCI_KEY_HOME:
+			case kSciKeyHome:
 				clearTextOnInput = false;
 				editor.cursorCharPosition = 0;
 				break;
 
-			case SCI_KEY_END:
+			case kSciKeyEnd:
 				clearTextOnInput = false;
 				editor.cursorCharPosition = editor.text.size();
 				break;
 
-			case SCI_KEY_INSERT:
+			case kSciKeyInsert:
 				clearTextOnInput = false;
 				// Redrawing also changes the cursor rect to
 				// reflect the new insertion mode
@@ -229,14 +229,14 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 				_overwriteMode = !_overwriteMode;
 				break;
 
-			case SCI_KEY_DELETE:
+			case kSciKeyDelete:
 				clearTextOnInput = false;
 				if (editor.cursorCharPosition < editor.text.size()) {
 					shouldDeleteChar = true;
 				}
 				break;
 
-			case SCI_KEY_BACKSPACE:
+			case kSciKeyBackspace:
 				clearTextOnInput = false;
 				shouldDeleteChar = true;
 				if (editor.cursorCharPosition > 0) {
@@ -244,7 +244,7 @@ reg_t GfxControls32::kernelEditText(const reg_t controlObject) {
 				}
 				break;
 
-			case SCI_KEY_ETX:
+			case kSciKeyEtx:
 				editor.text.clear();
 				editor.cursorCharPosition = 0;
 				shouldRedrawText = true;
diff --git a/engines/sci/graphics/cursor.cpp b/engines/sci/graphics/cursor.cpp
index 89bb2a2..6b18dce 100644
--- a/engines/sci/graphics/cursor.cpp
+++ b/engines/sci/graphics/cursor.cpp
@@ -479,7 +479,7 @@ void GfxCursor::kernelMoveCursor(Common::Point pos) {
 
 	// Trigger event reading to make sure the mouse coordinates will
 	// actually have changed the next time we read them.
-	_event->getSciEvent(SCI_EVENT_PEEK);
+	_event->getSciEvent(kSciEventPeek);
 }
 
 void GfxCursor::kernelSetMacCursor(GuiResourceId viewNum, int loopNum, int celNum) {
diff --git a/engines/sci/graphics/frameout.h b/engines/sci/graphics/frameout.h
index cd48c78..a7b529f 100644
--- a/engines/sci/graphics/frameout.h
+++ b/engines/sci/graphics/frameout.h
@@ -394,7 +394,7 @@ private:
 		// position is rendered instead of whatever position it was at the last
 		// time kGetEvent was called. Without this, the mouse appears stuck
 		// during loops that do not make calls to kGetEvent, like transitions.
-		g_sci->getEventManager()->getSciEvent(SCI_EVENT_PEEK);
+		g_sci->getEventManager()->getSciEvent(kSciEventPeek);
 	}
 
 	/**
diff --git a/engines/sci/graphics/maciconbar.cpp b/engines/sci/graphics/maciconbar.cpp
index 3a62760..c5c2fb9 100644
--- a/engines/sci/graphics/maciconbar.cpp
+++ b/engines/sci/graphics/maciconbar.cpp
@@ -245,10 +245,10 @@ bool GfxMacIconBar::pointOnIcon(uint32 iconIndex, Common::Point point) {
 reg_t GfxMacIconBar::handleEvents() {
 	// Peek event queue for a mouse button press
 	EventManager *evtMgr = g_sci->getEventManager();
-	SciEvent evt = evtMgr->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
+	SciEvent evt = evtMgr->getSciEvent(kSciEventMousePress | kSciEventPeek);
 
 	// No mouse press found
-	if (evt.type == SCI_EVENT_NONE)
+	if (evt.type == kSciEventNone)
 		return NULL_REG;
 
 	// If the mouse is not over the icon bar, return
@@ -256,7 +256,7 @@ reg_t GfxMacIconBar::handleEvents() {
 		return NULL_REG;
 
 	// Remove event from queue
-	evtMgr->getSciEvent(SCI_EVENT_MOUSE_PRESS);
+	evtMgr->getSciEvent(kSciEventMousePress);
 
 	// Mouse press on the icon bar, check the icon rectangles
 	uint iconNr;
@@ -273,14 +273,14 @@ reg_t GfxMacIconBar::handleEvents() {
 	bool isSelected = true;
 
 	// Wait for mouse release
-	while (evt.type != SCI_EVENT_MOUSE_RELEASE) {
+	while (evt.type != kSciEventMouseRelease) {
 		// Mimic behavior of SSCI when moving mouse with button held down
 		if (isSelected != pointOnIcon(iconNr, evt.mousePos)) {
 			isSelected = !isSelected;
 			drawIcon(iconNr, isSelected);
 		}
 
-		evt = evtMgr->getSciEvent(SCI_EVENT_MOUSE_RELEASE);
+		evt = evtMgr->getSciEvent(kSciEventMouseRelease);
 		g_system->delayMillis(10);
 	}
 
diff --git a/engines/sci/graphics/menu.cpp b/engines/sci/graphics/menu.cpp
index b7030bb..e677f0b 100644
--- a/engines/sci/graphics/menu.cpp
+++ b/engines/sci/graphics/menu.cpp
@@ -142,7 +142,7 @@ void GfxMenu::kernelAddEntry(Common::String title, Common::String content, reg_t
 		// Control/Alt/Function key mapping...
 		if (controlPos) {
 			content.setChar(SCI_MENU_REPLACE_ONCONTROL, controlPos);
-			itemEntry->keyModifier = SCI_KEYMOD_CTRL;
+			itemEntry->keyModifier = kSciKeyModCtrl;
 			tempPos = controlPos + 1;
 			if (tempPos >= contentSize)
 				error("control marker at end of item");
@@ -151,7 +151,7 @@ void GfxMenu::kernelAddEntry(Common::String title, Common::String content, reg_t
 		}
 		if (altPos) {
 			content.setChar(SCI_MENU_REPLACE_ONALT, altPos);
-			itemEntry->keyModifier = SCI_KEYMOD_ALT;
+			itemEntry->keyModifier = kSciKeyModAlt;
 			tempPos = altPos + 1;
 			if (tempPos >= contentSize)
 				error("alt marker at end of item");
@@ -165,16 +165,16 @@ void GfxMenu::kernelAddEntry(Common::String title, Common::String content, reg_t
 				error("function marker at end of item");
 			itemEntry->keyPress = content[tempPos];
 			switch (content[functionPos + 1]) {
-			case '1': itemEntry->keyPress = SCI_KEY_F1; break;
-			case '2': itemEntry->keyPress = SCI_KEY_F2; break;
-			case '3': itemEntry->keyPress = SCI_KEY_F3; break;
-			case '4': itemEntry->keyPress = SCI_KEY_F4; break;
-			case '5': itemEntry->keyPress = SCI_KEY_F5; break;
-			case '6': itemEntry->keyPress = SCI_KEY_F6; break;
-			case '7': itemEntry->keyPress = SCI_KEY_F7; break;
-			case '8': itemEntry->keyPress = SCI_KEY_F8; break;
-			case '9': itemEntry->keyPress = SCI_KEY_F9; break;
-			case '0': itemEntry->keyPress = SCI_KEY_F10; break;
+			case '1': itemEntry->keyPress = kSciKeyF1; break;
+			case '2': itemEntry->keyPress = kSciKeyF2; break;
+			case '3': itemEntry->keyPress = kSciKeyF3; break;
+			case '4': itemEntry->keyPress = kSciKeyF4; break;
+			case '5': itemEntry->keyPress = kSciKeyF5; break;
+			case '6': itemEntry->keyPress = kSciKeyF6; break;
+			case '7': itemEntry->keyPress = kSciKeyF7; break;
+			case '8': itemEntry->keyPress = kSciKeyF8; break;
+			case '9': itemEntry->keyPress = kSciKeyF9; break;
+			case '0': itemEntry->keyPress = kSciKeyF10; break;
 			default:
 				error("illegal function key specified");
 			}
@@ -215,7 +215,7 @@ void GfxMenu::kernelAddEntry(Common::String title, Common::String content, reg_t
 			tempPtr = itemEntry->text.c_str();
 			tempPtr = strstr(tempPtr, "Ctrl-");
 			if (tempPtr) {
-				itemEntry->keyModifier = SCI_KEYMOD_CTRL;
+				itemEntry->keyModifier = kSciKeyModCtrl;
 				itemEntry->keyPress = tolower(tempPtr[5]);
 			}
 		}
@@ -403,7 +403,7 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 	bool forceClaimed = false;
 
 	switch (eventType) {
-	case SCI_EVENT_KEYBOARD:
+	case kSciEventKeyDown:
 		keyPress = readSelectorValue(_segMan, eventObject, SELECTOR(message));
 		keyModifier = readSelectorValue(_segMan, eventObject, SELECTOR(modifiers));
 
@@ -411,14 +411,14 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 		// Ctrl+<key> is pressed, but this kMenuSelect implementation matches
 		// on modifier + printable character, so we must convert the control
 		// characters to their lower-case latin printed equivalents
-		if ((keyModifier & SCI_KEYMOD_NON_STICKY) == SCI_KEYMOD_CTRL && keyPress > 0 && keyPress < 27) {
+		if ((keyModifier & kSciKeyModNonSticky) == kSciKeyModCtrl && keyPress > 0 && keyPress < 27) {
 			keyPress += 96;
 		}
 
 		switch (keyPress) {
 		case 0:
 			break;
-		case SCI_KEY_ESC:
+		case kSciKeyEsc:
 			interactiveStart(pauseSound);
 			itemEntry = interactiveWithKeyboard();
 			interactiveEnd(pauseSound);
@@ -434,8 +434,8 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 				// to Ctrl+I or the modifier check will fail and the Tab key
 				// won't do anything. (This is also why Ctrl+I and Ctrl+Shift+I
 				// would both bring up the inventory in SSCI QFG1EGA)
-				if (keyPress == SCI_KEY_TAB) {
-					keyModifier = SCI_KEYMOD_CTRL;
+				if (keyPress == kSciKeyTab) {
+					keyModifier = kSciKeyModCtrl;
 					keyPress = 'i';
 				}
 
@@ -455,7 +455,7 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 		}
 		break;
 
-	case SCI_EVENT_SAID:
+	case kSciEventSaid:
 		while (itemIterator != itemEnd) {
 			itemEntry = *itemIterator;
 
@@ -476,7 +476,7 @@ reg_t GfxMenu::kernelSelect(reg_t eventObject, bool pauseSound) {
 			itemEntry = NULL;
 		break;
 
-	case SCI_EVENT_MOUSE_PRESS: {
+	case kSciEventMousePress: {
 		Common::Point mousePosition;
 		mousePosition.x = readSelectorValue(_segMan, eventObject, SELECTOR(x));
 		mousePosition.y = readSelectorValue(_segMan, eventObject, SELECTOR(y));
@@ -755,34 +755,34 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 	_paint16->bitsShow(_menuRect);
 
 	while (true) {
-		curEvent = _event->getSciEvent(SCI_EVENT_ANY);
+		curEvent = _event->getSciEvent(kSciEventAny);
 
 		switch (curEvent.type) {
-		case SCI_EVENT_KEYBOARD:
+		case kSciEventKeyDown:
 			// We don't 100% follow sierra here:
 			// - sierra didn't wrap around when changing item id
 			// - sierra allowed item id to be 0, which didn't make any sense
 			do {
 				switch (curEvent.character) {
-				case SCI_KEY_ESC:
+				case kSciKeyEsc:
 					_curMenuId = curItemEntry->menuId; _curItemId = curItemEntry->id;
 					return NULL;
-				case SCI_KEY_ENTER:
+				case kSciKeyEnter:
 					if (curItemEntry->enabled)  {
 						_curMenuId = curItemEntry->menuId; _curItemId = curItemEntry->id;
 						return curItemEntry;
 					}
 					break;
-				case SCI_KEY_LEFT:
+				case kSciKeyLeft:
 					newMenuId--; newItemId = 1;
 					break;
-				case SCI_KEY_RIGHT:
+				case kSciKeyRight:
 					newMenuId++; newItemId = 1;
 					break;
-				case SCI_KEY_UP:
+				case kSciKeyUp:
 					newItemId--;
 					break;
-				case SCI_KEY_DOWN:
+				case kSciKeyDown:
 					newItemId++;
 					break;
 				}
@@ -793,9 +793,9 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 
 					// if we do this step again because of a separator line -> don't repeat left/right, but go down
 					switch (curEvent.character) {
-					case SCI_KEY_LEFT:
-					case SCI_KEY_RIGHT:
-						curEvent.character = SCI_KEY_DOWN;
+					case kSciKeyLeft:
+					case kSciKeyRight:
+						curEvent.character = kSciKeyDown;
 					}
 				}
 			} while (newItemEntry->separatorLine);
@@ -813,7 +813,7 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 			}
 			break;
 
-		case SCI_EVENT_MOUSE_PRESS: {
+		case kSciEventMousePress: {
 			Common::Point mousePosition = curEvent.mousePos;
 			if (mousePosition.y < 10) {
 				// Somewhere on the menubar
@@ -846,9 +846,12 @@ GuiMenuItemEntry *GfxMenu::interactiveWithKeyboard() {
 			}
 			} break;
 
-		case SCI_EVENT_NONE:
+		case kSciEventNone:
 			g_sci->sleep(2500 / 1000);
 			break;
+
+		default:
+			break;
 		}
 	}
 }
@@ -875,19 +878,22 @@ GuiMenuItemEntry *GfxMenu::interactiveWithMouse() {
 	_paint16->bitsShow(_ports->_menuRect);
 
 	while (true) {
-		curEvent = _event->getSciEvent(SCI_EVENT_ANY);
+		curEvent = _event->getSciEvent(kSciEventAny);
 
 		switch (curEvent.type) {
-		case SCI_EVENT_MOUSE_RELEASE:
+		case kSciEventMouseRelease:
 			if ((curMenuId == 0) || (curItemId == 0))
 				return NULL;
 			if ((!curItemEntry->enabled) || (curItemEntry->separatorLine))
 				return NULL;
 			return curItemEntry;
 
-		case SCI_EVENT_NONE:
+		case kSciEventNone:
 			g_sci->sleep(2500 / 1000);
 			break;
+
+		default:
+			break;
 		}
 
 		// Find out where mouse is currently pointing to
diff --git a/engines/sci/graphics/portrait.cpp b/engines/sci/graphics/portrait.cpp
index 6b66ac5..8c41dab 100644
--- a/engines/sci/graphics/portrait.cpp
+++ b/engines/sci/graphics/portrait.cpp
@@ -302,9 +302,9 @@ void Portrait::doit(Common::Point position, uint16 resourceId, uint16 noun, uint
 		if (timerPosition > 0) {
 			do {
 				g_sci->getEngineState()->wait(1);
-				curEvent = _event->getSciEvent(SCI_EVENT_ANY);
-				if (curEvent.type == SCI_EVENT_MOUSE_PRESS ||
-					(curEvent.type == SCI_EVENT_KEYBOARD && curEvent.character == SCI_KEY_ESC) ||
+				curEvent = _event->getSciEvent(kSciEventAny);
+				if (curEvent.type == kSciEventMousePress ||
+					(curEvent.type == kSciEventKeyDown && curEvent.character == kSciKeyEsc) ||
 					g_sci->getEngineState()->abortScriptProcessing == kAbortQuitGame ||
 					g_sci->getEngineState()->_delayedRestoreGameId != -1)
 					userAbort = true;
@@ -325,9 +325,9 @@ void Portrait::doit(Common::Point position, uint16 resourceId, uint16 noun, uint
 
 				do {
 					g_sci->getEngineState()->wait(1);
-					curEvent = _event->getSciEvent(SCI_EVENT_ANY);
-					if (curEvent.type == SCI_EVENT_MOUSE_PRESS ||
-						(curEvent.type == SCI_EVENT_KEYBOARD && curEvent.character == SCI_KEY_ESC) ||
+					curEvent = _event->getSciEvent(kSciEventAny);
+					if (curEvent.type == kSciEventMousePress ||
+						(curEvent.type == kSciEventKeyDown && curEvent.character == kSciKeyEsc) ||
 						g_sci->getEngineState()->abortScriptProcessing == kAbortQuitGame)
 						userAbort = true;
 					curPosition = _audio->getAudioPosition();
@@ -384,9 +384,9 @@ void Portrait::doit(Common::Point position, uint16 resourceId, uint16 noun, uint
 		// Wait till syncTime passed, then show specific animation bitmap
 		do {
 			g_sci->getEngineState()->wait(1);
-			curEvent = _event->getSciEvent(SCI_EVENT_ANY);
-			if (curEvent.type == SCI_EVENT_MOUSE_PRESS ||
-				(curEvent.type == SCI_EVENT_KEYBOARD && curEvent.data == SCI_KEY_ESC) ||
+			curEvent = _event->getSciEvent(kSciEventAny);
+			if (curEvent.type == kSciEventMousePress ||
+				(curEvent.type == kSciEventKeyboard && curEvent.data == kSciKeyEsc) ||
 				g_sci->getEngineState()->abortScriptProcessing == kAbortQuitGame)
 				userAbort = true;
 			curPosition = _audio->getAudioPosition();
diff --git a/engines/sci/graphics/video32.cpp b/engines/sci/graphics/video32.cpp
index 16884a1..736ee15 100644
--- a/engines/sci/graphics/video32.cpp
+++ b/engines/sci/graphics/video32.cpp
@@ -168,21 +168,21 @@ VideoPlayer::EventFlags VideoPlayer::checkForEvent(const EventFlags flags) {
 		return kEventFlagEnd;
 	}
 
-	SciEvent event = _eventMan->getSciEvent(SCI_EVENT_MOUSE_PRESS | SCI_EVENT_PEEK);
-	if ((flags & kEventFlagMouseDown) && event.type == SCI_EVENT_MOUSE_PRESS) {
+	SciEvent event = _eventMan->getSciEvent(kSciEventMousePress | kSciEventPeek);
+	if ((flags & kEventFlagMouseDown) && event.type == kSciEventMousePress) {
 		return kEventFlagMouseDown;
 	}
 
-	event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD | SCI_EVENT_PEEK);
-	if ((flags & kEventFlagEscapeKey) && event.type == SCI_EVENT_KEYBOARD) {
+	event = _eventMan->getSciEvent(kSciEventKeyDown | kSciEventPeek);
+	if ((flags & kEventFlagEscapeKey) && event.type == kSciEventKeyDown) {
 		if (getSciVersion() < SCI_VERSION_3) {
-			while ((event = _eventMan->getSciEvent(SCI_EVENT_KEYBOARD)),
-				   event.type != SCI_EVENT_NONE) {
-				if (event.character == SCI_KEY_ESC) {
+			while ((event = _eventMan->getSciEvent(kSciEventKeyDown)),
+				   event.type != kSciEventNone) {
+				if (event.character == kSciKeyEsc) {
 					return kEventFlagEscapeKey;
 				}
 			}
-		} else if (event.character == SCI_KEY_ESC) {
+		} else if (event.character == kSciKeyEsc) {
 			return kEventFlagEscapeKey;
 		}
 	}
@@ -734,8 +734,8 @@ VMDPlayer::EventFlags VMDPlayer::checkForEvent(const EventFlags flags) {
 		return stopFlag;
 	}
 
-	const SciEvent event = _eventMan->getSciEvent(SCI_EVENT_HOT_RECTANGLE | SCI_EVENT_PEEK);
-	if ((flags & kEventFlagHotRectangle) && event.type == SCI_EVENT_HOT_RECTANGLE) {
+	const SciEvent event = _eventMan->getSciEvent(kSciEventHotRectangle | kSciEventPeek);
+	if ((flags & kEventFlagHotRectangle) && event.type == kSciEventHotRectangle) {
 		return kEventFlagHotRectangle;
 	}
 
diff --git a/engines/sci/sci.cpp b/engines/sci/sci.cpp
index 6ed7411..c6d8b8b 100644
--- a/engines/sci/sci.cpp
+++ b/engines/sci/sci.cpp
@@ -857,7 +857,8 @@ void SciEngine::sleep(uint32 msecs) {
 
 	for (;;) {
 		// let backend process events and update the screen
-		_eventMan->getSciEvent(SCI_EVENT_PEEK);
+		_eventMan->getSciEvent(kSciEventPeek);
+
 #ifdef ENABLE_SCI32
 		// If a game is in a wait loop, kFrameOut is not called, but mouse
 		// movement is still occurring and the screen needs to be updated to


Commit: 4bd31dae9b638bb6c80ddc3db7b41f34c68626fc
    https://github.com/scummvm/scummvm/commit/4bd31dae9b638bb6c80ddc3db7b41f34c68626fc
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-27T20:27:33-05:00

Commit Message:
SCI: Fix use-after-free when kernel call debugging is active during a save game restore

Changed paths:
    engines/sci/engine/scriptdebug.cpp


diff --git a/engines/sci/engine/scriptdebug.cpp b/engines/sci/engine/scriptdebug.cpp
index cfe0ebe..d46b630 100644
--- a/engines/sci/engine/scriptdebug.cpp
+++ b/engines/sci/engine/scriptdebug.cpp
@@ -988,6 +988,10 @@ void debugPropertyAccess(Object *obj, reg_t objp, unsigned int index, reg_t curV
 }
 
 void logKernelCall(const KernelFunction *kernelCall, const KernelSubFunction *kernelSubCall, EngineState *s, int argc, reg_t *argv, reg_t result) {
+	if (s->abortScriptProcessing != kAbortNone) {
+		return;
+	}
+
 	Kernel *kernel = g_sci->getKernel();
 	if (!kernelSubCall) {
 		debugN("k%s: ", kernelCall->name);


Commit: 7a41b6f023e31a5f6845b9a256d966270fa60151
    https://github.com/scummvm/scummvm/commit/7a41b6f023e31a5f6845b9a256d966270fa60151
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-27T20:27:34-05:00

Commit Message:
SCI: Add support for keyup events

Basic keyup event support appears to have been added in the SCI1.1
IBM keyboard driver, and more robust support was provided in SCI32
which actually gets used by at least Lighthouse. This patch adds
support for keyup events in SCI1.1+.

Fixes Trac#10242.

Changed paths:
    engines/sci/engine/kevent.cpp
    engines/sci/event.cpp


diff --git a/engines/sci/engine/kevent.cpp b/engines/sci/engine/kevent.cpp
index 0f77b47..608e2e4 100644
--- a/engines/sci/engine/kevent.cpp
+++ b/engines/sci/engine/kevent.cpp
@@ -58,6 +58,8 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 
 	// If there's a simkey pending, and the game wants a keyboard event, use the
 	// simkey instead of a normal event
+	// TODO: This does not really work as expected for keyup events, since the
+	// fake event is disposed halfway through the normal event lifecycle.
 	if (g_debug_simulated_key && (mask & kSciEventKeyDown)) {
 		// In case we use a simulated event we query the current mouse position
 		mousePos = g_sci->_gfxCursor->getPosition();
@@ -180,6 +182,7 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 		break;
 
 	case kSciEventKeyDown:
+	case kSciEventKeyUp:
 		writeSelectorValue(segMan, obj, SELECTOR(type), curEvent.type);
 		writeSelectorValue(segMan, obj, SELECTOR(message), curEvent.character);
 		// We only care about the translated character
@@ -230,6 +233,7 @@ reg_t kGetEvent(EngineState *s, int argc, reg_t *argv) {
 			con->debugPrintf("quit event\n");
 			break;
 		case kSciEventKeyDown:
+		case kSciEventKeyUp:
 			con->debugPrintf("keyboard event\n");
 			break;
 		case kSciEventMousePress:
diff --git a/engines/sci/event.cpp b/engines/sci/event.cpp
index 2967b55..e69095a 100644
--- a/engines/sci/event.cpp
+++ b/engines/sci/event.cpp
@@ -253,10 +253,6 @@ SciEvent EventManager::getScummVMEvent() {
 		}
 	}
 
-	// If we reached here, make sure that it's a keydown event
-	if (ev.type != Common::EVENT_KEYDOWN)
-		return noEvent;
-
 	// Check for Control-Shift-D (debug console)
 	if (ev.type == Common::EVENT_KEYDOWN && ev.kbd.hasFlags(Common::KBD_CTRL | Common::KBD_SHIFT) && ev.kbd.keycode == Common::KEYCODE_d) {
 		// Open debug console
@@ -265,19 +261,23 @@ SciEvent EventManager::getScummVMEvent() {
 		return noEvent;
 	}
 
+	// The IBM keyboard driver prior to SCI1.1 only sent keydown events to the
+	// interpreter
+	if (ev.type != Common::EVENT_KEYDOWN && getSciVersion() < SCI_VERSION_1_1) {
+		return noEvent;
+	}
 
 	const Common::KeyCode scummVMKeycode = ev.kbd.keycode;
 
 	input.character = ev.kbd.ascii;
-	input.type = kSciEventKeyDown;
-
-	if (scummVMKeycode >= Common::KEYCODE_KP0 && scummVMKeycode <= Common::KEYCODE_KP9) {
-		if (!(scummVMKeyFlags & Common::KBD_NUM)) {
-			// HACK: Num-Lock not enabled
-			// We shouldn't get a valid ascii code in these cases. We fix it here, so that cursor keys
-			// on the numpad work properly.
-			input.character = 0;
-		}
+
+	if (scummVMKeycode >= Common::KEYCODE_KP0 && scummVMKeycode <= Common::KEYCODE_KP9 && !(scummVMKeyFlags & Common::KBD_NUM)) {
+		// TODO: Leaky abstractions from SDL should not be handled in game
+		// engines!
+		// SDL may provide a character value for numpad keys even if numlock is
+		// turned off, but arrow/navigation keys won't get mapped below if a
+		// character value is provided
+		input.character = 0;
 	}
 
 	if (input.character && input.character <= 0xFF) {
@@ -336,10 +336,24 @@ SciEvent EventManager::getScummVMEvent() {
 		input.character -= 96;
 	}
 
-	// If no actual key was pressed (e.g. if only a modifier key was pressed),
-	// ignore the event
-	if (!input.character)
+	// In SCI1.1, if only a modifier key is pressed, the IBM keyboard driver
+	// sends an event the same as if a key had been released
+	if (getSciVersion() != SCI_VERSION_1_1 && !input.character) {
 		return noEvent;
+	} else if (!input.character || ev.type == Common::EVENT_KEYUP) {
+		input.type = kSciEventKeyUp;
+
+		// SCI32 includes the released key character code in keyup messages, but
+		// the IBM keyboard driver in SCI1.1 sends a special character value
+		// instead. This is necessary to prevent at least Island of Dr Brain
+		// from processing keyup events as though they were keydown events in
+		// the word search puzzle
+		if (getSciVersion() == SCI_VERSION_1_1) {
+			input.character = 0x8000;
+		}
+	} else {
+		input.type = kSciEventKeyDown;
+	}
 
 	return input;
 }


Commit: a4eb6a5ca55ccf29d905d9947f2355580cf860c3
    https://github.com/scummvm/scummvm/commit/a4eb6a5ca55ccf29d905d9947f2355580cf860c3
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-09-27T20:27:34-05:00

Commit Message:
SCI: Remove old SCI32 view scaling code from SCI16 graphics code

Changed paths:
    engines/sci/graphics/compare.cpp
    engines/sci/graphics/screen.cpp
    engines/sci/graphics/screen.h
    engines/sci/graphics/view.cpp
    engines/sci/graphics/view.h


diff --git a/engines/sci/graphics/compare.cpp b/engines/sci/graphics/compare.cpp
index 582dd32..736f009 100644
--- a/engines/sci/graphics/compare.cpp
+++ b/engines/sci/graphics/compare.cpp
@@ -224,15 +224,7 @@ void GfxCompare::kernelBaseSetter(reg_t object) {
 		if (scaleSignal & kScaleSignalDoScaling) {
 			celRect = getNSRect(object);
 		} else {
-			if (tmpView->isSci2Hires())
-				tmpView->adjustToUpscaledCoordinates(y, x);
-
 			tmpView->getCelRect(loopNo, celNo, x, y, z, celRect);
-
-			if (tmpView->isSci2Hires()) {
-				tmpView->adjustBackUpscaledCoordinates(celRect.top, celRect.left);
-				tmpView->adjustBackUpscaledCoordinates(celRect.bottom, celRect.right);
-			}
 		}
 
 		celRect.bottom = y + 1;
diff --git a/engines/sci/graphics/screen.cpp b/engines/sci/graphics/screen.cpp
index e69c432..1af3667 100644
--- a/engines/sci/graphics/screen.cpp
+++ b/engines/sci/graphics/screen.cpp
@@ -760,37 +760,16 @@ void GfxScreen::scale2x(const SciSpan<const byte> &src, SciSpan<byte> &dst, int1
 
 struct UpScaledAdjust {
 	GfxScreenUpscaledMode gameHiresMode;
-	Sci32ViewNativeResolution viewNativeRes;
 	int numerator;
 	int denominator;
 };
 
-static const UpScaledAdjust s_upscaledAdjustTable[] = {
-	{ GFX_SCREEN_UPSCALED_640x480, SCI_VIEW_NATIVERES_640x400, 5, 6 }
-};
-
-void GfxScreen::adjustToUpscaledCoordinates(int16 &y, int16 &x, Sci32ViewNativeResolution viewNativeRes) {
+void GfxScreen::adjustToUpscaledCoordinates(int16 &y, int16 &x) {
 	x = _upscaledWidthMapping[x];
 	y = _upscaledHeightMapping[y];
-
-	for (int i = 0; i < ARRAYSIZE(s_upscaledAdjustTable); i++) {
-		if (s_upscaledAdjustTable[i].gameHiresMode == _upscaledHires &&
-				s_upscaledAdjustTable[i].viewNativeRes == viewNativeRes) {
-			y = (y * s_upscaledAdjustTable[i].numerator) / s_upscaledAdjustTable[i].denominator;
-			break;
-		}
-	}
 }
 
-void GfxScreen::adjustBackUpscaledCoordinates(int16 &y, int16 &x, Sci32ViewNativeResolution viewNativeRes) {
-	for (int i = 0; i < ARRAYSIZE(s_upscaledAdjustTable); i++) {
-		if (s_upscaledAdjustTable[i].gameHiresMode == _upscaledHires &&
-				s_upscaledAdjustTable[i].viewNativeRes == viewNativeRes) {
-			y = (y * s_upscaledAdjustTable[i].denominator) / s_upscaledAdjustTable[i].numerator;
-			break;
-		}
-	}
-
+void GfxScreen::adjustBackUpscaledCoordinates(int16 &y, int16 &x) {
 	switch (_upscaledHires) {
 	case GFX_SCREEN_UPSCALED_480x300:
 		x = (x * 4) / 6;
diff --git a/engines/sci/graphics/screen.h b/engines/sci/graphics/screen.h
index 46214b7..8bdf647 100644
--- a/engines/sci/graphics/screen.h
+++ b/engines/sci/graphics/screen.h
@@ -119,8 +119,8 @@ public:
 
 	void scale2x(const SciSpan<const byte> &src, SciSpan<byte> &dst, int16 srcWidth, int16 srcHeight, byte bytesPerPixel = 1);
 
-	void adjustToUpscaledCoordinates(int16 &y, int16 &x, Sci32ViewNativeResolution viewScalingType = SCI_VIEW_NATIVERES_NONE);
-	void adjustBackUpscaledCoordinates(int16 &y, int16 &x, Sci32ViewNativeResolution viewScalingType = SCI_VIEW_NATIVERES_NONE);
+	void adjustToUpscaledCoordinates(int16 &y, int16 &x);
+	void adjustBackUpscaledCoordinates(int16 &y, int16 &x);
 
 	void dither(bool addToFlag);
 
diff --git a/engines/sci/graphics/view.cpp b/engines/sci/graphics/view.cpp
index f593225..02ffc24 100644
--- a/engines/sci/graphics/view.cpp
+++ b/engines/sci/graphics/view.cpp
@@ -119,7 +119,6 @@ void GfxView::initData(GuiResourceId resourceId) {
 	_loop.resize(0);
 	_embeddedPal = false;
 	_EGAmapping.clear();
-	_sci2ScaleRes = SCI_VIEW_NATIVERES_NONE;
 	_isScaleable = true;
 
 	// we adjust inside getCelRect for SCI0EARLY (that version didn't have the +1 when calculating bottom)
@@ -415,10 +414,6 @@ Palette *GfxView::getPalette() {
 	return _embeddedPal ? &_viewPalette : NULL;
 }
 
-bool GfxView::isSci2Hires() {
-	return _sci2ScaleRes > SCI_VIEW_NATIVERES_320x200;
-}
-
 bool GfxView::isScaleable() {
 	return _isScaleable;
 }
@@ -933,11 +928,11 @@ void GfxView::drawScaled(const Common::Rect &rect, const Common::Rect &clipRect,
 }
 
 void GfxView::adjustToUpscaledCoordinates(int16 &y, int16 &x) {
-	_screen->adjustToUpscaledCoordinates(y, x, _sci2ScaleRes);
+	_screen->adjustToUpscaledCoordinates(y, x);
 }
 
 void GfxView::adjustBackUpscaledCoordinates(int16 &y, int16 &x) {
-	_screen->adjustBackUpscaledCoordinates(y, x, _sci2ScaleRes);
+	_screen->adjustBackUpscaledCoordinates(y, x);
 }
 
 } // End of namespace Sci
diff --git a/engines/sci/graphics/view.h b/engines/sci/graphics/view.h
index d4ec167..5ecf7e6 100644
--- a/engines/sci/graphics/view.h
+++ b/engines/sci/graphics/view.h
@@ -27,13 +27,6 @@
 
 namespace Sci {
 
-enum Sci32ViewNativeResolution {
-	SCI_VIEW_NATIVERES_NONE = -1,
-	SCI_VIEW_NATIVERES_320x200 = 0,
-	SCI_VIEW_NATIVERES_640x480 = 1,
-	SCI_VIEW_NATIVERES_640x400 = 2
-};
-
 struct CelInfo {
 	int16 width, height;
 	int16 scriptWidth, scriptHeight;
@@ -84,7 +77,6 @@ public:
 	Palette *getPalette();
 
 	bool isScaleable();
-	bool isSci2Hires();
 
 	void adjustToUpscaledCoordinates(int16 &y, int16 &x);
 	void adjustBackUpscaledCoordinates(int16 &y, int16 &x);
@@ -106,9 +98,6 @@ private:
 	bool _embeddedPal;
 	Palette _viewPalette;
 
-	// specifies scaling resolution for SCI2 views (see gk1/windows, Wolfgang in room 720)
-	Sci32ViewNativeResolution _sci2ScaleRes;
-
 	SciSpan<const byte> _EGAmapping;
 
 	// this is set for sci0early to adjust for the getCelRect() change





More information about the Scummvm-git-logs mailing list