flightlessmango / MangoHud

A Vulkan and OpenGL overlay for monitoring FPS, temperatures, CPU/GPU load and more. Discord: https://discordapp.com/invite/Gj5YmBb
MIT License
6.28k stars 272 forks source link

Pressing Shift_R+F9 crashes game when not displaying fps_metrics #1257

Closed jntesteves closed 5 months ago

jntesteves commented 5 months ago

Describe the bug When the fps_metrics config is not in use, pressing Shift_R+F9 crashes MangoHud and the game.

List relevant hardware/software information

To Reproduce Steps to reproduce the behavior:

  1. Make sure fps_metrics is commented out in the config file
  2. Start game with MANGOHUD=1
  3. Press Shift_R+F9
  4. Game crashes.

Expected behavior Nothing should happen when pressing Shift_R+F9 while not using fps_metrics.

Screenshots N/A

Additional context Tested mainly on Steam Flatpak. The problem was first identified on flathub/org.freedesktop.Platform.VulkanLayer.MangoHud#44

Side note: The Shift_R+F9 shortcut is currently undocumented. It's not clear if it can be changed in the config file.

Perkolator commented 5 months ago

Nothing should happen when pressing Shift_R+F9 while not using fps_metrics.

I have the same crash problem, BUT, I have toggle_fps_limit=Shift_R+F9 set in the config (and no fps_metrics), so I expect that the fps limit is toggled when pressing that key combo. Worked fine in 0.7.0 version.

flightlessmango commented 5 months ago

I can't reproduce this on latest, can you try compiling master and see if it's been fixed?

jntesteves commented 5 months ago

I can't reproduce this on latest, can you try compiling master and see if it's been fixed?

I can confirm it's been fixed by 95141de657e02577f1d3fab0ca12122bfb68bc56. I've cherry-picked this commit over 0.7.1 and that solves the issue. I don't want to ship master on the stable channel, so I think I'll push a hotfix to Flathub with just this patch added for now.

Thanks for your time looking into this.

Perkolator commented 5 months ago

I can confirm it's been fixed by 95141de. I've cherry-picked this commit over 0.7.1 and that solves the issue. I don't want to ship master on the stable channel, so I think I'll push a hotfix to Flathub with just this patch added for now.

Do you know if toggle_fps_limit=Shift_R+F9 would still work? EDIT: I mean, is that key combo now "hardcoded" only to changing the fps_metrics?

jntesteves commented 5 months ago

I can confirm it's been fixed by 95141de. I've cherry-picked this commit over 0.7.1 and that solves the issue. I don't want to ship master on the stable channel, so I think I'll push a hotfix to Flathub with just this patch added for now.

Do you know if toggle_fps_limit=Shift_R+F9 would still work? EDIT: I mean, is that key combo now "hardcoded" only to changing the fps_metrics?

There's a patched test build in flathub/org.freedesktop.Platform.VulkanLayer.MangoHud#46. You can use that with a Flatpak game to check if your use-case works.

Although, on a best case scenario, the shortcut might do 2 actions. I think it's not a good idea to do that, at least until an option is added to the config to change the shortcut for resetting fps_metrics. If this is a problem for you, you can open a new issue for that.

Perkolator commented 5 months ago

I tried the test build and with my config the Shift_R+F9 toggles fps_limit, dunno if it tries to toggle fps_metrics too, if it does, that's just bad designing/coding.

flightlessmango commented 5 months ago

Most likely it toggles both, but I don't see a problem with that behavior. If the user wants to toggle multiple things with one keybind they're allowed to. If they don't, they can rebind

Perkolator commented 5 months ago

I stand with my opinion, it's bad design. Because there are "hardcoded" key combos to toggle stuff (and a config system of "Parameters that are enabled by default have to be explicitly disabled"), it means that user needs to understand to check that their custom key combo, for whatever they use it, if it's already "hardcoded" to some function and learn to disable it. There's no documentation of this "a single hotkey can be used for multiple things" behavior and the "Keybindings" documentation is incomplete. The "variables" section (though "A partial list of parameters") has some hotkeys mentioned and that they are default. Also there doesn't seem to be any variable that would allow to disable (or change to some other hotkey) the "hardcoded" fps_metrics toggle, or is there (since you mentioned that user can rebind)? Documentation is lacking and changelogs don't seem to have all the changes listed in them (0.7.1 changelog seems to miss at least that the default frametime graph in preset 4 is changed and there's no mention that it can be changed with frame_timing_detailed (though it doesn't work for me! not the first time a setting doesn't work) in the changelog nor in the documentation, and that Shift_R+F9 is now hardcoded to some function (is this a new change or older?)). Incomplete docs/parameters list makes it hard to tame this config system.

jntesteves commented 5 months ago

Closing as this is fixed in master. Please don't use the issue tracker as a chat, there are other places for that.

Perkolator commented 5 months ago

Please don't use the issue tracker as a chat, there are other places for that.

Where? This repo doesn't have discussions section enabled. Besides, I think my writing was on point, it was an answer to flightlessmango.