TouchControls: Implement an option for dig/place buttons (#15845)

src/gui/touchcontrols.cpp

@@ -15,6 +15,7 @@
 #include "client/guiscalingfilter.h"
 #include "client/renderingengine.h"
 #include "client/texturesource.h"
+#include "util/enum_string.h"
 #include "util/numeric.h"
 #include "irr_gui_ptr.h"
 #include "IGUIImage.h"
@@ -78,15 +79,18 @@ bool TouchControls::buttonsHandlePress(std::vector<button_info> &buttons, size_t
 
 	for (button_info &btn : buttons) {
 		if (btn.gui_button.get() == element) {
+			// Allow moving the camera with the same finger that holds dig/place.
+			bool absorb = btn.id != dig_id && btn.id != place_id;
+
 			assert(std::find(btn.pointer_ids.begin(), btn.pointer_ids.end(), pointer_id) == btn.pointer_ids.end());
 			btn.pointer_ids.push_back(pointer_id);
 
 			if (btn.pointer_ids.size() > 1)
-				return true;
+				return absorb;
 
 			buttonEmitAction(btn, true);
 			btn.repeat_counter = -BUTTON_REPEAT_DELAY;
-			return true;
+			return absorb;
 		}
 	}
 
@@ -99,13 +103,16 @@ bool TouchControls::buttonsHandleRelease(std::vector<button_info> &buttons, size
 	for (button_info &btn : buttons) {
 		auto it = std::find(btn.pointer_ids.begin(), btn.pointer_ids.end(), pointer_id);
 		if (it != btn.pointer_ids.end()) {
+			// Don't absorb since we didn't absorb the press event either.
+			bool absorb = btn.id != dig_id && btn.id != place_id;
+
 			btn.pointer_ids.erase(it);
 
 			if (!btn.pointer_ids.empty())
-				return true;
+				return absorb;
 
 			buttonEmitAction(btn, false);
-			return true;
+			return absorb;
 		}
 	}
 
@@ -117,6 +124,8 @@ bool TouchControls::buttonsStep(std::vector<button_info> &buttons, float dtime)
 	bool has_pointers = false;
 
 	for (button_info &btn : buttons) {
+		if (btn.id == dig_id || btn.id == place_id)
+			continue; // key repeats would cause glitches here
 		if (btn.pointer_ids.empty())
 			continue;
 		has_pointers = true;
@@ -205,7 +214,7 @@ static const KeyPress &id_to_keypress(touch_gui_button_id id)
 
 
 static const char *setting_names[] = {
-	"touch_use_crosshair",
+	"touch_interaction_style",
 	"touchscreen_threshold", "touch_long_tap_delay",
 	"fixed_virtual_joystick", "virtual_joystick_triggers_aux1",
 	"touch_layout",
@@ -232,14 +241,20 @@ void TouchControls::settingChangedCallback(const std::string &name, void *data)
 
 void TouchControls::readSettings()
 {
-	m_use_crosshair = g_settings->getBool("touch_use_crosshair");
+	const std::string &s = g_settings->get("touch_interaction_style");
+	if (!string_to_enum(es_TouchInteractionStyle, m_interaction_style, s)) {
+		m_interaction_style = TAP;
+		warningstream << "Invalid touch_interaction_style value" << std::endl;
+	}
+
 	m_touchscreen_threshold = g_settings->getU16("touchscreen_threshold");
 	m_long_tap_delay = g_settings->getU16("touch_long_tap_delay");
 	m_fixed_joystick = g_settings->getBool("fixed_virtual_joystick");
 	m_joystick_triggers_aux1 = g_settings->getBool("virtual_joystick_triggers_aux1");
 
-	// Note that "fixed_virtual_joystick" and "virtual_joystick_triggers_aux1"
-	// also affect the layout.
+	// Note that other settings also affect the layout:
+	// - ButtonLayout::loadFromSettings: "touch_interaction_style" and "virtual_joystick_triggers_aux1"
+	// - applyLayout: "fixed_virtual_joystick"
 	applyLayout(ButtonLayout::loadFromSettings());
 }
 
@@ -305,8 +320,8 @@ void TouchControls::applyLayout(const ButtonLayout &layout)
 	overflow_buttons.erase(std::remove_if(
 			overflow_buttons.begin(), overflow_buttons.end(),
 			[&](touch_gui_button_id id) {
-				// There's no sense in adding the overflow button to the overflow
-				// menu (also, it's impossible since it doesn't have a keycode).
+				// There would be no sense in adding the overflow button to the
+				// overflow menu.
 				return !mayAddButton(id) || id == overflow_id;
 			}), overflow_buttons.end());
 
@@ -346,13 +361,10 @@ TouchControls::~TouchControls()
 
 bool TouchControls::mayAddButton(touch_gui_button_id id)
 {
-	if (!ButtonLayout::isButtonAllowed(id))
-		return false;
-	if (id == aux1_id && m_joystick_triggers_aux1)
-		return false;
-	if (id != overflow_id && !id_to_keypress(id))
-		return false;
-	return true;
+	assert(ButtonLayout::isButtonValid(id));
+	assert(ButtonLayout::isButtonAllowed(id));
+	// The overflow button doesn't need a keycode to be valid.
+	return id == overflow_id || id_to_keypress(id);
 }
 
 void TouchControls::addButton(std::vector<button_info> &buttons, touch_gui_button_id id,
@@ -363,6 +375,7 @@ void TouchControls::addButton(std::vector<button_info> &buttons, touch_gui_butto
 	loadButtonTexture(btn_gui_button, image);
 
 	button_info &btn = buttons.emplace_back();
+	btn.id = id;
 	btn.keypress = id_to_keypress(id);
 	btn.gui_button = grab_gui_element<IGUIImage>(btn_gui_button);
 }
@@ -429,7 +442,8 @@ void TouchControls::handleReleaseEvent(size_t pointer_id)
 		// If m_tap_state is already set to TapState::ShortTap, we must keep
 		// that value. Otherwise, many short taps will be ignored if you tap
 		// very fast.
-		if (!m_move_has_really_moved && !m_move_prevent_short_tap &&
+		if (m_interaction_style != BUTTONS_CROSSHAIR &&
+				!m_move_has_really_moved && !m_move_prevent_short_tap &&
 				m_tap_state != TapState::LongTap) {
 			m_tap_state = TapState::ShortTap;
 		} else {
@@ -655,7 +669,9 @@ void TouchControls::step(float dtime)
 	applyJoystickStatus();
 
 	// if a new placed pointer isn't moved for some time start digging
-	if (m_has_move_id && !m_move_has_really_moved && m_tap_state == TapState::None) {
+	if (m_interaction_style != BUTTONS_CROSSHAIR &&
+			m_has_move_id && !m_move_has_really_moved &&
+			m_tap_state == TapState::None) {
 		u64 delta = porting::getDeltaMs(m_move_downtime, porting::getTimeMs());
 
 		if (delta > m_long_tap_delay) {
@@ -669,7 +685,7 @@ void TouchControls::step(float dtime)
 	// shootline when a touch event occurs.
 	// Only updating when m_has_move_id means that the shootline will stay at
 	// it's last in-world position when the player doesn't need it.
-	if (!m_use_crosshair && (m_has_move_id || m_had_move_id)) {
+	if (m_interaction_style == TAP && (m_has_move_id || m_had_move_id)) {
 		m_shootline = m_device
 				->getSceneManager()
 				->getSceneCollisionManager()
@@ -757,6 +773,9 @@ void TouchControls::show()
 
 void TouchControls::applyContextControls(const TouchInteractionMode &mode)
 {
+	if (m_interaction_style == BUTTONS_CROSSHAIR)
+		return;
+
 	// Since the pointed thing has already been determined when this function
 	// is called, we cannot use this function to update the shootline.
 

src/gui/touchcontrols.h

@@ -57,6 +57,7 @@ enum class TapState
 
 struct button_info
 {
+	touch_gui_button_id id;
 	float repeat_counter;
 	KeyPress keypress;
 	std::vector<size_t> pointer_ids;
@@ -94,7 +95,7 @@ public:
 		return res;
 	}
 
-	bool isShootlineAvailable() { return !m_use_crosshair; }
+	bool isShootlineAvailable() { return m_interaction_style == TAP; }
 
 	/**
 	 * Returns a line which describes what the player is pointing at.
@@ -137,7 +138,7 @@ private:
 	s32 m_button_size;
 
 	// cached settings
-	bool m_use_crosshair;
+	TouchInteractionStyle m_interaction_style;
 	double m_touchscreen_threshold;
 	u16 m_long_tap_delay;
 	bool m_fixed_joystick;
@@ -161,7 +162,7 @@ private:
 	 * The line ends on the camera's far plane.
 	 * The coordinates do not contain the camera offset.
 	 *
-	 * Only valid if !m_use_crosshair
+	 * Only used for m_interaction_style == TAP
 	 */
 	line3d<f32> m_shootline;
 
@@ -243,6 +244,8 @@ private:
 	// map to store the IDs and positions of currently pressed pointers
 	std::unordered_map<size_t, v2s32> m_pointer_pos;
 
+	// The following are not used if m_interaction_style == BUTTONS_CROSSHAIR
+
 	TouchInteractionMode m_last_mode = TouchInteractionMode_END;
 	TapState m_tap_state = TapState::None;
 

src/gui/touchscreeneditor.cpp

@@ -371,7 +371,7 @@ bool GUITouchscreenLayout::OnEvent(const SEvent& event)
 			}
 
 			if (event.GUIEvent.Caller == m_gui_reset_btn.get()) {
-				m_layout = ButtonLayout::predefined;
+				m_layout = ButtonLayout::loadDefault();
 				regenerateGui(screensize);
 				return true;
 			}

src/gui/touchscreenlayout.cpp

@@ -12,6 +12,15 @@
 
 #include "IGUIFont.h"
 #include "IGUIStaticText.h"
+#include "util/enum_string.h"
+
+const struct EnumString es_TouchInteractionStyle[] =
+{
+	{TAP, "tap"},
+	{TAP_CROSSHAIR, "tap_crosshair"},
+	{BUTTONS_CROSSHAIR, "buttons_crosshair"},
+	{0, NULL},
+};
 
 const char *button_names[] = {
 	"dig",
@@ -73,8 +82,8 @@ const char *button_titles[] = {
 };
 
 const char *button_image_names[] = {
-	"",
-	"",
+	"dig_btn.png",
+	"place_btn.png",
 
 	"jump_btn.png",
 	"down.png",
@@ -130,11 +139,21 @@ void ButtonMeta::setPos(v2s32 pos, v2u32 screensize, s32 button_size)
 	offset.Y = (pos.Y - (position.Y * screensize.Y)) / button_size;
 }
 
+bool ButtonLayout::isButtonValid(touch_gui_button_id id)
+{
+	return id != joystick_off_id && id != joystick_bg_id && id != joystick_center_id &&
+			id < touch_gui_button_id_END;
+}
+
+static const char *buttons_crosshair = enum_to_string(es_TouchInteractionStyle, BUTTONS_CROSSHAIR);
+
 bool ButtonLayout::isButtonAllowed(touch_gui_button_id id)
 {
-	return id != dig_id && id != place_id &&
-			id != joystick_off_id && id != joystick_bg_id && id != joystick_center_id &&
-			id != touch_gui_button_id_END;
+	if (id == dig_id || id == place_id)
+		return g_settings->get("touch_interaction_style") == buttons_crosshair;
+	if (id == aux1_id)
+		return !g_settings->getBool("virtual_joystick_triggers_aux1");
+	return true;
 }
 
 bool ButtonLayout::isButtonRequired(touch_gui_button_id id)
@@ -149,7 +168,15 @@ s32 ButtonLayout::getButtonSize(v2u32 screensize)
 					g_settings->getFloat("hud_scaling"));
 }
 
-const ButtonLayout ButtonLayout::predefined {{
+const ButtonLayout::ButtonMap ButtonLayout::default_data {
+	{dig_id, {
+		v2f(1.0f, 1.0f),
+		v2f(-2.0f, -2.75f),
+	}},
+	{place_id, {
+		v2f(1.0f, 1.0f),
+		v2f(-2.0f, -4.25f),
+	}},
 	{jump_id, {
 		v2f(1.0f, 1.0f),
 		v2f(-1.0f, -0.5f),
@@ -170,28 +197,40 @@ const ButtonLayout ButtonLayout::predefined {{
 		v2f(1.0f, 1.0f),
 		v2f(-0.75f, -5.0f),
 	}},
-}};
+};
+
+ButtonLayout ButtonLayout::postProcessLoaded(const ButtonMap &data)
+{
+	ButtonLayout layout;
+	for (const auto &[id, meta] : data) {
+		assert(isButtonValid(id));
+		if (isButtonAllowed(id))
+			layout.layout.emplace(id, meta);
+		else
+			layout.preserved_disallowed.emplace(id, meta);
+	}
+	return layout;
+}
+
+ButtonLayout ButtonLayout::loadDefault()
+{
+	return postProcessLoaded(default_data);
+}
 
 ButtonLayout ButtonLayout::loadFromSettings()
 {
-	bool restored = false;
-	ButtonLayout layout;
-
 	std::string str = g_settings->get("touch_layout");
 	if (!str.empty()) {
 		std::istringstream iss(str);
 		try {
-			layout.deserializeJson(iss);
-			restored = true;
+			ButtonMap data = deserializeJson(iss);
+			return postProcessLoaded(data);
 		} catch (const Json::Exception &e) {
 			warningstream << "Could not parse touchscreen layout: " << e.what() << std::endl;
 		}
 	}
 
-	if (!restored)
-		return predefined;
-
-	return layout;
+	return loadDefault();
 }
 
 std::unordered_map<touch_gui_button_id, irr_ptr<video::ITexture>> ButtonLayout::texture_cache;
@@ -234,7 +273,7 @@ std::vector<touch_gui_button_id> ButtonLayout::getMissingButtons()
 	std::vector<touch_gui_button_id> missing_buttons;
 	for (u8 i = 0; i < touch_gui_button_id_END; i++) {
 		touch_gui_button_id btn = (touch_gui_button_id)i;
-		if (isButtonAllowed(btn) && layout.count(btn) == 0)
+		if (isButtonValid(btn) && isButtonAllowed(btn) && layout.count(btn) == 0)
 			missing_buttons.push_back(btn);
 	}
 	return missing_buttons;
@@ -243,16 +282,19 @@ std::vector<touch_gui_button_id> ButtonLayout::getMissingButtons()
 void ButtonLayout::serializeJson(std::ostream &os) const
 {
 	Json::Value root = Json::objectValue;
+	root["version"] = 1;
 	root["layout"] = Json::objectValue;
 
-	for (const auto &[id, meta] : layout) {
-		Json::Value button = Json::objectValue;
-		button["position_x"] = meta.position.X;
-		button["position_y"] = meta.position.Y;
-		button["offset_x"] = meta.offset.X;
-		button["offset_y"] = meta.offset.Y;
+	for (const auto &list : {layout, preserved_disallowed}) {
+		for (const auto &[id, meta] : list) {
+			Json::Value button = Json::objectValue;
+			button["position_x"] = meta.position.X;
+			button["position_y"] = meta.position.Y;
+			button["offset_x"] = meta.offset.X;
+			button["offset_y"] = meta.offset.Y;
 
-		root["layout"][button_names[id]] = button;
+			root["layout"][button_names[id]] = button;
+		}
 	}
 
 	fastWriteJson(root, os);
@@ -267,13 +309,19 @@ static touch_gui_button_id button_name_to_id(const std::string &name)
 	return touch_gui_button_id_END;
 }
 
-void ButtonLayout::deserializeJson(std::istream &is)
+ButtonLayout::ButtonMap ButtonLayout::deserializeJson(std::istream &is)
 {
-	layout.clear();
+	ButtonMap data;
 
 	Json::Value root;
 	is >> root;
 
+	u8 version;
+	if (root["version"].isUInt())
+		version = root["version"].asUInt();
+	else
+		version = 0;
+
 	if (!root["layout"].isObject())
 		throw Json::RuntimeError("invalid type for layout");
 
@@ -281,7 +329,7 @@ void ButtonLayout::deserializeJson(std::istream &is)
 	Json::ValueIterator iter;
 	for (iter = obj.begin(); iter != obj.end(); iter++) {
 		touch_gui_button_id id = button_name_to_id(iter.name());
-		if (!isButtonAllowed(id))
+		if (!isButtonValid(id))
 			throw Json::RuntimeError("invalid button name");
 
 		Json::Value &value = *iter;
@@ -300,8 +348,20 @@ void ButtonLayout::deserializeJson(std::istream &is)
 		meta.offset.X = value["offset_x"].asFloat();
 		meta.offset.Y = value["offset_y"].asFloat();
 
-		layout.emplace(id, meta);
+		data.emplace(id, meta);
 	}
+
+	if (version < 1) {
+		// Version 0 did not have dig/place buttons, so add them in.
+		// Otherwise, the missing buttons would cause confusion if the user
+		// switches to "touch_interaction_style = buttons_crosshair".
+		// This may result in overlapping buttons (could be fixed by resolving
+		// collisions in postProcessLoaded).
+		data.emplace(dig_id, default_data.at(dig_id));
+		data.emplace(place_id, default_data.at(place_id));
+	}
+
+	return data;
 }
 
 void layout_button_grid(v2u32 screensize, ISimpleTextureSource *tsrc,

src/gui/touchscreenlayout.h

@@ -7,6 +7,7 @@
 #include "irr_ptr.h"
 #include "irrlichttypes_bloated.h"
 #include "rect.h"
+#include "util/enum_string.h"
 #include <iostream>
 #include <unordered_map>
 
@@ -20,9 +21,16 @@ namespace irr::video
 	class ITexture;
 }
 
+enum TouchInteractionStyle : u8
+{
+	TAP,
+	TAP_CROSSHAIR,
+	BUTTONS_CROSSHAIR,
+};
+extern const struct EnumString es_TouchInteractionStyle[];
+
 enum touch_gui_button_id : u8
 {
-	// these two are no actual buttons ... yet
 	dig_id = 0,
 	place_id,
 
@@ -75,15 +83,34 @@ struct ButtonMeta {
 };
 
 struct ButtonLayout {
+	using ButtonMap = std::unordered_map<touch_gui_button_id, ButtonMeta>;
+
+	static bool isButtonValid(touch_gui_button_id id);
 	static bool isButtonAllowed(touch_gui_button_id id);
 	static bool isButtonRequired(touch_gui_button_id id);
 	static s32 getButtonSize(v2u32 screensize);
+
+	// Returns the default layout.
+	// Note: Indirectly depends on settings.
+	static ButtonLayout loadDefault();
+	// Reads the layout from the "touch_layout" setting. Falls back to loadDefault
+	// if the setting is absent or invalid.
+	// Note: Indirectly depends on additional settings.
 	static ButtonLayout loadFromSettings();
 
 	static video::ITexture *getTexture(touch_gui_button_id btn, ISimpleTextureSource *tsrc);
 	static void clearTextureCache();
 
-	std::unordered_map<touch_gui_button_id, ButtonMeta> layout;
+	ButtonMap layout;
+
+	// Contains disallowed buttons that have been loaded.
+	// These are only preserved to be saved again later.
+	// This exists to prevent data loss: If you edit the layout while some button
+	// is temporarily disallowed, this prevents that button's custom position
+	// from being lost. See isButtonAllowed.
+	// This may result in overlapping buttons when the buttons are allowed again
+	// (could be fixed by resolving collisions in postProcessLoaded).
+	ButtonMap preserved_disallowed;
 
 	core::recti getRect(touch_gui_button_id btn,
 			v2u32 screensize, s32 button_size, ISimpleTextureSource *tsrc);
@@ -91,11 +118,12 @@ struct ButtonLayout {
 	std::vector<touch_gui_button_id> getMissingButtons();
 
 	void serializeJson(std::ostream &os) const;
-	void deserializeJson(std::istream &is);
-
-	static const ButtonLayout predefined;
 
 private:
+	static const ButtonMap default_data;
+	static ButtonMap deserializeJson(std::istream &is);
+	static ButtonLayout postProcessLoaded(const ButtonMap &map);
+
 	static std::unordered_map<touch_gui_button_id, irr_ptr<video::ITexture>> texture_cache;
 };