- User Since
- Jul 2 2017, 4:22 PM (111 w, 1 d)
Jul 20 2019
I'm not entirely sure that all these changes are needed, but they seem okay. Coming from mostly writing C, it strikes me as particularly strange to modify something passed as a C++ reference. However, it doesn't seem that there's anything technically wrong with that. I assume you've tested that hosting a game and STUN still work?
Jul 19 2019
Thank you for bringing the problems with rP19703 to our attention Krinkle! It's much appreciated.
Correct the handling of sessionInitiate based off discussions with elexis on IRC yesterday.
Jul 17 2019
Remove a superfluous newline and tweak the wording
Add copyright year updates
Change the diff to be from the repository root rather than the source folder.
D2090 brought to my attention that a few outstanding comments on this were never addressed. I've annotated the remaining comments with where they were fixed and opened D2093 to handle the remaining problems.
Jul 15 2019
Why put this behind a hotkey? This seems to me like desirable default placement behavior.
Jul 16 2018
Okay, seems reasonable; we want the user to verify their identity before accepting the terms. We could ask for the user to re-accept the terms (if they've changed) on another page after the login one to improve the experience. I can look into implementing that (and maybe storing the terms acceptance state per-account on the server) if there are no objections.
There seems to be an issue where we do not always correctly persist the state of the user's acceptance on first-run.
- Reset to default config
- Open lobby, enter incorrect password, accept terms, click login
- Observe incorrect password message
- Enter correct password
- Observe requirement to re-read and accept the terms, even though you already did that during your earlier login attempt
This seems to almost always work great, and the code looks pretty good. One bug though when I tested locally:
Jan 24 2018
Changes look great at a glance! I will try to take a longer look at it and try running it locally later today.
Fantastic direction! That panel should always have automatically hidden when empty, so no need to make it configurable. Thanks for finally looking into fixing this! I did a quick run over the patch and tried to point out anything that I imagine possibly being a problem.