fix: Race condition between GamepadPoll detour and ImGui setup

Ideally, we would use
(ImGui.GetIO().ConfigFlags & ImGuiConfigFlags.NavEnableGamepad) > 0
for testing in the GamepadPollDetour whether ImGui should handle gamepad controls
or not. However, this has a race condition during load with the detour which sets
up ImGui and throws if our detour gets called before the other, so we opt-in
for a dedicated boolean.
This commit is contained in:
Chivalrik 2021-05-01 13:43:48 +02:00
parent 9a9aae3db7
commit e4521c0100
2 changed files with 78 additions and 46 deletions

View file

@ -3,6 +3,7 @@
using Dalamud.Game.ClientState.Structs; using Dalamud.Game.ClientState.Structs;
using Dalamud.Hooking; using Dalamud.Hooking;
using ImGuiNET; using ImGuiNET;
using Serilog;
namespace Dalamud.Game.ClientState namespace Dalamud.Game.ClientState
{ {
@ -28,6 +29,9 @@ namespace Dalamud.Game.ClientState
/// <param name="resolver">Resolver knowing the pointer to the GamepadPoll function.</param> /// <param name="resolver">Resolver knowing the pointer to the GamepadPoll function.</param>
public GamepadState(ClientStateAddressResolver resolver) public GamepadState(ClientStateAddressResolver resolver)
{ {
#if DEBUG
Log.Verbose("GamepadPoll address {GamepadPoll}", resolver.GamepadPoll);
#endif
this.gamepadPoll = new Hook<ControllerPoll>( this.gamepadPoll = new Hook<ControllerPoll>(
resolver.GamepadPoll, resolver.GamepadPoll,
(ControllerPoll)this.GamepadPollDetour); (ControllerPoll)this.GamepadPollDetour);
@ -118,10 +122,21 @@ namespace Dalamud.Game.ClientState
/// </summary> /// </summary>
internal ushort ButtonsRepeat { get; private set; } internal ushort ButtonsRepeat { get; private set; }
/// <summary>
/// Gets or sets a value indicating whether detour should block gamepad input for game.
///
/// Ideally, we would use
/// (ImGui.GetIO().ConfigFlags & ImGuiConfigFlags.NavEnableGamepad) > 0
/// but this has a race condition during load with the detour which sets up ImGui
/// and throws if our detour gets called before the other.
/// </summary>
internal bool NavEnableGamepad { get; set; }
/// <summary> /// <summary>
/// Gets whether <paramref name="button"/> has been pressed. /// Gets whether <paramref name="button"/> has been pressed.
/// ///
/// Only true on first frame of the press. /// Only true on first frame of the press.
/// If ImGuiConfigFlags.NavEnableGamepad is set, this is unreliable.
/// </summary> /// </summary>
/// <param name="button">The button to check for.</param> /// <param name="button">The button to check for.</param>
/// <returns>1 if pressed, 0 otherwise.</returns> /// <returns>1 if pressed, 0 otherwise.</returns>
@ -131,6 +146,7 @@ namespace Dalamud.Game.ClientState
/// Gets whether <paramref name="button"/> is being pressed. /// Gets whether <paramref name="button"/> is being pressed.
/// ///
/// True in intervals if button is held down. /// True in intervals if button is held down.
/// If ImGuiConfigFlags.NavEnableGamepad is set, this is unreliable.
/// </summary> /// </summary>
/// <param name="button">The button to check for.</param> /// <param name="button">The button to check for.</param>
/// <returns>1 if still pressed during interval, 0 otherwise or in between intervals.</returns> /// <returns>1 if still pressed during interval, 0 otherwise or in between intervals.</returns>
@ -140,6 +156,7 @@ namespace Dalamud.Game.ClientState
/// Gets whether <paramref name="button"/> has been released. /// Gets whether <paramref name="button"/> has been released.
/// ///
/// Only true the frame after release. /// Only true the frame after release.
/// If ImGuiConfigFlags.NavEnableGamepad is set, this is unreliable.
/// </summary> /// </summary>
/// <param name="button">The button to check for.</param> /// <param name="button">The button to check for.</param>
/// <returns>1 if released, 0 otherwise.</returns> /// <returns>1 if released, 0 otherwise.</returns>
@ -173,10 +190,12 @@ namespace Dalamud.Game.ClientState
private int GamepadPollDetour(IntPtr gamepadInput) private int GamepadPollDetour(IntPtr gamepadInput)
{ {
var original = this.gamepadPoll.Original(gamepadInput);
try
{
#if DEBUG #if DEBUG
this.GamepadInput = gamepadInput; this.GamepadInput = gamepadInput;
#endif #endif
var original = this.gamepadPoll.Original(gamepadInput);
var input = (GamepadInput*)gamepadInput; var input = (GamepadInput*)gamepadInput;
this.leftStickX = input->LeftStickX; this.leftStickX = input->LeftStickX;
this.leftStickY = input->LeftStickY; this.leftStickY = input->LeftStickY;
@ -187,7 +206,7 @@ namespace Dalamud.Game.ClientState
this.ButtonsReleased = input->ButtonsReleased; this.ButtonsReleased = input->ButtonsReleased;
this.ButtonsRepeat = input->ButtonsRepeat; this.ButtonsRepeat = input->ButtonsRepeat;
if ((ImGui.GetIO().ConfigFlags & ImGuiConfigFlags.NavEnableGamepad) > 0) if (this.NavEnableGamepad)
{ {
input->LeftStickX = 0; input->LeftStickX = 0;
input->LeftStickY = 0; input->LeftStickY = 0;
@ -205,15 +224,16 @@ namespace Dalamud.Game.ClientState
// (L2/R2 being held down activates CrossHotBar but activating an ability is impossible because of the others blocked input, // (L2/R2 being held down activates CrossHotBar but activating an ability is impossible because of the others blocked input,
// Digipad is ignored in menus but without any menu's one still switches target or party members, but cannot interact with them // Digipad is ignored in menus but without any menu's one still switches target or party members, but cannot interact with them
// because of the other blocked input) // because of the other blocked input)
// `ButtonPressed` is pretty useful so we opt-in to (b). // `ButtonPressed` is pretty useful but its hella confusing to the user, so we do (a) and advise plugins do not rely on
// `ButtonPressed` while ImGuiConfigFlags.NavEnableGamepad is set.
// This is debatable. // This is debatable.
// ImGui itself does not care either way as it uses the Raw values and does its own state handling. // ImGui itself does not care either way as it uses the Raw values and does its own state handling.
// input->ButtonsRaw &= (ushort)~GamepadButtons.L2; input->ButtonsRaw &= (ushort)~GamepadButtons.L2;
// input->ButtonsRaw &= (ushort)~GamepadButtons.R2; input->ButtonsRaw &= (ushort)~GamepadButtons.R2;
// input->ButtonsRaw &= (ushort)~GamepadButtons.DpadDown; input->ButtonsRaw &= (ushort)~GamepadButtons.DpadDown;
// input->ButtonsRaw &= (ushort)~GamepadButtons.DpadLeft; input->ButtonsRaw &= (ushort)~GamepadButtons.DpadLeft;
// input->ButtonsRaw &= (ushort)~GamepadButtons.DpadUp; input->ButtonsRaw &= (ushort)~GamepadButtons.DpadUp;
// input->ButtonsRaw &= (ushort)~GamepadButtons.DpadRight; input->ButtonsRaw &= (ushort)~GamepadButtons.DpadRight;
input->ButtonsPressed = 0; input->ButtonsPressed = 0;
input->ButtonsReleased = 0; input->ButtonsReleased = 0;
input->ButtonsRepeat = 0; input->ButtonsRepeat = 0;
@ -224,6 +244,15 @@ namespace Dalamud.Game.ClientState
// original, zero or do the work adjusting the bits. // original, zero or do the work adjusting the bits.
return original; return original;
} }
catch (Exception e)
{
Log.Error(e, $"Gamepad Poll detour critical error! Gamepad navigation will not work!");
// NOTE (Chiv) Explicitly deactivate on error
ImGui.GetIO().ConfigFlags &= ~ImGuiConfigFlags.NavEnableGamepad;
return original;
}
}
private void Dispose(bool disposing) private void Dispose(bool disposing)
{ {

View file

@ -304,6 +304,9 @@ namespace Dalamud.Interface
ImGui.GetIO().ConfigFlags |= ImGuiConfigFlags.NavEnableSetMousePos; ImGui.GetIO().ConfigFlags |= ImGuiConfigFlags.NavEnableSetMousePos;
} }
// NOTE (Chiv) Explicitly deactivate on dalamud boot
ImGui.GetIO().ConfigFlags &= ~ImGuiConfigFlags.NavEnableGamepad;
ImGuiHelpers.MainViewport = ImGui.GetMainViewport(); ImGuiHelpers.MainViewport = ImGui.GetMainViewport();
Log.Information("[IM] Scene & ImGui setup OK!"); Log.Information("[IM] Scene & ImGui setup OK!");
@ -484,7 +487,7 @@ namespace Dalamud.Interface
&& this.dalamud.ClientState.GamepadState.Pressed(GamepadButtons.L3) > 0) && this.dalamud.ClientState.GamepadState.Pressed(GamepadButtons.L3) > 0)
{ {
ImGui.GetIO().ConfigFlags ^= ImGuiConfigFlags.NavEnableGamepad; ImGui.GetIO().ConfigFlags ^= ImGuiConfigFlags.NavEnableGamepad;
this.dalamud.DalamudUi.TogglePluginInstaller(); this.dalamud.ClientState.GamepadState.NavEnableGamepad ^= true;
} }
if (gamepadEnabled if (gamepadEnabled