AGS: rewrote eAGSMouseButton enum to sync with the script API

* Changed from -1 - based constants to 0 - based constants;
* These were purely internal values, therefore no API would be harmed;
* Removed necessity of converting from internal mouse button code to script and back all the time;
* On a side note, this fixes a bug where WaitMouse etc were returning incorrect values on mouse skip, because we forgot to convert from internal to script code.

From upstream 471098d239b076c494beb2d554572ccb69372667

engines/ags/engine/ac/dialog.cpp

@@ -915,14 +915,14 @@ bool DialogOptions::Run() {
 			parserActivated = 1;
 	}
 
-	int mouseButtonPressed = MouseNone;
-	int mouseWheelTurn = 0;
-	if (run_service_mb_controls(mouseButtonPressed, mouseWheelTurn) && mouseButtonPressed >= 0 &&
+	eAGSMouseButton mbut;
+	int mwheelz;
+	if (run_service_mb_controls(mbut, mwheelz) && mbut > kMouseNone &&
 		!_GP(play).IsIgnoringInput()) {
 		if (mouseison < 0 && !new_custom_render) {
 			if (usingCustomRendering) {
 				_GP(runDialogOptionMouseClickHandlerFunc).params[0].SetDynamicObject(&_GP(ccDialogOptionsRendering), &_GP(ccDialogOptionsRendering));
-				_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32(mouseButtonPressed + 1);
+				_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32(mbut);
 				run_function_on_non_blocking_thread(&_GP(runDialogOptionMouseClickHandlerFunc));
 
 				if (_GP(runDialogOptionMouseClickHandlerFunc).atLeastOneImplementationExists) {
@@ -937,7 +937,7 @@ bool DialogOptions::Run() {
 			parserActivated = 1;
 		} else if (new_custom_render) {
 			_GP(runDialogOptionMouseClickHandlerFunc).params[0].SetDynamicObject(&_GP(ccDialogOptionsRendering), &_GP(ccDialogOptionsRendering));
-			_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32(mouseButtonPressed + 1);
+			_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32(mbut);
 			run_function_on_non_blocking_thread(&_GP(runDialogOptionMouseClickHandlerFunc));
 		} else if (usingCustomRendering) {
 			chose = mouseison;
@@ -949,9 +949,9 @@ bool DialogOptions::Run() {
 	}
 
 	if (usingCustomRendering) {
-		if (mouseWheelTurn != 0) {
+		if (mwheelz != 0) {
 			_GP(runDialogOptionMouseClickHandlerFunc).params[0].SetDynamicObject(&_GP(ccDialogOptionsRendering), &_GP(ccDialogOptionsRendering));
-			_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32((mouseWheelTurn < 0) ? 9 : 8);
+			_GP(runDialogOptionMouseClickHandlerFunc).params[1].SetInt32((mwheelz < 0) ? 9 : 8);
 			run_function_on_non_blocking_thread(&_GP(runDialogOptionMouseClickHandlerFunc));
 
 			if (!new_custom_render) {

engines/ags/engine/ac/display.cpp

@@ -283,8 +283,9 @@ ScreenOverlay *_display_main(int xx, int yy, int wii, const char *text, int disp
 
 			update_audio_system_on_game_loop();
 			render_graphics();
-			int mbut, mwheelz;
-			if (run_service_mb_controls(mbut, mwheelz) && mbut >= 0) {
+			eAGSMouseButton mbut;
+			int mwheelz;
+			if (run_service_mb_controls(mbut, mwheelz) && mbut > kMouseNone) {
 				check_skip_cutscene_mclick(mbut);
 				if (_GP(play).fast_forward)
 					break;

engines/ags/engine/ac/game.cpp

@@ -1081,7 +1081,7 @@ bool check_skip_cutscene_keypress(int kgn) {
 bool check_skip_cutscene_mclick(int mbut) {
 	CutsceneSkipStyle skip = get_cutscene_skipstyle();
 	if (skip == eSkipSceneMouse || skip == eSkipSceneKeyMouse ||
-	        (mbut == MouseRight && skip == eSkipSceneEscOrRMB)) {
+	        (mbut == kMouseRight && skip == eSkipSceneEscOrRMB)) {
 		start_skipping_cutscene();
 		return true;
 	}

engines/ags/engine/ac/inv_window.cpp

@@ -364,12 +364,13 @@ bool InventoryScreen::Run() {
 	if ((isonitem < 0) | (isonitem >= numitems) | (isonitem >= top_item + num_visible_items))
 		isonitem = -1;
 
-	int mclick, mwheelz;
-	if (!run_service_mb_controls(mclick, mwheelz) || _GP(play).IsIgnoringInput()) {
-		mclick = MouseNone;
+	eAGSMouseButton mbut;
+	int mwheelz;
+	if (!run_service_mb_controls(mbut, mwheelz) || _GP(play).IsIgnoringInput()) {
+		mbut = kMouseNone;
 	}
 
-	if (mclick == MouseLeft) {
+	if (mbut == kMouseLeft) {
 		if ((my < 0) | (my > windowhit) | (mx < 0) | (mx > windowwid))
 			return true; // continue inventory screen loop
 		if (my < buttonyp) {
@@ -455,7 +456,7 @@ bool InventoryScreen::Run() {
 			}
 			set_mouse_cursor(cmode);
 		}
-	} else if (mclick == MouseRight) {
+	} else if (mbut == kMouseRight) {
 		if (cmode == CURS_ARROW)
 			cmode = MODE_LOOK;
 		else

engines/ags/engine/ac/mouse.cpp

@@ -297,11 +297,9 @@ int GetCursorMode() {
 }
 
 int IsButtonDown(int which) {
-	if ((which < 1) || (which > 3))
+	if ((which < kMouseLeft) || (which > kMouseMiddle))
 		quit("!IsButtonDown: only works with eMouseLeft, eMouseRight, eMouseMiddle");
-	if (ags_misbuttondown(which - 1))
-		return 1;
-	return 0;
+	return ags_misbuttondown(static_cast<eAGSMouseButton>(which)) ? 1 : 0;
 }
 
 int IsModeEnabled(int which) {

engines/ags/engine/ac/sys_events.cpp

@@ -53,7 +53,9 @@ using namespace AGS::Shared;
 using namespace AGS::Engine;
 
 extern void domouse(int str);
-const int MB_ARRAY[3] = { MouseBitLeft, MouseBitRight, MouseBitMiddle };
+// Convert mouse button id to flags
+const int MouseButton2Bits[kNumMouseButtons] =
+	{ MouseBitLeft, MouseBitRight, MouseBitMiddle };
 static void(*_on_quit_callback)(void) = nullptr;
 static void(*_on_switchin_callback)(void) = nullptr;
 static void(*_on_switchout_callback)(void) = nullptr;
@@ -157,22 +159,19 @@ static void on_mouse_wheel(const Common::Event &event) {
 		_G(sys_mouse_z)--;
 }
 
-int mgetbutton() {
-	int toret = MouseNone;
-	int butis = mouse_button_poll();
+static eAGSMouseButton mgetbutton() {
+	const int butis = mouse_button_poll();
 
 	if ((butis > 0) & (_G(butwas) > 0))
-		return MouseNone;  // don't allow holding button down
+		return kMouseNone;  // don't allow holding button down
 
 	if (butis & MouseBitLeft)
-		toret = MouseLeft;
+		return kMouseLeft;
 	else if (butis & MouseBitRight)
-		toret = MouseRight;
+		return kMouseRight;
 	else if (butis & MouseBitMiddle)
-		toret = MouseMiddle;
-
-	_G(butwas) = butis;
-	return toret;
+		return kMouseMiddle;
+	return kMouseNone;
 
 	// TODO: presumably this was a hack for 1-button Mac mouse;
 	// is this still necessary?
@@ -184,23 +183,19 @@ int mgetbutton() {
 		toret = RIGHT;
 	}
 #endif
-	return 0;
 }
 
-bool ags_misbuttondown(int but) {
-	return (mouse_button_poll() & MB_ARRAY[but]) != 0;
+bool ags_misbuttondown(eAGSMouseButton but) {
+	return (mouse_button_poll() & MouseButton2Bits[but]) != 0;
 }
 
-int ags_mgetbutton() {
-	int result;
-
-	if (_G(pluginSimulatedClick) > MouseNone) {
-		result = _G(pluginSimulatedClick);
-		_G(pluginSimulatedClick) = MouseNone;
-	} else {
-		result = mgetbutton();
+eAGSMouseButton ags_mgetbutton() {
+	if (_G(pluginSimulatedClick) > kMouseNone) {
+		eAGSMouseButton mbut = _G(pluginSimulatedClick);
+		_G(pluginSimulatedClick) = kMouseNone;
+		return mbut;
 	}
-	return result;
+	return mgetbutton();;
 }
 
 void ags_mouse_get_relxy(int &x, int &y) {

engines/ags/engine/ac/sys_events.h

@@ -27,21 +27,6 @@
 
 namespace AGS3 {
 
-// AGS own mouse button codes
-// TODO: these were internal button codes, but AGS script uses different ones,
-// which start with Left=1, and make more sense (0 is easier to use as "no value").
-// Must research if there are any dependencies to these internal values, and if not,
-// then just replace these matching script ones!
-// UPD: even plugin API seem to match script codes and require remap to internals.
-// UPD: or use SDL constants in the engine, but make conversion more visible by using a function.
-enum eAGSMouseButton {
-	MouseNone = -1,
-	MouseLeft = 0,
-	MouseRight = 1,
-	MouseMiddle = 2
-};
-
-
 // Keyboard input handling
 //
 // avoid including SDL.h here, at least for now, because that leads to conflicts with allegro
@@ -85,9 +70,9 @@ extern void ags_simulate_keypress(eAGSKeyCode ags_key);
 // Mouse input handling
 //
 // Tells if the mouse button is currently down
-extern bool ags_misbuttondown(int but);
+extern bool ags_misbuttondown(eAGSMouseButton but);
 // Returns mouse button code
-extern int  ags_mgetbutton();
+extern eAGSMouseButton ags_mgetbutton();
 // Returns recent relative mouse movement
 extern void ags_mouse_get_relxy(int &x, int &y);
 // Updates mouse cursor position in game

engines/ags/engine/gui/csci_dialog.cpp

@@ -156,8 +156,9 @@ int CSCIWaitMessage(CSCIMessage *cscim) {
 			}
 		}
 
-		int mbut, mwheelz;
-		if (run_service_mb_controls(mbut, mwheelz) && mbut >= 0 && !_GP(play).IsIgnoringInput()) {
+		eAGSMouseButton mbut;
+		int mwheelz;
+		if (run_service_mb_controls(mbut, mwheelz) && (mbut > kMouseNone) && !_GP(play).IsIgnoringInput()) {
 			if (checkcontrols()) {
 				cscim->id = _G(controlid);
 				cscim->code = CM_COMMAND;

engines/ags/engine/gui/gui_dialog_internal_defs.h

@@ -30,7 +30,6 @@
 #undef WINAPI
 #endif
 #define WINAPI
-#define mbutrelease(X) (!ags_misbuttondown(X))
 #define TEXT_HT _GP(usetup).textheight
 
 #endif

engines/ags/engine/gui/my_push_button.cpp

@@ -72,7 +72,7 @@ void MyPushButton::draw(Bitmap *ds) {
 
 int MyPushButton::pressedon(int mx, int my) {
 	int wasstat;
-	while (mbutrelease(MouseLeft) == 0) {
+	while (!ags_misbuttondown(kMouseLeft) == 0) {
 
 		wasstat = state;
 		state = mouseisinarea(mx, my);

engines/ags/engine/main/game_run.cpp

@@ -192,16 +192,16 @@ static void check_mouse_controls() {
 		remove_popup_interface(_G(ifacepopped));
 
 	// check mouse clicks on GUIs
-	if ((_G(wasbutdown) > 0) && (ags_misbuttondown(_G(wasbutdown) - 1))) {
+	if ((_G(wasbutdown) > kMouseNone) && (ags_misbuttondown(_G(wasbutdown)))) {
 		gui_on_mouse_hold(_G(wasongui), _G(wasbutdown));
-	} else if ((_G(wasbutdown) > 0) && (!ags_misbuttondown(_G(wasbutdown) - 1))) {
+	} else if ((_G(wasbutdown) > kMouseNone) && (!ags_misbuttondown(_G(wasbutdown)))) {
 		gui_on_mouse_up(_G(wasongui), _G(wasbutdown));
-		_G(wasbutdown) = 0;
+		_G(wasbutdown) = kMouseNone;
 	}
 
-	int mbut = MouseNone;
-	int mwheelz = 0;
-	if (run_service_mb_controls(mbut, mwheelz) && mbut >= 0) {
+	eAGSMouseButton mbut;
+	int mwheelz;
+	if (run_service_mb_controls(mbut, mwheelz) && mbut > kMouseNone) {
 
 		check_skip_cutscene_mclick(mbut);
 
@@ -214,17 +214,17 @@ static void check_mouse_controls() {
 				_GP(play).SetWaitSkipResult(SKIP_MOUSECLICK, mbut);
 			}
 		} else if (!IsInterfaceEnabled());  // blocking cutscene, ignore mouse
-		else if (pl_run_plugin_hooks(AGSE_MOUSECLICK, mbut + 1)) {
+		else if (pl_run_plugin_hooks(AGSE_MOUSECLICK, mbut)) {
 			// plugin took the click
-			debug_script_log("Plugin handled mouse button %d", mbut + 1);
+			debug_script_log("Plugin handled mouse button %d", mbut);
 		} else if (mongu >= 0) {
-			if (_G(wasbutdown) == 0) {
-				gui_on_mouse_down(mongu, mbut + 1);
+			if (_G(wasbutdown) == kMouseNone) {
+				gui_on_mouse_down(mongu, mbut);
 			}
 			_G(wasongui) = mongu;
-			_G(wasbutdown) = mbut + 1;
+			_G(wasbutdown) = mbut;
 		} else
-			setevent(EV_TEXTSCRIPT, TS_MCLICK, mbut + 1);
+			setevent(EV_TEXTSCRIPT, TS_MCLICK, mbut);
 	}
 
 	if (mwheelz < 0)
@@ -256,6 +256,8 @@ bool run_service_key_controls(KeyInput &out_key) {
 	const bool is_only_mod_key = key_evt.type == Common::EVENT_KEYDOWN ?
 		is_mod_key(key_evt.kbd.keycode) : false;
 
+	out_key = KeyInput(); // reset to default
+
 	// Following section is for testing for pushed and released mod-keys.
 	// A bit of explanation: some service actions may require combination of
 	// mod-keys, for example [Ctrl + Alt] toggles mouse lock in window.
@@ -398,14 +400,12 @@ bool run_service_key_controls(KeyInput &out_key) {
 	return true;
 }
 
-bool run_service_mb_controls(int &mbut, int &mwheelz) {
-	int mb = ags_mgetbutton();
-	int mz = ags_check_mouse_wheel();
-	if (mb == MouseNone && mz == 0)
+bool run_service_mb_controls(eAGSMouseButton &mbut, int &mwheelz) {
+	mbut = ags_mgetbutton();
+	mwheelz = ags_check_mouse_wheel();
+	if (mbut == kMouseNone && mwheelz == 0)
 		return false;
-	lock_mouse_on_click(); // do not claim
-	mbut = mb;
-	mwheelz = mz;
+	lock_mouse_on_click();
 	return true;
 }
 

engines/ags/engine/main/game_run.h

@@ -22,6 +22,8 @@
 #ifndef AGS_ENGINE_MAIN_GAME_RUN_H
 #define AGS_ENGINE_MAIN_GAME_RUN_H
 
+#include "ags/shared/ac/keycode.h"
+
 namespace AGS3 {
 
 namespace AGS {
@@ -51,13 +53,12 @@ void UpdateGameAudioOnly();
 // Gets current logical game FPS, this is normally a fixed number set in script;
 // in case of "maxed fps" mode this function returns real measured FPS.
 float get_current_fps();
-struct KeyInput;
 // Runs service key controls, returns false if no key was pressed or key input was claimed by the engine,
 // otherwise returns true and provides a keycode.
 bool run_service_key_controls(KeyInput &kgn);
 // Runs service mouse controls, returns false if mouse input was claimed by the engine,
 // otherwise returns true and provides mouse button code.
-bool run_service_mb_controls(int &mbut, int &mwheelz);
+bool run_service_mb_controls(eAGSMouseButton &mbut, int &mwheelz);
 
 } // namespace AGS3
 

engines/ags/engine/media/video/video.cpp

@@ -111,7 +111,8 @@ static bool play_video(Video::VideoDecoder *decoder, const char *name, int flags
 		if (skip != VideoSkipNone) {
 			// Check for whether user aborted video
 			KeyInput key;
-			int mbut, mwheelz;
+			eAGSMouseButton mbut;
+			int mwheelz;
 			if (run_service_key_controls(key)) {
 				if (key.Key == 27 && skip >= VideoSkipEscape)
 					return true;
@@ -119,7 +120,7 @@ static bool play_video(Video::VideoDecoder *decoder, const char *name, int flags
 					return true;  // skip on any key
 			}
 
-			if (run_service_mb_controls(mbut, mwheelz) && mbut >= 0 && skip == VideoSkipKeyOrMouse) {
+			if (run_service_mb_controls(mbut, mwheelz) && mbut >= kMouseNone && skip == VideoSkipKeyOrMouse) {
 				return true; // skip on mouse click
 			}
 		}

engines/ags/globals.h

@@ -27,6 +27,7 @@
 
 #include "ags/lib/std/queue.h"
 #include "ags/shared/ac/game_version.h"
+#include "ags/shared/ac/keycode.h"
 #include "ags/shared/util/stdio_compat.h"
 #include "ags/shared/util/string.h"
 #include "ags/shared/util/string_types.h"
@@ -242,7 +243,8 @@ public:
 	int _mouse_accum_button_state = 0;
 	uint32 _mouse_clear_at_time = 0;
 	int _mouse_accum_relx = 0, _mouse_accum_rely = 0;
-	int _wasbutdown = 0, _wasongui = 0;
+	eAGSMouseButton _wasbutdown = kMouseNone;
+	int _wasongui = 0;
 
 	/**@}*/
 
@@ -1324,7 +1326,7 @@ public:
 	long _pl_file_handle = -1;
 	AGS::Shared::Stream *_pl_file_stream = nullptr;
 
-	int _pluginSimulatedClick = -1;
+	eAGSMouseButton _pluginSimulatedClick = kMouseNone;
 	int _mouse_z_was = 0;
 
 	/**@}*/

engines/ags/plugins/ags_plugin.cpp

@@ -86,7 +86,7 @@ const int PLUGIN_API_VERSION = 25;
 // we can reuse the same handle.
 
 void PluginSimulateMouseClick(int pluginButtonID) {
-	_G(pluginSimulatedClick) = pluginButtonID - 1;
+	_G(pluginSimulatedClick) = static_cast<eAGSMouseButton>(pluginButtonID);
 }
 
 void IAGSEngine::AbortGame(const char *reason) {
@@ -342,8 +342,9 @@ void IAGSEngine::BlitSpriteRotated(int32 x, int32 y, BITMAP *bmp, int32 angle) {
 void IAGSEngine::PollSystem() {
 	ags_domouse();
 	update_polled_stuff_if_runtime();
-	int mbut, mwheelz;
-	if (run_service_mb_controls(mbut, mwheelz) && mbut >= 0 && !_GP(play).IsIgnoringInput())
+	eAGSMouseButton mbut;
+	int mwheelz;
+	if (run_service_mb_controls(mbut, mwheelz) && mbut > kMouseNone && !_GP(play).IsIgnoringInput())
 		pl_run_plugin_hooks(AGSE_MOUSECLICK, mbut);
 	KeyInput kp;
 	if (run_service_key_controls(kp) && !_GP(play).IsIgnoringInput()) {

engines/ags/shared/ac/keycode.h

@@ -276,6 +276,17 @@ struct KeyInput {
 	KeyInput() = default;
 };
 
+// AGS own mouse button codes;
+// These correspond to MouseButton enum in script API (sans special values)
+enum eAGSMouseButton
+{
+	kMouseNone = 0,
+	kMouseLeft = 1,
+	kMouseRight = 2,
+	kMouseMiddle = 3,
+	kNumMouseButtons
+};
+
 // Converts eAGSKeyCode to script API code, for "on_key_press" and similar callbacks
 int AGSKeyToScriptKey(int keycode);
 // Converts eAGSKeyCode to ASCII text representation with the range check; returns 0 on failure