xOBSE icon indicating copy to clipboard operation
xOBSE copied to clipboard

Number of reports that disabled keys are staying disabled when you open the inventory menu

Open SMUnlimited opened this issue 3 years ago • 10 comments

Normally a disabled key is automatically re-enabled when the inventory menu is opened and is also my own experience (and wiki's experience https://cs.elderscrolls.com/index.php?title=DisableControl)

But I've had 3 reports on one of my mods suggesting that is not the case for them. Disabling the run control (usually shift key) instead also prevents them from dropping any items from the inventory menu.

I've forced everyone up to xOBSE 23, but still getting some reports of this.

I don't have any more information at this stage, still trying to get more info if its a particular mod or plugin causing this difference, or if its just behaving differently for their oblivion copy (I'm using steam, win10 21H1 with no issue).

SMUnlimited avatar Jun 14 '21 06:06 SMUnlimited

It seems more of a case that some menus were totally ignoring the disabled key, probably some menu depended on a different dinput method. I have to say I'm unsure. The more correct way should be to edit the mod and reenable the keys when in a menu. I could just skip applying disabling keys in the menus, but then I don't know which menus should ignore this or which menu respect this by design.

Goddammit whatever I do it risk breaking something

llde avatar Jun 14 '21 15:06 llde

Yeah it is a bit annoying as every mod that does disable a control you have no way of knowing if it happens to match a key that is used in menus due to someone modifying there key bindings, so everyone will be forced to cancel them out in all menus just to be sure.

Perhaps the better way is having a flag which can disable control everywhere or disable only outside of menu's or disable only within menus. Let the modder control the decision rather than it being effectively magic. Although that doesn't solve menus that just outright do not respect it -meh-

SMUnlimited avatar Jun 14 '21 16:06 SMUnlimited

I could do this, and it isn't even complex to do. The only caveat is what default I use? Block in menu by default, Allow in menu by default? Change the default based if I'm disabling a key or a control?

llde avatar Jun 14 '21 16:06 llde

My 2 cents considering the list of controls to disable are listed as non-menu options it probably makes sense to default to allowing in menus by default?

Then when disabling individual keys, block for all by default makes more sense?

SMUnlimited avatar Jun 14 '21 21:06 SMUnlimited

@SMUnlimited Testing shown that the behaviour between OBSe and xOBSE 22.2 aren't actually different. Both me and @Laulajatar (in the Oblivion Community Server on Discord), aren't able to properly replicate this bug. However it seems (tested by Laulajatar) that NorthenerUI DLL (the DLL only seems enough) from DavidJCobb, may actually cause this incompatibility. (But also on the original OBSE 21) @DavidJCobb may elaborate a bit better maybe?

So Evidence suggest this is a "Not Our Bug" type of issue.

llde avatar Aug 03 '21 22:08 llde

This isn't something I would do intentionally, and NorthernUI shouldn't even be able to see OBSE's key override states, let alone directly change how they're applied. If I'm causing a problem here, then it'd have to be a side-effect of something I'm doing. Indeed, looking at my InventoryMenu patches, I don't see anything that should affect input processing; just patches for where certain strings are displayed.

I'm happy to look into this on my end, but I'd need a place to start. What is the mechanism by which OBSE would prevent its key state overrides from bleeding into menu mode? I looked through your Hooks_Input.cpp and saw no obvious references to menu state.

Incidentally I'm also not sure whether your intention is that the OBSE input commands have no effect in menu mode generally, or just in specific menus including InventoryMenu. Even the CS wiki article on DisableControl, linked above and last updated in 2009, notes that not all control overrides are ignored by InventoryMenu, giving LMB as a particularly bad case. Are we sure that overrides being ignored is an intentional OBSE behavior?

It seems more of a case that some menus were totally ignoring the disabled key, probably some menu depended on a different dinput method.

Menu input events should all be dispatched when InterfaceManager checks the key states on OSInputGlobals every frame, calling virtual functions on menus as appropriate. Menus also have a frame handler, and can check input states from inside of it, but they should also be doing that through OSInputGlobals. Any changes you make to key states in OSInputGlobals should always affect menu behavior.

Looking at InventoryMenu, its frame handler checks input through OSInputGlobals to handle the quick keys. All other inputs should be handled by the menu input handlers.

The only other ways I can think of for Bethesda to check input without OSInputGlobals are:

  • Intermodular calls to DInput. However, the only such call I see is a single call to DirectInput8Create, which stores a pointer in OSInputGlobals; IIRC all later interactions with DInput rely on virtual member functions on that pointer, so anything wishing to check input would still have to go through OSInputGlobals at least to get the DInput pointers it stores.
  • The Wndproc, which I believe is at 0x004060F5, and which Bethesda uses to handle all of the following messages. None of them seem to be used for handling game/UI input.
    • WM_NCHITTEST is used for ShowCursor calls, to toggle the OS mouse cursor based on whether the window has focus.
    • WM_DESTROY.
    • WM_ACTIVATE. OSInputGlobals is told to flush its keyboard buffer and then poll for inputs. The g_timeInfo singleton is managed here as well, but I'm not sure on the details.
    • WM_ERASEBKGND.
    • WM_WINDOWPOSCHANGED. Window position seems to be tracked when the game is not running in full screen.

The only caveat is what default I use? Block in menu by default, Allow in menu by default? Change the default based if I'm disabling a key or a control?

Looking at your input hooks and commands, it seems that when OBSE "enables or disables a control," it's actually just enabling or disabling the key to which that control is currently mapped. If the control is remapped after being "disabled," the previous key will remain disabled and the new key will be unaffected.

To my thinking, the most intuitive model would be to have the "control" commands actually affect the specific mapped control -- so, maintain flags for controls in addition to for keys, and use the controls' flags to alter the states of the mapped keys on a per-frame basis. It would probably also be more intuitive to have control overrides not apply while the game is in menu mode, since none of the controls (attack, cast spell, etc.) are applicable in menu mode. However, these would be pretty significant changes to the behavior of the control-related OBSE commands, and it's possible that they might break preexisting mods somehow.

DavidJCobb avatar Aug 04 '21 12:08 DavidJCobb

@DavidJCobb I did some tests and controls are affected by this issue only in a tangent way. The proper issue lies with the keycodes. The run control is mentioned just becouse it's generally mapped to 42 the key needed to drop items from the inventory.

The original code intercepted the call to DInputDevice_Create inserting ashim between the game and the proper device. The new system instead operate directly on the Polling functions for OSInputGlobal.

It should be fundamentally equal (minus bugs and a still non reimplmented functions). And in fact test shows that on a "clean" evironment if I disable the 42 key the override is in effect (can't run/slow down anymore) but I can properly drop stuffs from the inventory. This is both OBSEv21 and the latest xOBSE. However this is not the case with NUI. @Laulajatar tested that even with all the options off, the NUI DLL change this behaviour and prevent the drop to work. Note there is a strong difference with mouse keys. They seems to be disabled even in menus.

This seems to imply (but could not verify in the code, that in menu mode (some of the menu modes at least) inputs are processed in another way), and NUI instead route them to OSInputGlobals::PollInput method.

llde avatar Aug 04 '21 13:08 llde

It seems you were right. I think I know what the problem is. Keycode 42 is left-shift? If I'm right, then the issue would affect the shift keys specifically.

Oblivion always processes input through OSInputGlobals, but sometimes in strange ways. The InterfaceManager uses OSInputGlobals to check the state of the Shift key, and stores that in a flag on the InterfaceManager... but not reliably. In the vanilla engine, this can result in bugs where the UI engine fails to notice that you've released the Shift key, causing the game to behave as if the Shift key is stuck down. Apparently, way back, I'd implemented a fix for this by patching the game to re-check OSInputGlobals and update the InterfaceManager's Shift key flag accordingly.

That's the first part of the puzzle.

The second part is that there's a function that never made it into my OSInputGlobals class definition: int OSInputGlobals::GetBufferedKeyStateChange(Keyboard_Device&), at 0x00403C90. It relies on IDirectInputDevice8::GetDeviceData to query key states; I'm not sure whether that would've allowed it to bypass the original OBSE DInput shim, but I presume you're not patching that function, so it'd bypass the shim you're using now.

This OSInputGlobals function is called by the InterfaceManager update function a few times as part of a loop, and each time, the results are passed to InterfaceManager::KeyStateChangeToChar, which updates the InterfaceManager's Shift key flag and other flags in the same mask (InterfaceManager::unk118). If the last received keypress produces a printable key, and if that isn't intercepted by the game's debug console, then it will be passed to an active menu first as a generic keyboard input and then possibly as a navigation input.

I'm not altogether sure why Bethesda chose to do things this way, and I'm not sure why it results in the menu manager's Shift key flag getting stuck. I'm also not familiar with the original OBSE DInput shim, so I don't know whether it failed to catch IDirectInputDevice8::GetDeviceData. However, I suspect that the ability to use disabled keys within menus may have originally been due to OBSE failing to shim that part of DInput -- an accident, then, rather than an intentional feature. That oversight would've persisted into your own implementation due to my failure to properly document OSInputGlobals::GetBufferedKeyStateChange for you and others.

...Which begs the question of how best to handle all of this. I can remove my Shift key fix from NorthernUI, though that then leaves us with the original issue of the vanilla game sometimes failing to notice when Shift is released -- especially nasty in menus where shift-clicking is meant to, say, delete save files. I'm not really sure how to proceed.

DavidJCobb avatar Aug 04 '21 14:08 DavidJCobb

@DavidJCobb Yes Left Shift. Maybe an alternative solution may be to reset that shift flag when a keyboard input is received?

The old DInput hook, did have a GetDeviceData hook, however while there were OBSE functions specific to feed keypresses (tap, hold, press) it had nothing to disable presses itself. However I did some tests and these function didn't seem to work at all (albeit maybe I just used them badly) and didn't find any mod to use them. If someone can give me amod that used that commands and they worked, then I will think about reimplmeneting them, otherwise I will keep this as is.

llde avatar Aug 04 '21 14:08 llde

@SMUnlimited Testing shown that the behaviour between OBSe and xOBSE 22.2 aren't actually different. Both me and @Laulajatar (in the Oblivion Community Server on Discord), aren't able to properly replicate this bug. However it seems (tested by Laulajatar) that NorthenerUI DLL (the DLL only seems enough) from DavidJCobb, may actually cause this incompatibility. (But also on the original OBSE 21) @DavidJCobb may elaborate a bit better maybe?

So Evidence suggest this is a "Not Our Bug" type of issue.

Thanks all that at least gives me the cause which I can mention to others.

SMUnlimited avatar Aug 04 '21 15:08 SMUnlimited