Merge pull request #1292 from TomHarte/KeyRepeat

Indicate whether a keypress is a repeat. Treat appropriately in the Apple II.

Analyser/Dynamic/MultiMachine/Implementation/MultiKeyboardMachine.cpp

@@ -56,10 +56,10 @@ MultiKeyboardMachine::MultiKeyboard::MultiKeyboard(const std::vector<::MachineTy
 	}
 }
 
-bool MultiKeyboardMachine::MultiKeyboard::set_key_pressed(Key key, char value, bool is_pressed) {
+bool MultiKeyboardMachine::MultiKeyboard::set_key_pressed(Key key, char value, bool is_pressed, bool is_repeat) {
 	bool was_consumed = false;
 	for(const auto &machine: machines_) {
-		was_consumed |= machine->get_keyboard().set_key_pressed(key, value, is_pressed);
+		was_consumed |= machine->get_keyboard().set_key_pressed(key, value, is_pressed, is_repeat);
 	}
 	return was_consumed;
 }

Analyser/Dynamic/MultiMachine/Implementation/MultiKeyboardMachine.hpp

@@ -31,7 +31,7 @@ class MultiKeyboardMachine: public MachineTypes::KeyboardMachine {
 			public:
 				MultiKeyboard(const std::vector<MachineTypes::KeyboardMachine *> &machines);
 
-				bool set_key_pressed(Key key, char value, bool is_pressed) final;
+				bool set_key_pressed(Key key, char value, bool is_pressed, bool is_repeat) final;
 				void reset_all_keys() final;
 				const std::set<Key> &observed_keys() const final;
 				bool is_exclusive() const final;

Inputs/Keyboard.cpp

@@ -21,7 +21,7 @@ Keyboard::Keyboard(const std::set<Key> &essential_modifiers) : essential_modifie
 Keyboard::Keyboard(const std::set<Key> &observed_keys, const std::set<Key> &essential_modifiers) :
 	observed_keys_(observed_keys), essential_modifiers_(essential_modifiers), is_exclusive_(false) {}
 
-bool Keyboard::set_key_pressed(Key key, char, bool is_pressed) {
+bool Keyboard::set_key_pressed(Key key, char, bool is_pressed, bool) {
 	const size_t key_offset = size_t(key);
 	if(key_offset >= key_states_.size()) {
 		key_states_.resize(key_offset+1, false);

Inputs/Keyboard.hpp

@@ -51,7 +51,7 @@ class Keyboard {
 		// Host interface.
 
 		/// @returns @c true if the key press affects the machine; @c false otherwise.
-		virtual bool set_key_pressed(Key key, char value, bool is_pressed);
+		virtual bool set_key_pressed(Key key, char value, bool is_pressed, bool is_repeat);
 		virtual void reset_all_keys();
 
 		/// @returns a set of all Keys that this keyboard responds to.

Machines/Apple/AppleII/AppleII.cpp

@@ -276,10 +276,20 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 			Keyboard(Processor *m6502) : m6502_(m6502) {}
 
 			void reset_all_keys() final {
-				open_apple_is_pressed = closed_apple_is_pressed = control_is_pressed = shift_is_pressed = key_is_down = false;
+				open_apple_is_pressed =
+				closed_apple_is_pressed =
+				control_is_pressed_ =
+				shift_is_pressed_ =
+				repeat_is_pressed_ =
+				key_is_down_ =
+				character_is_pressed_ = false;
 			}
 
-			bool set_key_pressed(Key key, char value, bool is_pressed) final {
+			bool set_key_pressed(Key key, char value, bool is_pressed, bool is_repeat) final {
+				if constexpr (!is_iie()) {
+					if(is_repeat && !repeat_is_pressed_) return true;
+				}
+
 				// If no ASCII value is supplied, look for a few special cases.
 				switch(key) {
 					case Key::Left:			value = 0x08;	break;
@@ -323,17 +333,27 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 						}
 
 					case Key::LeftControl:
-						control_is_pressed = is_pressed;
+						control_is_pressed_ = is_pressed;
 					return true;
 
 					case Key::LeftShift:
 					case Key::RightShift:
-						shift_is_pressed = is_pressed;
+						shift_is_pressed_ = is_pressed;
 					return true;
 
 					case Key::F1:	case Key::F2:	case Key::F3:	case Key::F4:
 					case Key::F5:	case Key::F6:	case Key::F7:	case Key::F8:
-					case Key::F9:	case Key::F10:	case Key::F11:	case Key::F12:
+					case Key::F9:	case Key::F10:	case Key::F11:
+						repeat_is_pressed_ = is_pressed;
+
+						if constexpr (!is_iie()) {
+							if(is_pressed && (!is_repeat || character_is_pressed_)) {
+								keyboard_input_ = uint8_t(last_pressed_character_ | 0x80);
+							}
+						}
+					return true;
+
+					case Key::F12:
 					case Key::PrintScreen:
 					case Key::ScrollLock:
 					case Key::Pause:
@@ -356,10 +376,10 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 						// Prior to the IIe, the keyboard could produce uppercase only.
 						if(!is_iie()) value = char(toupper(value));
 
-						if(control_is_pressed && isalpha(value)) value &= 0xbf;
+						if(control_is_pressed_ && isalpha(value)) value &= 0xbf;
 
 						// TODO: properly map IIe keys
-						if(!is_iie() && shift_is_pressed) {
+						if(!is_iie() && shift_is_pressed_) {
 							switch(value) {
 							case 0x27: value = 0x22; break; // ' -> "
 							case 0x2c: value = 0x3c; break; // , -> <
@@ -383,11 +403,16 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 				}
 
 				if(is_pressed) {
-					keyboard_input = uint8_t(value | 0x80);
+					last_pressed_character_ = value;
-					key_is_down = true;
+					character_is_pressed_ = true;
+					keyboard_input_ = uint8_t(value | 0x80);
+					key_is_down_ = true;
 				} else {
-					if((keyboard_input & 0x3f) == value) {
+					if(value == last_pressed_character_) {
-						key_is_down = false;
+						character_is_pressed_ = false;
+					}
+					if((keyboard_input_ & 0x3f) == value) {
+						key_is_down_ = false;
 					}
 				}
 
@@ -395,24 +420,54 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 			}
 
 			uint8_t get_keyboard_input() {
-				if(string_serialiser) {
+				if(string_serialiser_) {
-					return string_serialiser->head() | 0x80;
+					return string_serialiser_->head() | 0x80;
 				} else {
-					return keyboard_input;
+					return keyboard_input_;
 				}
 			}
 
-			bool shift_is_pressed = false;
+			void clear_keyboard_input() {
-			bool control_is_pressed = false;
+				keyboard_input_ &= 0x7f;
+				if(string_serialiser_ && !string_serialiser_->advance()) {
+					string_serialiser_.reset();
+				}
+			}
+
+			bool get_key_is_down() {
+				return key_is_down_;
+			}
+
+			void set_string_serialiser(std::unique_ptr<Utility::StringSerialiser> &&serialiser) {
+				string_serialiser_ = std::move(serialiser);
+			}
+
 			// The IIe has three keys that are wired directly to the same input as the joystick buttons.
 			bool open_apple_is_pressed = false;
 			bool closed_apple_is_pressed = false;
-			uint8_t keyboard_input = 0x00;
-			bool key_is_down = false;
-			std::unique_ptr<Utility::StringSerialiser> string_serialiser;
 
 			private:
-			Processor *const m6502_;
+				// Current keyboard input register, as exposed to the programmer; on the IIe the programmer
+				// can also poll for whether any key is currently down.
+				uint8_t keyboard_input_ = 0x00;
+				bool key_is_down_ = false;
+
+				// ASCII input state, referenced by the REPT key on models before the IIe.
+				char last_pressed_character_ = 0;
+				bool character_is_pressed_ = false;
+
+				// The repeat key itself.
+				bool repeat_is_pressed_ = false;
+
+				// Modifier states.
+				bool shift_is_pressed_ = false;
+				bool control_is_pressed_ = false;
+
+				// A string serialiser for receiving copy and paste.
+				std::unique_ptr<Utility::StringSerialiser> string_serialiser_;
+
+				// 6502 connection, for applying the reset button.
+				Processor *const m6502_;
 		};
 		Keyboard keyboard_;
 
@@ -727,15 +782,11 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 					break;
 
 					case 0xc010:
-						keyboard_.keyboard_input &= 0x7f;
+						keyboard_.clear_keyboard_input();
-						if(keyboard_.string_serialiser) {
-							if(!keyboard_.string_serialiser->advance())
-								keyboard_.string_serialiser.reset();
-						}
 
 						// On the IIe, reading C010 returns additional key info.
 						if(is_iie() && isReadOperation(operation)) {
-							*value = (keyboard_.key_is_down ? 0x80 : 0x00) | (keyboard_.keyboard_input & 0x7f);
+							*value = (keyboard_.get_key_is_down() ? 0x80 : 0x00) | (keyboard_.get_keyboard_input() & 0x7f);
 						}
 					break;
 
@@ -879,7 +930,7 @@ template <Analyser::Static::AppleII::Target::Model model> class ConcreteMachine:
 		}
 
 		void type_string(const std::string &string) final {
-			keyboard_.string_serialiser = std::make_unique<Utility::StringSerialiser>(string, true);
+			keyboard_.set_string_serialiser(std::make_unique<Utility::StringSerialiser>(string, true));
 		}
 
 		bool can_type(char c) const final {

Machines/KeyboardMachine.hpp

@@ -75,12 +75,12 @@ class KeyboardMachine: public KeyActions {
 				(i) if @c symbol can be typed and this is a key down, @c type_string it;
 				(ii) if @c symbol cannot be typed, set @c key as @c is_pressed
 		*/
-		bool apply_key(Inputs::Keyboard::Key key, char symbol, bool is_pressed, bool map_logically) {
+		bool apply_key(Inputs::Keyboard::Key key, char symbol, bool is_pressed, bool is_repeat, bool map_logically) {
 			Inputs::Keyboard &keyboard = get_keyboard();
 
 			if(!map_logically) {
 				// Try a regular keypress first, and stop if that works.
-				if(keyboard.set_key_pressed(key, symbol, is_pressed)) {
+				if(keyboard.set_key_pressed(key, symbol, is_pressed, is_repeat)) {
 					return true;
 				}
 
@@ -103,7 +103,7 @@ class KeyboardMachine: public KeyActions {
 				// That didn't work. Forward as a keypress. As, either:
 				//	(i) this is a key down, but doesn't have a symbol, or is an untypeable symbol; or
 				//	(ii) this is a key up, which it won't be an issue to miscommunicate.
-				return keyboard.set_key_pressed(key, symbol, is_pressed);
+				return keyboard.set_key_pressed(key, symbol, is_pressed, is_repeat);
 			}
 		}
 };

OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj

@@ -6592,6 +6592,7 @@
 					"$(inherited)",
 					IGNORE_APPLE,
 				);
+				HEADER_SEARCH_PATHS = "$(USER_LIBRARY_DIR)/Frameworks/SDL2.framework/Headers";
 				LD_RUNPATH_SEARCH_PATHS = "$(USER_LIBRARY_DIR)/Frameworks";
 				MACOSX_DEPLOYMENT_TARGET = 10.13;
 				PRODUCT_NAME = "$(TARGET_NAME)";
@@ -6613,6 +6614,7 @@
 					NDEBUG,
 					IGNORE_APPLE,
 				);
+				HEADER_SEARCH_PATHS = "$(USER_LIBRARY_DIR)/Frameworks/SDL2.framework/Headers";
 				LD_RUNPATH_SEARCH_PATHS = "$(USER_LIBRARY_DIR)/Frameworks";
 				MACOSX_DEPLOYMENT_TARGET = 10.13;
 				ONLY_ACTIVE_ARCH = YES;

OSBindings/Mac/Clock Signal/Documents/MachineDocument.swift

@@ -345,14 +345,14 @@ class MachineDocument:
 	/// Forwards key down events directly to the machine.
 	func keyDown(_ event: NSEvent) {
 		if let machine = self.machine {
-			machine.setKey(event.keyCode, characters: event.characters, isPressed: true)
+			machine.setKey(event.keyCode, characters: event.characters, isPressed: true, isRepeat: event.isARepeat)
 		}
 	}
 
 	/// Forwards key up events directly to the machine.
 	func keyUp(_ event: NSEvent) {
 		if let machine = self.machine {
-			machine.setKey(event.keyCode, characters: event.characters, isPressed: false)
+			machine.setKey(event.keyCode, characters: event.characters, isPressed: false, isRepeat: false)
 		}
 	}
 
@@ -361,19 +361,19 @@ class MachineDocument:
 		if let machine = self.machine {
 			if newModifiers.modifierFlags.contains(.shift) != shiftIsDown {
 				shiftIsDown = newModifiers.modifierFlags.contains(.shift)
-				machine.setKey(VK_Shift, characters: nil, isPressed: shiftIsDown)
+				machine.setKey(VK_Shift, characters: nil, isPressed: shiftIsDown, isRepeat: false)
 			}
 			if newModifiers.modifierFlags.contains(.control) != controlIsDown {
 				controlIsDown = newModifiers.modifierFlags.contains(.control)
-				machine.setKey(VK_Control, characters: nil, isPressed: controlIsDown)
+				machine.setKey(VK_Control, characters: nil, isPressed: controlIsDown, isRepeat: false)
 			}
 			if newModifiers.modifierFlags.contains(.command) != commandIsDown {
 				commandIsDown = newModifiers.modifierFlags.contains(.command)
-				machine.setKey(VK_Command, characters: nil, isPressed: commandIsDown)
+				machine.setKey(VK_Command, characters: nil, isPressed: commandIsDown, isRepeat: false)
 			}
 			if newModifiers.modifierFlags.contains(.option) != optionIsDown {
 				optionIsDown = newModifiers.modifierFlags.contains(.option)
-				machine.setKey(VK_Option, characters: nil, isPressed: optionIsDown)
+				machine.setKey(VK_Option, characters: nil, isPressed: optionIsDown, isRepeat: false)
 			}
 		}
 	}

OSBindings/Mac/Clock Signal/Machine/CSMachine.h

@@ -67,7 +67,7 @@ typedef NS_ENUM(NSInteger, CSMachineKeyboardInputMode) {
 - (void)start;
 - (void)stop;
 
-- (void)setKey:(uint16_t)key characters:(nullable NSString *)characters isPressed:(BOOL)isPressed;
+- (void)setKey:(uint16_t)key characters:(nullable NSString *)characters isPressed:(BOOL)isPressed isRepeat:(BOOL)isRepeat;
 - (void)clearAllKeys;
 
 - (void)setMouseButton:(int)button isPressed:(BOOL)isPressed;

OSBindings/Mac/Clock Signal/Machine/CSMachine.mm

@@ -366,7 +366,7 @@ struct ActivityObserver: public Activity::Observer {
 	[self updateJoystickTimer];
 }
 
-- (void)setKey:(uint16_t)key characters:(NSString *)characters isPressed:(BOOL)isPressed {
+- (void)setKey:(uint16_t)key characters:(NSString *)characters isPressed:(BOOL)isPressed isRepeat:(BOOL)isRepeat {
 	[self applyInputEvent:^{
 		auto keyboard_machine = self->_machine->keyboard_machine();
 		if(keyboard_machine && (self.inputMode != CSMachineKeyboardInputModeJoystick || !keyboard_machine->get_keyboard().is_exclusive())) {
@@ -437,7 +437,13 @@ struct ActivityObserver: public Activity::Observer {
 			}
 
 			@synchronized(self) {
-				if(keyboard_machine->apply_key(mapped_key, pressedKey, isPressed, self.inputMode == CSMachineKeyboardInputModeKeyboardLogical)) {
+				if(keyboard_machine->apply_key(
+					mapped_key,
+					pressedKey,
+					isPressed,
+					isRepeat,
+					self.inputMode == CSMachineKeyboardInputModeKeyboardLogical)
+				) {
 					return;
 				}
 			}

OSBindings/Qt/mainwindow.cpp

@@ -920,7 +920,7 @@ bool MainWindow::processEvent(QKeyEvent *event) {
 			if(!keyboardMachine) return true;
 
 			auto &keyboard = keyboardMachine->get_keyboard();
-			keyboard.set_key_pressed(*key, event->text().size() ? event->text()[0].toLatin1() : '\0', isPressed);
+			keyboard.set_key_pressed(*key, event->text().size() ? event->text()[0].toLatin1() : '\0', isPressed, event->isAutoRepeat());
 			if(keyboard.is_exclusive() || keyboard.observed_keys().find(*key) != keyboard.observed_keys().end()) {
 				return false;
 			}

OSBindings/SDL/main.cpp

@@ -976,9 +976,11 @@ int main(int argc, char *argv[]) {
 		SDL_Scancode scancode = SDL_SCANCODE_UNKNOWN;
 		SDL_Keycode keycode = SDLK_UNKNOWN;
 		bool is_down = true;
+		bool repeat = false;
 
 		KeyPress(uint32_t timestamp, const char *text) : timestamp(timestamp), input(text) {}
-		KeyPress(uint32_t timestamp, SDL_Scancode scancode, SDL_Keycode keycode, bool is_down) : timestamp(timestamp), scancode(scancode), keycode(keycode), is_down(is_down) {}
+		KeyPress(uint32_t timestamp, SDL_Scancode scancode, SDL_Keycode keycode, bool is_down, bool repeat) :
+			timestamp(timestamp), scancode(scancode), keycode(keycode), is_down(is_down), repeat(repeat) {}
 		KeyPress() {}
 	};
 	std::vector<KeyPress> keypresses;
@@ -1129,7 +1131,12 @@ int main(int argc, char *argv[]) {
 						break;
 					}
 
-					keypresses.emplace_back(event.text.timestamp, event.key.keysym.scancode, event.key.keysym.sym, event.type == SDL_KEYDOWN);
+					keypresses.emplace_back(
+						event.text.timestamp,
+						event.key.keysym.scancode,
+						event.key.keysym.sym,
+						event.type == SDL_KEYDOWN,
+						event.key.repeat);
 				} break;
 
 				case SDL_MOUSEBUTTONDOWN:
@@ -1214,14 +1221,14 @@ int main(int argc, char *argv[]) {
 					// is sufficiently untested on SDL, and somewhat too reliant on empirical timestamp behaviour,
 					// for it to be trustworthy enough otherwise to expose.
 					if(logical_keyboard) {
-						if(keyboard_machine->apply_key(key, keypress.input.size() ? keypress.input[0] : 0, keypress.is_down, logical_keyboard)) {
+						if(keyboard_machine->apply_key(key, keypress.input.size() ? keypress.input[0] : 0, keypress.is_down, keypress.repeat, logical_keyboard)) {
 							continue;
 						}
 					} else {
 						// This is a slightly terrible way of obtaining a symbol for the key, e.g. for letters it will always return
 						// the capital letter version, at least empirically. But it'll have to do for now.
 						const char *key_name = SDL_GetKeyName(keypress.keycode);
-						if(keyboard_machine->get_keyboard().set_key_pressed(key, (strlen(key_name) == 1) ? key_name[0] : 0, keypress.is_down)) {
+						if(keyboard_machine->get_keyboard().set_key_pressed(key, (strlen(key_name) == 1) ? key_name[0] : 0, keypress.is_down, keypress.repeat)) {
 							continue;
 						}
 					}