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

Enforce con_enable 1 and multiple fixes for keybind handling #589

Merged

Conversation

nullsystem
Copy link
Collaborator

@nullsystem nullsystem commented Sep 15, 2024

Description

  • Enforce con_enable 1 to fix console toggle issue
  • Instead always enforce the "legacy" toggleconsole, because internally it requires con_enable to be set anyway, although the legacy menu toggles this.
  • Basically means the support for legacy menu on this is an unsupported thing anyway
  • Write the command file on save
  • Fix attempts to write empty keys
  • Unbind duplicate key

Toolchain

  • Linux GCC Distro Native Arch/GCC 14

Linked Issues

@nullsystem nullsystem added the bug Something isn't working label Sep 17, 2024
@nullsystem nullsystem requested a review from a team September 18, 2024 17:27
@nullsystem nullsystem added Minor release priority Pull request is a priority for the next minor release and removed Minor release priority Pull request is a priority for the next minor release labels Sep 18, 2024
@nullsystem nullsystem added this to the v8.1-prealpha milestone Sep 18, 2024
@Rainyan
Copy link
Collaborator

Rainyan commented Sep 18, 2024

I'm having trouble saving the preference, so I can't really test the change. Using the legacy menu, I can save the "developer console" setting correctly however.

Example:

bind.webm

@nullsystem
Copy link
Collaborator Author

nullsystem commented Sep 18, 2024

@Rainyan It seems like it failed because 'q' was also used for lean left. Guess that's also why the legacy menu discards the other bind since it seems the behavior for GetButtonCodeForBind is 1:1 and can't handle more.
I'll put in a "fix" to match the legacy menu's behavior of unbinding the other key.

@Rainyan
Copy link
Collaborator

Rainyan commented Sep 18, 2024

@Rainyan It seems like it failed because 'q' was also used for lean left. Guess that's also why the legacy menu discards the other bind since it seems the behavior for GetButtonCodeForBind is 1:1 and can't handle more. I'll put in a "fix" to match the legacy menu's behavior of unbinding the other key.

That was the other thing, but my main problem was being unable to turn off developer mode in the new menu.

@nullsystem nullsystem marked this pull request as draft September 18, 2024 21:34
@nullsystem
Copy link
Collaborator Author

@Rainyan I was able to turn it off... but then I checked again and realised the main issue wasn't fixed anyway.
I think I'll just force the legacy option to be on at all times. It's going to be odd to try to deal with the two anyway...

@nullsystem nullsystem changed the title SendMainMenuCommand so neo_toggleconsole is not tied to toggleconsole DRAFT: Enforce toggleconsole to be on all times so it only handled by root menu Sep 18, 2024
@nullsystem nullsystem marked this pull request as ready for review September 18, 2024 23:03
@nullsystem nullsystem changed the title DRAFT: Enforce toggleconsole to be on all times so it only handled by root menu Enforce toggleconsole to be on all times so root overridden key handles it itself Sep 18, 2024
@nullsystem nullsystem changed the title Enforce toggleconsole to be on all times so root overridden key handles it itself Enforce con_enable 1 so root overridden key handles it itself Sep 18, 2024
@nullsystem
Copy link
Collaborator Author

@Rainyan I've changed the description + title to reflect the way the change of the fix is done now. Basically the game will try to enforce con_enable 1 when possible and won't be dealing with the situation of using both legacy and new settings since they both collides anyway.

* Instead always enforce the "legacy" toggleconsole, because internally
  it requires con_enable to be set anyway, although the legacy menu
  toggles this.
* Basically means the support for legacy menu on this is an unsupported
  thing anyway
* Write the command file on save
* Fix attempts to write empty keys
* Unbind duplicate key
* fixes NeotokyoRebuild#586
@nullsystem nullsystem changed the title Enforce con_enable 1 so root overridden key handles it itself Enforce con_enable 1 and multiple fixes for keybind handling Sep 18, 2024
@nullsystem
Copy link
Collaborator Author

I've also gone ahead and do the duplicate keybind unbinding enforcement also.

@Rainyan
Copy link
Collaborator

Rainyan commented Sep 19, 2024

This seems to fix my previous inability to toggle the developer console setting in the new menu.

However I'm a bit unclear as to what the ` bind stuff in NeoToggleConsoleEnforce is achieving. It seems that typing: bind ` in the dev console instantly toggles the console off from under your feet while you're still typing, which feels a bit weird.

Also worth noting that the default backtick bind does not work for some keyboard layouts, for example for the nordic kb layout it's impossible to press the ` bind afaik, rendering such a bind useless for those users.

@nullsystem
Copy link
Collaborator Author

@Rainyan It's mostly to try to enforce that bind to route to the NeoToggleConsole function. I think this solution would be fine for now as a quickfix but fixing it for other layouts/trying to check console can be done as a separate PR.

@AdamTadeusz
Copy link
Contributor

Lets open an issue and merge this in then

@nullsystem
Copy link
Collaborator Author

@Rainyan I've put in a rough issue for now so it's noted: #620

@nullsystem nullsystem merged commit a51dc11 into NeotokyoRebuild:master Sep 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Minor release priority Pull request is a priority for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot enable developer console using new menu. It can only be enabled using the legacy menu.
3 participants