search

idkr-client / idkr

:video_game: idk, just a Krunker client
GNU Affero General Public License v3.0
56 stars 37 forks source link

Fix DevTools for certain operating systems and allow alternative shortcuts for navigation #62

Closed asger-finding closed 2 years ago

asger-finding commented 2 years ago

Fixes a problem where BrowserWindows would lose their WebContents under certain circumstances in electron < 13.5.0.

Adds missing shortcut navigation for Toggle Devtools, Reload and Reload Without Cache (F12, CommandOrControl+R and CommandOrControl+Shift+R respectively)

Tested in fedora 36. Remains untested in Windows 10/11.

Mixaz017 commented 2 years ago

I checked the new devToolsWebContents based detection but unfortunately it still has the issue of running the fallback code no matter what. And again, if I wait a bit (setTimeout) before running the detection, it properly detects that I have successfully opened the DevTools. Of course using setTimeout is not a reliable solution... I wonder how we should handle this. And as I suggested in Discord, I dont mind making a toggle option for fixes like this, if none of our other detection methods work.

asger-finding commented 2 years ago

Huh, that's strange. It worked for me in the WIn 10 VM. I guess it'll have to be a toggle then. I'm not really familiar with how you guys handle settings, so is that something you're up to doing?

asger-finding commented 2 years ago

Just for clarification, when you say it uses the fallback method, do you mean that it opens DevTools in a new window no matter what?

asger-finding commented 2 years ago

If you haven't already, also check that the normal method works

asger-finding commented 2 years ago

Switched to using setTimeout. It sucks, but it works. I found that it takes 280-300 ms to open devtools the normal way in my VM so I've assigned the setTimeout 500 ms.

Mixaz017 commented 2 years ago

The normal openDevTools() works for me, and by that I mean it works even with all the fallback code removed (I prefer the detached mode so mine opens in that mode). I tested both old and new detection methods but they run the fallback code even when it shouldn't. To be honest, I think this should be a toggle rather than using setTimeout. It probably would be better if the user only have to toggle the option once rather than having to wait .5 seconds for DevTools to open every time.

Mixaz017 commented 2 years ago

Sorry for a lot of requests.

asger-finding commented 2 years ago

Personally I think this is the better alternative. The timeout only applies to the small minority where this bug occurs, so its really negligible IMO. In my opinion, it's more obtrusive to have an extra setting only dedicated to those with the bug, but which can be toggled on by everyone. Either way, it's your choice, feel free to open a PR on my fork if you want any changes.

Mixaz017 commented 2 years ago

Thats a fair point. Unrelated but I think we should revert the Ctrl + R binds, they are too easy to accidentally press (and the reload key is definitely not something you want to accidentally press in this game).

asger-finding commented 2 years ago

I haven't personally have any problems with it in my client over multiple months, but sure. Maybe if some psycho has crouch bound to Ctrl. Done.

Mixaz017 commented 2 years ago

Thank you for your work on this!