The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Status: Needs review => Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+me
Review: Approve
@bunnybot merge
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.
Code LGTM.
One tweak I'd like to see after testing:
Error message "The sent password was incorrect!" - call it "Wrong password" or
"Wrong password, please try again." The user doesn't care about the technical
detail that it was sent. This is coming from the metaserver, so I have opened a
but
Review: Approve testing
Ok, we may need to file a bug against the metaserver then.
This branch is ok for me now :-)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
Also, this needs to be resolved on the Metaserver side.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.
> Setting a correct password and clicking on 'Save' connects immediately to the
> lobby and immediately disconnect again. One can see this in channel #widelands
> on IRC.
>
That is currently the only way to check whether the password was correct or not.
--
Setting a correct password and clicking on 'Save' connects immediately to the
lobby and immediately disconnect again. One can see this in channel #widelands
on IRC.
But the assert is gone now :-)
--
Popup if wrong password is set works now. But i can still provoke the Assertion:
1. Open the settings
2. Enter a wrong password and close the popup
3. Click cancel
4. Try to connect with the metaserver (click on Online Game)
Running widelands with a new home directory does not show this error.
The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Commit message changed to:
redesigned login box
- renamed to Online game Settings
- limit the possible characters for usernames
- draw a red box around the input
Can you give it another try?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.
Review: Needs Fixing
Hm, there is some inconsistency and an assertion:
1. Start widelands
2. Set a wrong password -> No hint about wrong password -> Login dialog closes
3. Trying to get into the lobby shows now a warning -> Loginbox appear
4. Set a wrong password again -> now the warning appears
Buttons use Title Case, so:
Online Game
Online Game Settings
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.
The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Commit message changed to:
redesigned login box
- renamed to Online game Settings
- limit the possible characters for usernames
- draw a red box around the input
How abut calling the "Back" button "Leave Lobby" rather than "Log Out"?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
I gave it another run. The button is now called “Login settings” and sits in
between the “LAN / Direct IP” ans “Back”.[1] The Title of the Login box has be
renamed accordingly, also the “Login” button has been renamed to “Save”.[2]
The window opens automatically when no username or an invalid
The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Commit message changed to:
redesigned login box
- limit the possible characters for usernames
- draw a red box around the input field for erroneous input
- tell
Hmm, I'm still uncertain. What about this?
https://fosuta.org/pics/multi-settings.png
I have to flesh out the details, though. Bu would this Screen look okay?
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands
Continuous integration builds have changed state:
Travis build 4988. State: passed. Details:
https://travis-ci.org/widelands/widelands/builds/532926409.
Appveyor build 4769. State: success. Details:
> We rename the back button to "Leave lobby"
> Above the button we have a checkbox "Clear login data"
"Leave lobby and clear login data" would be the correct explanation. But it's
confusing anyway to have this in the lobby.
Maybe i am too nitpicking ;)
--
Well another solution would be:
We rename the back button to "Leave lobby"
Above the button we have a checkbox "Clear login data"
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to
I am not convinced by this solution, it is confusing:
1. We have two places (views) which interact with the same thing now.
2. We have two buttons, 'Logout' and 'Back', in the lobby now, which do the
same at first sight: Go back to the 'Multiplayer' view. How will a tooltip, if
there were any,
The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Commit message changed to:
redesigned login box
- limit the possible characters for usernames
- draw a red box around the input field for erroneous input
- tell
We have a logout button now :D
The login dialog will be shown only for non registered users. Registered users
may log out from their account from within the lobby.
The additional login button from the Multiplayer menu is removed.
--
I have moved username validation to internet gaming.
I'm currently experimenting with a logout button stay tuned :=)
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
That would not allow different players to use the same computer, because you
can never log out when the password is correct.
I'd say revert the changes for now and let's think about having a logout button
in the lobby.
Regarding the user name validation, this should be implemented in internet
I could think of another option. We remove the button entirely. The login
window is always shown for non registered users. When a user has entered a
password, the dialog won't be shown as long as the password is valid.
--
Continuous integration builds have changed state:
Travis build 4976. State: errored. Details:
https://travis-ci.org/widelands/widelands/builds/532515887.
Appveyor build 4757. State: success. Details:
I made the "show login dialog" button a proper button. kaputtnik has second
thoughts, though.
here is a screenshot:
https://i.ibb.co/b3W3x3W/internetgame.png
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands
Carets are also fixed.
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367320
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start.
Please test again it should address all issues. The only problem is that the
caret does not move properly. Help is appreciated. :)
@Gun have a look at the diff comment, please.
Diff comments:
>
> === modified file 'src/ui_fsmenu/multiplayer.cc'
> --- src/ui_fsmenu/multiplayer.cc
It does not solve comment #1 in the attached bug though:
> We should also add a password mode to the Editbox class that will show ***
> instead of the actual text.
So, either open a new bug before merging, or fix it in this branch.
--
Looks good, imho. Graying out the 'Password' is sufficient :-)
The green arrow on the right of 'Internet Game' is always shown now? -> I tried
to start widelands with a new homedir, and the green arrow was shown.
Don't forget to remove the 'red border' thing from the commit message please ;)
--
Continuous integration builds have changed state:
Travis build 4956. State: passed. Details:
https://travis-ci.org/widelands/widelands/builds/531407887.
Appveyor build 4737. State: success. Details:
I have pushed a small tweak and added some more comments.
Diff comments:
> === modified file 'src/ui_fsmenu/multiplayer.cc'
> --- src/ui_fsmenu/multiplayer.cc 2019-05-11 18:50:30 +
> +++ src/ui_fsmenu/multiplayer.cc 2019-05-12 11:12:04 +
> @@ -58,21 +58,46 @@
>
The proposal to merge
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands has been updated.
Status: Needs review => Superseded
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-st
Toni Förster has proposed merging
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands with lp:~widelands-dev/widelands/bug-1825932-open-games as a
prerequisite.
Commit message:
redesigned login box
- limit the possible characters for usernames
- draw a
merged
--
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367312
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into
lp:widelands
t;
https://bugs.launchpad.net/widelands/+bug/1827786
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start/+merge/367312
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/bug-1827786-
38 matches
Mail list logo
