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.

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

In Controls Custom Scheme, the "/" next to "Enter Console" is blinking #1218

Closed
aredfan opened this issue Mar 28, 2024 · 14 comments · Fixed by #1260
Closed

In Controls Custom Scheme, the "/" next to "Enter Console" is blinking #1218

aredfan opened this issue Mar 28, 2024 · 14 comments · Fixed by #1260
Assignees
Labels
TR1X bug A bug with TR1X
Milestone

Comments

@aredfan
Copy link
Collaborator

aredfan commented Mar 28, 2024

This happens in the 2 most recent dev builds. Very minor issue I know, probably related to #1177.

(In the keyboard section)

Doesn't happen in 3.1.1.

Screenshot

@aredfan aredfan changed the title In Controls Custom Scheme, the "/" next to "Enter Console" is blinking. In Controls Custom Scheme, the "/" next to "Enter Console" is blinking Mar 28, 2024
@rr- rr- added TR1X bug A bug with TR1X Internal The invisible stuff labels Mar 29, 2024
@rr-
Copy link
Collaborator

rr- commented Mar 30, 2024

I can't quite reproduce this in the most recent 60 FPS build: https://nebula.wind.garden/serve/pub/TR1X%203.1.1-110-g3721734-Windows.zip

@aredfan
Copy link
Collaborator Author

aredfan commented Mar 30, 2024

@rr- I re-checked and initially did still have the "/" blinking, I then tried doing a Reset All: Hold R which seemed to have stopped the blinking. In the TR1X.json5 file, this changed 56 to 46 on the 126th line.

I'm guessing the fault was mine for using TR1X.json5 shared with 60_FPS_v1. I'll share this file in case you need to check it. The blinking issue is still present in Custom Scheme 2 and 3.

TR1X.zip

@rr- rr- removed the Internal The invisible stuff label Mar 30, 2024
@rr- rr- added this to the 3.2 milestone Mar 30, 2024
@rr- rr- self-assigned this Apr 1, 2024
@rr-
Copy link
Collaborator

rr- commented Apr 2, 2024

