Trenchbroom: Permissive entity editor keypress handling allows for accidental overwrites

[Shfty]

While using TB, I've often found myself accidentally overwriting recently-edited entity properties with nonsense values like sd, es, fs, etc. from the movement keys.

This happens while moving the mouse back to the viewport after editing an entity field with the intent to immediately start moving around the scene: If I press a movement key before the viewport is focused with a mouse-down, the entity field gets overwritten.
What's worse is that it's easy to continue editing and focus other entities without realising that a given field has changed until I spot an error in either QBSP's compile output or the level itself during testing.

Having looked into the EntityAttributeTable code, it's evident that this behaviour is intentional since shortcuts like UV lock have special-case handling.

I realise this might be a me problem, but it's not one I've encountered in any other 3D editor as most of them either force the user to focus the viewport with RMB in order to move the camera, or defocus property fields after editing. I've started using Esc to defocus the field manually, but still find myself mangling properties on occasion.

System Information

TrenchBroom 2019.6 Build v2019.6 Release
(Also affects current master branch)

Expected Behavior

• Accidental entity property overwrites are prevented

Steps to Reproduce

• Select an entity
• Open the entity editor
• Double click a property value and edit it
• Move the mouse back over the 3D viewport and press a movement key

[Shfty]

I'm happy to look at implementing something if this ends up being considered.

Initially I was going to suggest adding an 'input guard' checkbox in preferences that would filter all keys except for return, delete, backspace and the arrow keys when the entity editor is focused, but that doesn't seem like a very elegant way to fix it.

One route might be to employ some focus-follows-mouse type logic similar to the one employed by split viewports. Since TB doesn't force the user to hold RMB in order to move, handing focus back to the viewport when the mouse leaves the sidebar would solve the problem and also speed up workflow a little by allowing for immediate movement.

[kduske]

I haven't seen a lot of reports from other people having similar problems. If this is a problem that happens to only few people, I'd rather not change the current behavior (or add an option to change it). If it is something that bothers more people, we can consider it.

[bstordeur]

For info this happens to me ALL THE TIME, but I haven't reported it cause I was waiting to see if it was still relevant in the QT version (since it's a big UI overhaul).
As Shfty described, dropping in some movement keys into target/targetnames is really common, I've seen others complain about it occasionally as well.

No idea for an elegant fix though.

[kduske]

Ok, good to know it affects more people. I see the following possible solutions:

• Require a hotkey to edit entity properties: Instead of allowing to edit a property just by typing, we could require the user to press Return / double click first.
• Only recognize fly mode keys when RMB is down. This is not an immediate solution, but it would over time train users to only press the fly mode keys when RMB is down.
• Focus an editing view immediately when the mouse cursor hovers over it.

[bstordeur]

Responding to your suggestions in order :

1. Editing the property is never the issue. This problem always happens when you've just edited something and then gone back to fly around in the 3D view, so just making it more cumbersome to initially edit won't help.
2. This is awkward, I know I often do fine movements without RMB pressed while editing stuff, just a quick strafe to the side and such.
3. I feel like this is an interesting option, might confuse some users though. Maybe have it so if a navigation key is pressed AND the mouse cursor is over the a 3d/2d view, then exit (and validate) any active value box and start flying around? Would that be possible?

[kduske]

I agree options 1 and 2 are not very good. I just listed them for completeness sake. Option 3 is most viable, maybe with the modifications you proposed. We'll have to see.

[Shfty]

Re. each option:

1. I actually implemented this locally (filtering all keys save for arrows, return and row delete shortcuts) before opening this issue. It's functional, but makes things feel clunky when you mouse back over to the viewport and have all the alphanum keys do nothing until you click.

2. I agree that tying fly mode to RMB would be a bit cumbersome, since it's nice for fine adjustments.

3. Playing devil's advocate on this one: I feel like adding overrides specifically for the navigation keys also has the potential to confuse users. While movement key overwrites are the most common outcome, it would be a fairly specific extra rule to keep in mind, and doesn't cover other keys that the user may press during the mouse move such as tool switching, show/hide, etc.

A more top-level solution could be to offer a focus-follows-mouse preference with None, Viewport Only and All options:

• None
  
  
  
  • No FFM at all, even for viewports. Focus must be changed via click.
  
  
  • Prevents users being tripped up by accidental focus changes during viewport-relative object moves
  
  
  • (May be beyond the scope of this issue since it's technically a separate usability thing)
  
  
• Viewport Only
  
  
  
  • Same as the existing behavior
  
  
  • Accounts for users accustomed to the current workflow
  
  
• Full
  
  
  
  • FFM applies to the viewports, sidebar and console (and toolbar?)
  
  
  • Addresses accidental overwrites
  
  

[kduske]

I try to avoid adding such preferences if at all possible. Often they are the result of not taking the time to find an optimal solution to a particular problem such as this. If we can find a good solution that feels natural to most users, then we don't need any further preferences for this. I think that that would be preferrable.

[ericwa]

Related: https://github.com/kduske/TrenchBroom/issues/2540

Regarding whether or not the first click should be consumed, this was an intentional change that I introduced because often times, you want to be able to give focus to the map view to make certain shortcuts work without changing the selection.

My initial request to change the first click consumption: https://github.com/kduske/TrenchBroom/issues/2042

[ericwa]

I commented on discord:

Personally I think "focus follows mouse" at least for the map views/texture browser would be interesting to try:
if the map view is unfocused, you can mouse over it and do a keyboard shortcut without changing the selection
the other possibility is just you have to click to focus the map view, and the click also does a selection. This is pretty common (UE4 does this iirc)
with the assumption that it's less annoying to undo a selection change than have to click twice all the time
Blender does focus follows mouse

Agree I would rather not have preferences for this, we should try a solution and test it (post 2020.1).

[kduske]

Of all the solutions so far, I find "focus follows mouse" the most natural, and I think it also fixes the original issue best.

[ericwa]

Yeah agreed