Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[no squash] Auto-toggle TouchScreenGUI in-game when receiving touch/mouse input #14542

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

[no squash] Auto-toggle TouchScreenGUI in-game when receiving touch/mouse input #14542

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
266137a
Rename TouchScreenGUI -> TouchControls
grorp May 15, 2024
e467fa5
split enable_touch to touch_controls (for touchscreen controls) and t…
grorp Jun 2, 2024
71c05c6
Auto-toggle TouchControls in-game when receiving touch/mouse input (c…
grorp Apr 13, 2024
348d003
Fix Android camera jump issue (phew)
grorp May 5, 2024
c6978db
Avoid bug on Linux/X11
grorp May 15, 2024
f74fff3
Manually set `Simulated = false` where necessary (fixup)
grorp Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion builtin/fstk/buttonbar.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

local BASE_SPACING = 0.1
local function get_scroll_btn_width()
return core.settings:get_bool("enable_touch") and 0.8 or 0.5
return core.settings:get_bool("touch_gui") and 0.8 or 0.5
end

local function buttonbar_formspec(self)
Expand Down
6 changes: 3 additions & 3 deletions builtin/mainmenu/content/dlg_contentdb.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ local function get_info_formspec(text)
return table.concat({
"formspec_version[6]",
"size[15.75,9.5]",
core.settings:get_bool("enable_touch") and "padding[0.01,0.01]" or "position[0.5,0.55]",
core.settings:get_bool("touch_gui") and "padding[0.01,0.01]" or "position[0.5,0.55]",

"label[4,4.35;", text, "]",
"container[0,", H - 0.8 - 0.375, "]",
Expand Down Expand Up @@ -221,7 +221,7 @@ local function get_formspec(dlgdata)
local formspec = {
"formspec_version[6]",
"size[15.75,9.5]",
core.settings:get_bool("enable_touch") and "padding[0.01,0.01]" or "position[0.5,0.55]",
core.settings:get_bool("touch_gui") and "padding[0.01,0.01]" or "position[0.5,0.55]",

"style[status,downloading,queued;border=false]",

Expand Down Expand Up @@ -472,7 +472,7 @@ end
local function handle_events(event)
if event == "DialogShow" then
-- On touchscreen, don't show the "MINETEST" header behind the dialog.
mm_game_theme.set_engine(core.settings:get_bool("enable_touch"))
mm_game_theme.set_engine(core.settings:get_bool("touch_gui"))

-- If ContentDB is already loaded, auto-install packages here.
do_auto_install()
Expand Down
2 changes: 1 addition & 1 deletion builtin/mainmenu/content/dlg_install.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ local function get_formspec(data)
message_bg = mt_color_orange
end

local ENABLE_TOUCH = core.settings:get_bool("enable_touch")
local ENABLE_TOUCH = core.settings:get_bool("touch_gui")

local w = ENABLE_TOUCH and 14 or 7
local padded_w = w - 2*0.375
Expand Down
75 changes: 38 additions & 37 deletions builtin/mainmenu/settings/dlg_settings.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ end
local change_keys = {
query_text = "Controls",
requires = {
keyboard_mouse = true,
touch_controls = false,
},
get_formspec = function(self, avail_w)
local btn_w = math.min(avail_w, 3)
Expand Down Expand Up @@ -313,11 +313,14 @@ local function check_requirements(name, requires)

local video_driver = core.get_active_driver()
local shaders_support = video_driver == "opengl" or video_driver == "opengl3" or video_driver == "ogles2"
local touch_controls = core.settings:get("touch_controls")
local special = {
android = PLATFORM == "Android",
desktop = PLATFORM ~= "Android",
touchscreen_gui = core.settings:get_bool("enable_touch"),
keyboard_mouse = not core.settings:get_bool("enable_touch"),
-- When touch_controls is "auto", we don't which input method will be used,
-- so we show settings for both.
touchscreen = touch_controls == "auto" or core.is_yes(touch_controls),
keyboard_mouse = touch_controls == "auto" or not core.is_yes(touch_controls),
shaders_support = shaders_support,
shaders = core.settings:get_bool("enable_shaders") and shaders_support,
opengl = video_driver == "opengl",
Expand Down Expand Up @@ -449,13 +452,13 @@ local function get_formspec(dialogdata)

local extra_h = 1 -- not included in tabsize.height
local tabsize = {
width = core.settings:get_bool("enable_touch") and 16.5 or 15.5,
height = core.settings:get_bool("enable_touch") and (10 - extra_h) or 12,
width = core.settings:get_bool("touch_gui") and 16.5 or 15.5,
height = core.settings:get_bool("touch_gui") and (10 - extra_h) or 12,
}

local scrollbar_w = core.settings:get_bool("enable_touch") and 0.6 or 0.4
local scrollbar_w = core.settings:get_bool("touch_gui") and 0.6 or 0.4

local left_pane_width = core.settings:get_bool("enable_touch") and 4.5 or 4.25
local left_pane_width = core.settings:get_bool("touch_gui") and 4.5 or 4.25
local left_pane_padding = 0.25
local search_width = left_pane_width + scrollbar_w - (0.75 * 2)

Expand All @@ -469,7 +472,7 @@ local function get_formspec(dialogdata)
local fs = {
"formspec_version[6]",
"size[", tostring(tabsize.width), ",", tostring(tabsize.height + extra_h), "]",
core.settings:get_bool("enable_touch") and "padding[0.01,0.01]" or "",
core.settings:get_bool("touch_gui") and "padding[0.01,0.01]" or "",
"bgcolor[#0000]",

-- HACK: this is needed to allow resubmitting the same formspec
Expand Down Expand Up @@ -618,6 +621,18 @@ function write_settings_early()
end
end

local function regenerate_page_list(dialogdata)
local suggested_page_id = update_filtered_pages(dialogdata.query)

dialogdata.components = nil

if not filtered_page_by_id[dialogdata.page_id] then
dialogdata.leftscroll = 0
dialogdata.rightscroll = 0

dialogdata.page_id = suggested_page_id
end
end

local function buttonhandler(this, fields)
local dialogdata = this.data
Expand All @@ -642,27 +657,7 @@ local function buttonhandler(this, fields)
local value = core.is_yes(fields.show_advanced)
core.settings:set_bool("show_advanced", value)
write_settings_early()
end

-- enable_touch is a checkbox in a setting component. We handle this
-- setting differently so we can hide/show pages using the next if-statement
if fields.enable_touch ~= nil then
local value = core.is_yes(fields.enable_touch)
core.settings:set_bool("enable_touch", value)
write_settings_early()
end

if fields.show_advanced ~= nil or fields.enable_touch ~= nil then
local suggested_page_id = update_filtered_pages(dialogdata.query)

dialogdata.components = nil

if not filtered_page_by_id[dialogdata.page_id] then
dialogdata.leftscroll = 0
dialogdata.rightscroll = 0

dialogdata.page_id = suggested_page_id
end
regenerate_page_list(dialogdata)

return true
end
Expand Down Expand Up @@ -695,20 +690,26 @@ local function buttonhandler(this, fields)
end
end

for i, comp in ipairs(dialogdata.components) do
if comp.on_submit and comp:on_submit(fields, this) then
write_settings_early()

local function after_setting_change(comp)
write_settings_early()
if comp.setting.name == "touch_controls" then
-- Changing the "touch_controls" setting may result in a different
-- page list.
regenerate_page_list(dialogdata)
else
-- Clear components so they regenerate
dialogdata.components = nil
end
end

for i, comp in ipairs(dialogdata.components) do
if comp.on_submit and comp:on_submit(fields, this) then
after_setting_change(comp)
return true
end
if comp.setting and fields["reset_" .. i] then
core.settings:remove(comp.setting.name)
write_settings_early()

-- Clear components so they regenerate
dialogdata.components = nil
after_setting_change(comp)
return true
end
end
Expand Down
2 changes: 1 addition & 1 deletion builtin/mainmenu/tab_local.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ function singleplayer_refresh_gamebar()

local btnbar = buttonbar_create(
"game_button_bar",
core.settings:get_bool("enable_touch") and {x = 0, y = 7.25} or {x = 0, y = 7.475},
core.settings:get_bool("touch_gui") and {x = 0, y = 7.25} or {x = 0, y = 7.475},
{x = 15.5, y = 1.25},
"#000000",
game_buttonbar_button_handler)
Expand Down
28 changes: 17 additions & 11 deletions builtin/settingtypes.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
#
# # This is a comment
# #
# # Requires: shaders, enable_dynamic_shadows, !touchscreen_gui
# # Requires: shaders, enable_dynamic_shadows, !enable_waving_leaves
# name (Readable name) type type_args
#
# A requirement can be the name of a boolean setting or an engine-defined value.
Expand All @@ -72,7 +72,7 @@
# * shaders_support (a video driver that supports shaders, may not be enabled)
# * shaders (both enable_shaders and shaders_support)
# * desktop / android
# * touchscreen_gui / keyboard_mouse
# * touchscreen / keyboard_mouse
# * opengl / gles
# * You can negate any requirement by prepending with !
#
Expand Down Expand Up @@ -152,40 +152,42 @@ invert_hotbar_mouse_wheel (Hotbar: Invert mouse wheel direction) bool false

[*Touchscreen]

# Enables touchscreen mode, allowing you to play the game with a touchscreen.
enable_touch (Enable touchscreen) bool true
# Enables the touchscreen controls, allowing you to play the game with a touchscreen.
# "auto" means that the touchscreen controls will be enabled and disabled
# automatically depending on the last used input method.
touch_controls (Enable touchscreen controls) enum auto auto,true,false

# Touchscreen sensitivity multiplier.
#
# Requires: touchscreen_gui
# Requires: touchscreen
touchscreen_sensitivity (Touchscreen sensitivity) float 0.2 0.001 10.0

# The length in pixels after which a touch interaction is considered movement.
#
# Requires: touchscreen_gui
# Requires: touchscreen
touchscreen_threshold (Movement threshold) int 20 0 100

# The delay in milliseconds after which a touch interaction is considered a long tap.
#
# Requires: touchscreen_gui
# Requires: touchscreen
touch_long_tap_delay (Threshold for long taps) int 400 100 1000

# Use crosshair to select object instead of whole screen.
# If enabled, a crosshair will be shown and will be used for selecting object.
#
# Requires: touchscreen_gui
# Requires: touchscreen
touch_use_crosshair (Use crosshair for touch screen) bool false

# Fixes the position of virtual joystick.
# If disabled, virtual joystick will center to first-touch's position.
#
# Requires: touchscreen_gui
# Requires: touchscreen
fixed_virtual_joystick (Fixed virtual joystick) bool false

# Use virtual joystick to trigger "Aux1" button.
# If enabled, virtual joystick will also tap "Aux1" button when out of main circle.
#
# Requires: touchscreen_gui
# Requires: touchscreen
virtual_joystick_triggers_aux1 (Virtual joystick triggers Aux1 button) bool false

# The gesture for for punching players/entities.
Expand All @@ -198,7 +200,7 @@ virtual_joystick_triggers_aux1 (Virtual joystick triggers Aux1 button) bool fals
# Known from the classic Minetest mobile controls.
# Combat is more or less impossible.
#
# Requires: touchscreen_gui
# Requires: touchscreen
touch_punch_gesture (Punch gesture) enum short_tap short_tap,long_tap


Expand Down Expand Up @@ -685,6 +687,10 @@ language (Language) enum ,be,bg,ca,cs,da,de,el,en,eo,es,et,eu,fi,fr,gd,gl,hu,i

[**GUI]

# When enabled, the GUI is optimized to be more usable on touchscreens.
# Whether this is enabled by default depends on your hardware form-factor.
touch_gui (Optimize GUI for touchscreens) bool false

# Scale GUI by a user specified value.
# Use a nearest-neighbor-anti-alias filter to scale the GUI.
# This will smooth over some of the rough edges, and blend
Expand Down
3 changes: 3 additions & 0 deletions irr/include/IEventReceiver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,9 @@ struct SEvent

//! Type of mouse event
EMOUSE_INPUT_EVENT Event;

//! Is this a simulated mouse event generated by Minetest itself?
bool Simulated;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could reuse e.g. ButtonStates with a flag that indicates simulated or not
but other than that setting this in the constructor sounds like the correct approach

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could reuse e.g. ButtonStates with a flag that indicates simulated or not

I fear bugs, consider this:

minetest/src/gui/guiFormSpecMenu.cpp

Lines 4122 to 4126 in 9def45a

// Mouse wheel and move events: send to hovered element instead of focused
if (event.EventType == EET_MOUSE_INPUT_EVENT &&
(event.MouseInput.Event == EMIE_MOUSE_WHEEL ||
(event.MouseInput.Event == EMIE_MOUSE_MOVED &&
event.MouseInput.ButtonStates == 0))) {

but other than that setting this in the constructor sounds like the correct approach

I now notice that setting it in the constructor probably isn't good either. Consider CIrrDeviceSDL, which reuses one SEvent variable for all Irrlicht events it creates from SDL events.

minetest/irr/src/CIrrDeviceSDL.cpp

Lines 646 to 650 in bc23a61

bool CIrrDeviceSDL::run()
{
os::Timer::tick();
SEvent irrevent;

Since union members other than MouseInput are used on that variable (at least KeyInput & TouchInput), MouseInput.Simulated might (in theory) already be overwritten when a mouse event is created.

Copy link
Member

@sfan5 sfan5 Jun 15, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now notice that setting it in the constructor probably isn't good either. Consider CIrrDeviceSDL, which reuses one SEvent variable for all Irrlicht events it creates from SDL events.

bad practice IMO, it shouldn't be doing that.
if you want to fix that you could simply insert irrevent = {} before every use

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, shouldn't replacing every occurrence of SEvent event; with SEvent event{}; also work as a solution?

};

//! Any kind of keyboard event.
Expand Down
2 changes: 2 additions & 0 deletions irr/src/CIrrDeviceLinux.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,7 @@ bool CIrrDeviceLinux::run()

case MotionNotify:
irrevent.EventType = irr::EET_MOUSE_INPUT_EVENT;
irrevent.MouseInput.Simulated = false;
irrevent.MouseInput.Event = irr::EMIE_MOUSE_MOVED;
irrevent.MouseInput.X = event.xbutton.x;
irrevent.MouseInput.Y = event.xbutton.y;
Expand All @@ -791,6 +792,7 @@ bool CIrrDeviceLinux::run()
case ButtonRelease:

irrevent.EventType = irr::EET_MOUSE_INPUT_EVENT;
irrevent.MouseInput.Simulated = false;
irrevent.MouseInput.X = event.xbutton.x;
irrevent.MouseInput.Y = event.xbutton.y;
irrevent.MouseInput.Control = (event.xkey.state & ControlMask) != 0;
Expand Down
9 changes: 9 additions & 0 deletions irr/src/CIrrDeviceOSX.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@ - (BOOL)isQuit

case NSLeftMouseDown:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_LMOUSE_PRESSED_DOWN;
MouseButtonStates |= irr::EMBSM_LEFT;
ievent.MouseInput.ButtonStates = MouseButtonStates;
Expand All @@ -796,6 +797,7 @@ - (BOOL)isQuit

case NSLeftMouseUp:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
MouseButtonStates &= !irr::EMBSM_LEFT;
ievent.MouseInput.ButtonStates = MouseButtonStates;
ievent.MouseInput.Event = irr::EMIE_LMOUSE_LEFT_UP;
Expand All @@ -804,6 +806,7 @@ - (BOOL)isQuit

case NSOtherMouseDown:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_MMOUSE_PRESSED_DOWN;
MouseButtonStates |= irr::EMBSM_MIDDLE;
ievent.MouseInput.ButtonStates = MouseButtonStates;
Expand All @@ -812,6 +815,7 @@ - (BOOL)isQuit

case NSOtherMouseUp:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
MouseButtonStates &= !irr::EMBSM_MIDDLE;
ievent.MouseInput.ButtonStates = MouseButtonStates;
ievent.MouseInput.Event = irr::EMIE_MMOUSE_LEFT_UP;
Expand All @@ -823,13 +827,15 @@ - (BOOL)isQuit
case NSRightMouseDragged:
case NSOtherMouseDragged:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_MOUSE_MOVED;
ievent.MouseInput.ButtonStates = MouseButtonStates;
postMouseEvent(event, ievent);
break;

case NSRightMouseDown:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_RMOUSE_PRESSED_DOWN;
MouseButtonStates |= irr::EMBSM_RIGHT;
ievent.MouseInput.ButtonStates = MouseButtonStates;
Expand All @@ -838,6 +844,7 @@ - (BOOL)isQuit

case NSRightMouseUp:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_RMOUSE_LEFT_UP;
MouseButtonStates &= !irr::EMBSM_RIGHT;
ievent.MouseInput.ButtonStates = MouseButtonStates;
Expand All @@ -846,6 +853,7 @@ - (BOOL)isQuit

case NSScrollWheel:
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_MOUSE_WHEEL;
ievent.MouseInput.Wheel = [(NSEvent *)event deltaY];
if (ievent.MouseInput.Wheel < 1.0f)
Expand Down Expand Up @@ -1046,6 +1054,7 @@ - (BOOL)isQuit
if (curr.X != x || curr.Y != y) {
irr::SEvent ievent;
ievent.EventType = irr::EET_MOUSE_INPUT_EVENT;
ievent.MouseInput.Simulated = false;
ievent.MouseInput.Event = irr::EMIE_MOUSE_MOVED;
ievent.MouseInput.X = x;
ievent.MouseInput.Y = y;
Expand Down