This flashing was introduced back in version 3.0 and manifests deficiency of our system to introduce new input roles. When a new input role is introduced, such as opening the developer console, it is added to the existing key mappings without altering the current configuration. This is done under the assumption that players have intentionally set their own key preferences and we need to respect that. Now, the configuration file stores the entire key mapping, not just the keys that have been modified from the default settings. This approach has benefits, as modifying a default input role will not affect players who are used to the original default keys. We could alleviate this by either changing the save mechanism to only store what's changed from the current defaults, or introducing a migration mechanism to handle different builtin configuration versions and user key choices, but even those solutions wouldn't be fully foolproof and could lead to potential conflicts with existing player preferences. For example:

  • We chose / for reset camera in 2.x
  • Now it's 3.0 and we want that key to be bound to =, and have / open the dev console
  • in 2.x the player bound = to look
  • we see that / is still bound to reset camera which we know it was previous defaults (thanks to either migration or the fact that it's missing from the config), and now we have two options:
    1. it's ok for us to rebind it to =
      …but we just generated a conflict with the player's choice to look
    2. it's not ok for us to rebind it to =, as we understand it'll generate a conflict with look
      …but we keep the conflict where / will be bound to both the dev console and the reset camera button

All in all I think it's too late for us to fix this particular bug, but we can reduce the impact of such scenario in the future by only saving what's deviated from defaults, and opt for the second scenario above.

Please LMK your thoughts @LostArtefacts/dev @LostArtefacts/qa

@Richard-L
Copy link
Collaborator

I'm sorry if I don't have more useful input as I don't understand the questions enough, but: I know I would want to remap everything, especially characters that aren't the alphabet, as I'm on a QWERTZ keyboard, and most any default key mappings of any app typically make no sense. So even the key opening the console I cannot access in the default mapping, because it requires SHIFT for me. Instead I would need it on the ^ key, etc.

So all this just to say that, as far as it's applicable, I would think that a system allowing to remap everything and that doesn't somehow rely on any "default", is best.

@rr-
Copy link
Collaborator

rr- commented Apr 2, 2024

@Richard-L, you currently and will continue to have the capability to remap all controls. I apologize if my post was too technical – let me try to rephrase it. My focus is on how to handle introducing new input methods or altering existing defaults, particularly if the player has not made any changes themselves. At the moment, our config code behaves as if the previous defaults were intentionally chosen by the player, even when we update them upstream. For example, when we changed the default key for resetting the camera from / to =, this only affected new installations – existing installations kept it at / due to how it currently works. This led to a key conflict when we also bound / to opening the developer console in the same version. In other words, players who updated from 2.16 to 3.0 without even touching their input settings, had a key conflict (this issue) that I believe can and should be avoided. My suggestion is that if the player has not explicitly changed a default key and we decide to change it in a new version, it should be acceptable to do so. Especially that this should only ever happen for more esoteric functions such as the developer console or camera overrides, rather than core gameplay.

@aredfan
Copy link
Collaborator Author

aredfan commented Apr 2, 2024

  1. it's ok for us to rebind it to =
    …but we just generated a conflict with the player's choice to look
  2. it's not ok for us to rebind it to =, as we understand it'll generate a conflict with look
    …but we keep the conflict where / will be bound to both the dev console and the reset camera button

I'm inclined toward the first option but I understand this would mean player A preferring to use "/" for Reset Camera will have to rebind when there's a new update. I'm not sure if that's a big problem because tr1x's new releases are often months apart. Personally I use the 0 on numpad which also resets the camera.

The second option is also fine, and technically solvable because we have the Reset All: Hold R.

@Richard-L
Copy link
Collaborator

Thanks for clarifying dash. That's super considerate of the users of you. Yet my stance would be to force new default keys on people, if new functionality has been added that requires it. I would expect these cases to be rare enough (and as you say for esoteric causes few people use) so that a custom remembering and respect for individual user binds is not necessary. It seems more important to me to reliably avoid input conflicts, and at the same time keep the workload in check for you. Even if that means some people are confused after an update why a particular input suddenly behaves unexpectedly.

@rr-
Copy link
Collaborator

rr- commented Apr 2, 2024

@LostArtefacts/dev WDYT? I'm against resetting all controls TBH.

@walkawayy
Copy link
Collaborator

I agree with not resetting all controls. We rarely add new keybinds, so this is a minor issue in the grand scheme of things right? I think if users just rebind the conflicted key that should be ok?

@lahm86
Copy link
Collaborator

lahm86 commented Apr 2, 2024

Sorry, I forgot to reply earlier. Agreed as well, in the event of issues it's not difficult to rebind so I don't think implementing any sort of "management" over existing controls should be supported.

@rr-
Copy link
Collaborator

rr- commented Apr 2, 2024

(Technical stuff ahead) Is it ok for me to change the config to not store the controls, if they do not deviate from the current version's defaults? This way we get to change defaults in later versions without introducing potential conflicts. @LostArtefacts/dev

@walkawayy
Copy link
Collaborator

walkawayy commented Apr 2, 2024

(Technical stuff ahead) Is it ok for me to change the config to not store the controls, if they do not deviate from the current version's defaults? This way we get to change defaults in later versions without introducing potential conflicts. @LostArtefacts/dev

I think that will work. If I understand, this will cause a one time break in people's custom controls in the next release since the config format will change?

Semi related, at one point I wanted to change our config to use the actual SDL mapping standard. I would need to read into it more though. They look like this: https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamepad_db.h

And would get saved in the config similar to: "03000000de280000ff11000000007701,Steam Virtual Gamepad,a:b0,b:b1,back:b6,dpdown:b12,dpleft:b13,dpright:b11,dpup:b10,leftshoulder:b4,leftstick:b8,lefttrigger:a4,leftx:a1,lefty:a0~,rightshoulder:b5,rightstick:b9,righttrigger:a5,rightx:a3,righty:a2~,start:b7,x:b2,y:b3,",

@rr-
Copy link
Collaborator

rr- commented Apr 2, 2024

(Technical stuff ahead) Is it ok for me to change the config to not store the controls, if they do not deviate from the current version's defaults? This way we get to change defaults in later versions without introducing potential conflicts. @LostArtefacts/dev

I think that will work. If I understand, this will cause a one time break in people's custom controls in the next release since the config format will change?

No :) the change should be seamless.

Semi related, at one point I wanted to change our config to use the actual SDL mapping standard. I would need to read into it more though. They look like this: https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamepad_db.h

And would get saved in the config similar to: "03000000de280000ff11000000007701,Steam Virtual Gamepad,a:b0,b:b1,back:b6,dpdown:b12,dpleft:b13,dpright:b11,dpup:b10,leftshoulder:b4,leftstick:b8,lefttrigger:a4,leftx:a1,lefty:a0~,rightshoulder:b5,rightstick:b9,righttrigger:a5,rightx:a3,righty:a2~,start:b7,x:b2,y:b3,",

Interesting. Thanks for looking into it. In the future we may revisit it, but I don't think now's the best time to tackle this.

@walkawayy
Copy link
Collaborator

I'm good with that then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TR1X bug A bug with TR1X
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants