Merge lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into lp:widelands

Proposed by Toni Förster
Status: Merged
Merged at revision: 9117
Proposed branch: lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start
Merge into: lp:widelands
Prerequisite: lp:~widelands-dev/widelands/bug-1825932-open-games-clean-start
Diff against target: 662 lines (+258/-127)
9 files modified
src/network/internet_gaming.cc (+11/-0)
src/network/internet_gaming.h (+2/-0)
src/ui_basic/editbox.cc (+46/-18)
src/ui_basic/editbox.h (+10/-0)
src/ui_fsmenu/internet_lobby.cc (+3/-2)
src/ui_fsmenu/multiplayer.cc (+39/-62)
src/ui_fsmenu/multiplayer.h (+2/-2)
src/wui/login_box.cc (+136/-38)
src/wui/login_box.h (+9/-5)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start
Reviewer Review Type Date Requested Status
GunChleoc Approve
kaputtnik (community) testing Approve
Review via email: mp+367320@code.launchpad.net

This proposal supersedes a proposal from 2019-05-12.

Commit message

redesigned login box

- renamed to Online game Settings
- limit the possible characters for usernames
- draw a red box around the input field for erroneous input
- tell user were to register their username
- clicking registered checkbox focuses password field
- remove check from registered_ clears password field
- password field is only accessible when checkbox is clicked
- when a password is set, ***** is shown on opening
- login box handles the settings instead of multiplayer

multiplayer login redesign

- only show login dialog when no name or invalid name is set
- Group of 2 buttons for Online Game & Settings
- renamed Internet game to Online game
- moved settings saving to login box

To post a comment you must log in.
Revision history for this message
kaputtnik (franku) wrote : Posted in a previous version of this proposal

It would be good to have the registered checkbox above the password field. It would be a more logical ordering, imho.

Revision history for this message
Toni Förster (stonerl) wrote : Posted in a previous version of this proposal

Your're right. Chnaged it.

Revision history for this message
bunnybot (widelandsofficial) wrote : Posted in a previous version of this proposal

Continuous integration builds have changed state:

Travis build 4915. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/529968856.
Appveyor build 4696. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box-4696.

Revision history for this message
GunChleoc (gunchleoc) : Posted in a previous version of this proposal
Revision history for this message
Toni Förster (stonerl) wrote : Posted in a previous version of this proposal

Will address the strings later. For the others see the diff comments.

Revision history for this message
bunnybot (widelandsofficial) wrote : Posted in a previous version of this proposal

Continuous integration builds have changed state:

Travis build 4925. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/530433164.
Appveyor build 4706. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box-4706.

Revision history for this message
kaputtnik (franku) wrote : Posted in a previous version of this proposal

As talked on IRC: Can you try to remove the red border around the password editbox and instead show the string 'Password' and the corresponding editbox look sort of grayed out?

I think the text below can also be improved: "You need an account on the widelands website to use a registered account. Please visit ...."

Revision history for this message
GunChleoc (gunchleoc) wrote : Posted in a previous version of this proposal

Changing editbox styles is implemented in https://code.launchpad.net/~widelands-dev/widelands/font_size-lua/+merge/366938, so we really need somebody to review that branch and make Toni's life easier.

Revision history for this message
GunChleoc (gunchleoc) wrote : Posted in a previous version of this proposal

The changes string is missing a . after the %

I'll do some testing before I reply to the remaining diff comments.

Revision history for this message
Toni Förster (stonerl) wrote : Posted in a previous version of this proposal

@kaputtnik

I addressed your comments. The only thing I can't do ATM is, greying out the password field.

Revision history for this message
Toni Förster (stonerl) wrote : Posted in a previous version of this proposal

But I'm not confident whether we should grey the password field out as well. I would look too similar to an active edit box. Greying out the text "Password" and making the input box not editable should be sufficient, IMHO.

Revision history for this message
Toni Förster (stonerl) wrote : Posted in a previous version of this proposal
Revision history for this message
GunChleoc (gunchleoc) wrote :

I have pushed a small tweak and added some more comments.

Revision history for this message
bunnybot (widelandsofficial) wrote :

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: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box_clean_start-4737.

Revision history for this message
kaputtnik (franku) wrote :

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 ;)

Revision history for this message
GunChleoc (gunchleoc) wrote :

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.

Revision history for this message
Toni Förster (stonerl) wrote :

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.

Revision history for this message
Toni Förster (stonerl) wrote :

Carets are also fixed.

Revision history for this message
Toni Förster (stonerl) wrote :

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

Revision history for this message
bunnybot (widelandsofficial) wrote :

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: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box_clean_start-4757.

Revision history for this message
Toni Förster (stonerl) wrote :

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.

Revision history for this message
GunChleoc (gunchleoc) wrote :

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 gaming somewhere, not in editbox. The editbox is a UI element and should not care about what a well-formed user name is. That way, you could also get rid of the remaining code duplication.

I have tested the password display and it's working fine :)

Revision history for this message
Toni Förster (stonerl) wrote :

I have moved username validation to internet gaming.

I'm currently experimenting with a logout button stay tuned :=)

Revision history for this message
Toni Förster (stonerl) wrote :

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.

Revision history for this message
kaputtnik (franku) wrote :

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, look to explain the buttons?

Having it like before is better, imho, although the small icon (button) to show the loginbox is visually disturbing. But better a visually disturbing thing than a confusing UI, especially because the Multiplayer view is mostly open for a short time.

Just my personal opinion :)

Revision history for this message
Toni Förster (stonerl) wrote :

Well another solution would be:

We rename the back button to "Leave lobby"
Above the button we have a checkbox "Clear login data"

Revision history for this message
kaputtnik (franku) wrote :

> 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 ;)

Revision history for this message
bunnybot (widelandsofficial) wrote :

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: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827786_metaserver_login_box_clean_start-4769.

Revision history for this message
Toni Förster (stonerl) wrote :

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?

Revision history for this message
Toni Förster (stonerl) wrote :

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 username is set or the wrong password has been entered. Otherwise, the user has to click the button. Clicking on “Save” in the box only closes the window but does not login automatically.

[1]https://fosuta.org/pics/multi-new.png
[2]https://fosuta.org/pics/login-new.png

Revision history for this message
kaputtnik (franku) wrote :

Ordering the buttons like here https://i.ibb.co/b3W3x3W/internetgame.png is more logical, imho, because the 'Login settings' belongs to 'Internet Game'. And maybe rename 'Login Settings' to 'Internet Settings'? So a user can directly associate that the two buttons belongs to the same thing.

All the rest is fine for me :-)

Revision history for this message
GunChleoc (gunchleoc) wrote :

How abut calling the "Back" button "Leave Lobby" rather than "Log Out"?

Revision history for this message
Toni Förster (stonerl) wrote :

I renamed the back button

Reorder button on the multiplayer screen.

I took the liberty to rename “Internet game” because this is the common term nowadays (e.g. Wikipedia redirects form Internet game to Online game). I also renamed the settings button to “Online game Settings”

@Gun is it:

1. Online game settings
2. Online game Settings
2. Online Game Settings

I went for number 2.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Buttons use Title Case, so:

Online Game
Online Game Settings

Revision history for this message
kaputtnik (franku) wrote :

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 immediately
5. Close dialog by pressing 'Cancel' (did not test right click to close it)
6. Try to enter the lobby -> Assertion:

widelands: /home/kaputtnik/Quellcode/widelands-repo/bug-1827786-metaserver-login-box-clean-start/src/ui_fsmenu/multiplayer.cc:134: void FullscreenMenuMultiPlayer::internet_login(): Assertion `!auth.empty()' failed.

If feasible the warning about wrong password should appear also in step 2.

I am fine with the wording though :-)

review: Needs Fixing
Revision history for this message
Toni Förster (stonerl) wrote :

Can you give it another try?

Revision history for this message
kaputtnik (franku) wrote :

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.

Revision history for this message
kaputtnik (franku) wrote :

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 :-)

Revision history for this message
Toni Förster (stonerl) wrote :

> 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.

Revision history for this message
Toni Förster (stonerl) wrote :

Also, this needs to be resolved on the Metaserver side.

Revision history for this message
kaputtnik (franku) wrote :

Ok, we may need to file a bug against the metaserver then.

This branch is ok for me now :-)

review: Approve (testing)
Revision history for this message
GunChleoc (gunchleoc) wrote :

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 over there. https://github.com/widelands/widelands_metaserver/issues/53

Revision history for this message
GunChleoc (gunchleoc) wrote :

@bunnybot merge

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/network/internet_gaming.cc'
--- src/network/internet_gaming.cc 2019-05-13 02:25:58 +0000
+++ src/network/internet_gaming.cc 2019-05-23 17:11:42 +0000
@@ -947,3 +947,14 @@
947 ingame_system_chat_.push_back(c);947 ingame_system_chat_.push_back(c);
948 }948 }
949}949}
950
951/**
952 * Check for vaild username characters.
953 */
954bool InternetGaming::valid_username(std::string username) {
955 if (username.empty() || username.find_first_not_of("abcdefghijklmnopqrstuvwxyz"
956 "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890@.+-_") <= username.size()) {
957 return false;
958 }
959 return true;
960}
950961
=== modified file 'src/network/internet_gaming.h'
--- src/network/internet_gaming.h 2019-05-11 22:02:09 +0000
+++ src/network/internet_gaming.h 2019-05-23 17:11:42 +0000
@@ -183,6 +183,8 @@
183 bool system,183 bool system,
184 const std::string& msg);184 const std::string& msg);
185185
186 bool valid_username(std::string);
187
186private:188private:
187 InternetGaming();189 InternetGaming();
188190
189191
=== modified file 'src/ui_basic/editbox.cc'
--- src/ui_basic/editbox.cc 2019-05-13 02:25:58 +0000
+++ src/ui_basic/editbox.cc 2019-05-23 17:11:42 +0000
@@ -19,7 +19,9 @@
1919
20#include "ui_basic/editbox.h"20#include "ui_basic/editbox.h"
2121
22#include <algorithm>
22#include <limits>23#include <limits>
24#include <string>
2325
24#include <SDL_keycode.h>26#include <SDL_keycode.h>
25#include <boost/format.hpp>27#include <boost/format.hpp>
@@ -86,7 +88,8 @@
86 m_(new EditBoxImpl),88 m_(new EditBoxImpl),
87 history_active_(false),89 history_active_(false),
88 history_position_(-1),90 history_position_(-1),
89 warning_(false) {91 password_(false),
92 warning_(false) {
90 set_thinks(false);93 set_thinks(false);
9194
92 m_->background_style = g_gr->styles().editbox_style(style);95 m_->background_style = g_gr->styles().editbox_style(style);
@@ -388,7 +391,8 @@
388 const int max_width = get_w() - 2 * kMarginX;391 const int max_width = get_w() - 2 * kMarginX;
389392
390 std::shared_ptr<const UI::RenderedText> rendered_text =393 std::shared_ptr<const UI::RenderedText> rendered_text =
391 UI::g_fh->render(as_editorfont(m_->text, m_->fontsize));394 UI::g_fh->render(as_editorfont(m_->text, m_->fontsize));
395
392396
393 const int linewidth = rendered_text->width();397 const int linewidth = rendered_text->width();
394 const int lineheight = m_->text.empty() ? text_height(m_->fontsize) : rendered_text->height();398 const int lineheight = m_->text.empty() ? text_height(m_->fontsize) : rendered_text->height();
@@ -400,31 +404,44 @@
400 UI::center_vertically(lineheight, &point);404 UI::center_vertically(lineheight, &point);
401405
402 // Crop to max_width while blitting406 // Crop to max_width while blitting
403 if (max_width < linewidth) {407 if (!password_) {
404 // Fix positioning for BiDi languages.408 if (max_width < linewidth) {
405 if (UI::g_fh->fontset()->is_rtl()) {409 // Fix positioning for BiDi languages.
406 point.x = 0.f;410 if (UI::g_fh->fontset()->is_rtl()) {
407 }411 point.x = 0.f;
408 // We want this always on, e.g. for mixed language savegame filenames412 }
409 if (i18n::has_rtl_character(m_->text.c_str(), 100)) { // Restrict check for efficiency413 // We want this always on, e.g. for mixed language savegame filenames
410 // TODO(GunChleoc): Arabic: Fix scrolloffset414 if (i18n::has_rtl_character(m_->text.c_str(), 100)) { // Restrict check for efficiency
411 rendered_text->draw(dst, point, Recti(linewidth - max_width, 0, linewidth, lineheight));
412 } else {
413 if (m_->align == UI::Align::kRight) {
414 // TODO(GunChleoc): Arabic: Fix scrolloffset415 // TODO(GunChleoc): Arabic: Fix scrolloffset
415 rendered_text->draw(416 rendered_text->draw(dst, point, Recti(linewidth - max_width, 0, linewidth, lineheight));
416 dst, point, Recti(point.x + m_->scrolloffset + kMarginX, 0, max_width, lineheight));
417 } else {417 } else {
418 rendered_text->draw(dst, point, Recti(-m_->scrolloffset, 0, max_width, lineheight));418 if (m_->align == UI::Align::kRight) {
419 // TODO(GunChleoc): Arabic: Fix scrolloffset
420 rendered_text->draw(
421 dst, point, Recti(point.x + m_->scrolloffset + kMarginX, 0, max_width, lineheight));
422 } else {
423 rendered_text->draw(dst, point, Recti(-m_->scrolloffset, 0, max_width, lineheight));
424 }
419 }425 }
426 } else {
427 rendered_text->draw(dst, point, Recti(0, 0, max_width, lineheight));
420 }428 }
421 } else {429 } else {
422 rendered_text->draw(dst, point, Recti(0, 0, max_width, lineheight));430 std::shared_ptr<const UI::RenderedText> password_text =
431 UI::g_fh->render(as_editorfont(text_to_asterisk(), m_->fontsize));
432 password_text->draw(dst, point, Recti(0, 0, max_width, lineheight));
423 }433 }
424434
425 if (has_focus()) {435 if (has_focus()) {
426 // Draw the caret436 // Draw the caret
427 std::string line_to_caret = m_->text.substr(0, m_->caret);437 std::string line_to_caret;
438
439 if (password_) {
440 line_to_caret = text_to_asterisk().substr(0, m_->caret);
441 } else {
442 line_to_caret = m_->text.substr(0, m_->caret);
443 }
444
428 // TODO(GunChleoc): Arabic: Fix cursor position for BIDI text.445 // TODO(GunChleoc): Arabic: Fix cursor position for BIDI text.
429 int caret_x = text_width(line_to_caret, m_->fontsize);446 int caret_x = text_width(line_to_caret, m_->fontsize);
430447
@@ -471,4 +488,15 @@
471 m_->scrolloffset = 0;488 m_->scrolloffset = 0;
472 }489 }
473}490}
491
492/**
493 * Return text as asterisks.
494 */
495std::string EditBox::text_to_asterisk() {
496 std::string asterisk;
497 for (int i = 0; i < int (m_->text.size()); i++) {
498 asterisk.append("*");
499 }
500 return asterisk;
501}
474} // namespace UI502} // namespace UI
475503
=== modified file 'src/ui_basic/editbox.h'
--- src/ui_basic/editbox.h 2019-05-11 22:02:09 +0000
+++ src/ui_basic/editbox.h 2019-05-23 17:11:42 +0000
@@ -72,18 +72,28 @@
7272
73 void draw(RenderTarget&) override;73 void draw(RenderTarget&) override;
7474
75 void set_password(bool pass) {
76 password_ = pass;
77 }
78
75 void set_warning(bool warn) {79 void set_warning(bool warn) {
76 warning_ = warn;80 warning_ = warn;
77 }81 }
7882
83 bool is_password() {
84 return password_;
85 }
86
79private:87private:
80 std::unique_ptr<EditBoxImpl> m_;88 std::unique_ptr<EditBoxImpl> m_;
8189
82 void check_caret();90 void check_caret();
91 std::string text_to_asterisk();
8392
84 bool history_active_;93 bool history_active_;
85 int16_t history_position_;94 int16_t history_position_;
86 std::string history_[CHAT_HISTORY_SIZE];95 std::string history_[CHAT_HISTORY_SIZE];
96 bool password_;
87 bool warning_;97 bool warning_;
88};98};
89} // namespace UI99} // namespace UI
90100
=== modified file 'src/ui_fsmenu/internet_lobby.cc'
--- src/ui_fsmenu/internet_lobby.cc 2019-05-13 02:25:58 +0000
+++ src/ui_fsmenu/internet_lobby.cc 2019-05-23 17:11:42 +0000
@@ -78,7 +78,7 @@
78 hostgame_(this,78 hostgame_(this,
79 "host_game",79 "host_game",
80 get_w() * 17 / 25,80 get_w() * 17 / 25,
81 get_h() * 81 / 100,81 get_h() * 73 / 100,
82 butw_,82 butw_,
83 buth_,83 buth_,
84 UI::ButtonStyle::kFsMenuSecondary,84 UI::ButtonStyle::kFsMenuSecondary,
@@ -90,7 +90,7 @@
90 butw_,90 butw_,
91 buth_,91 buth_,
92 UI::ButtonStyle::kFsMenuSecondary,92 UI::ButtonStyle::kFsMenuSecondary,
93 _("Back")),93 _("Leave Lobby")),
9494
95 // Edit boxes95 // Edit boxes
96 edit_servername_(96 edit_servername_(
@@ -165,6 +165,7 @@
165165
166 // set focus to chat input166 // set focus to chat input
167 chat.focus_edit();167 chat.focus_edit();
168
168}169}
169170
170void FullscreenMenuInternetLobby::layout() {171void FullscreenMenuInternetLobby::layout() {
171172
=== modified file 'src/ui_fsmenu/multiplayer.cc'
--- src/ui_fsmenu/multiplayer.cc 2019-05-11 18:50:30 +0000
+++ src/ui_fsmenu/multiplayer.cc 2019-05-23 17:11:42 +0000
@@ -21,7 +21,6 @@
2121
22#include "base/i18n.h"22#include "base/i18n.h"
23#include "graphic/text_constants.h"23#include "graphic/text_constants.h"
24#include "network/crypto.h"
25#include "network/internet_gaming.h"24#include "network/internet_gaming.h"
26#include "profile/profile.h"25#include "profile/profile.h"
27#include "random/random.h"26#include "random/random.h"
@@ -36,9 +35,10 @@
3635
37 // Buttons36 // Buttons
38 metaserver(37 metaserver(
39 &vbox_, "metaserver", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Internet game")),38 &vbox_, "metaserver", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Online Game")),
40 showloginbox(nullptr),
41 lan(&vbox_, "lan", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("LAN / Direct IP")),39 lan(&vbox_, "lan", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("LAN / Direct IP")),
40 showloginbox(
41 &vbox_, "lan", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Online Game Settings")),
42 back(&vbox_, "back", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Back")) {42 back(&vbox_, "back", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Back")) {
43 metaserver.sigclicked.connect(43 metaserver.sigclicked.connect(
44 boost::bind(&FullscreenMenuMultiPlayer::internet_login, boost::ref(*this)));44 boost::bind(&FullscreenMenuMultiPlayer::internet_login, boost::ref(*this)));
@@ -47,6 +47,9 @@
47 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,47 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,
48 boost::ref(*this), FullscreenMenuBase::MenuTarget::kLan));48 boost::ref(*this), FullscreenMenuBase::MenuTarget::kLan));
4949
50 showloginbox.sigclicked.connect(
51 boost::bind(&FullscreenMenuMultiPlayer::show_internet_login, boost::ref(*this)));
52
50 back.sigclicked.connect(53 back.sigclicked.connect(
51 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,54 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,
52 boost::ref(*this), FullscreenMenuBase::MenuTarget::kBack));55 boost::ref(*this), FullscreenMenuBase::MenuTarget::kBack));
@@ -54,73 +57,50 @@
54 title.set_fontsize(fs_big());57 title.set_fontsize(fs_big());
5558
56 vbox_.add(&metaserver, UI::Box::Resizing::kFullSize);59 vbox_.add(&metaserver, UI::Box::Resizing::kFullSize);
60 vbox_.add(&showloginbox, UI::Box::Resizing::kFullSize);
61 vbox_.add_inf_space();
57 vbox_.add(&lan, UI::Box::Resizing::kFullSize);62 vbox_.add(&lan, UI::Box::Resizing::kFullSize);
58 vbox_.add_inf_space();63 vbox_.add_inf_space();
64 vbox_.add_inf_space();
65 vbox_.add_inf_space();
59 vbox_.add(&back, UI::Box::Resizing::kFullSize);66 vbox_.add(&back, UI::Box::Resizing::kFullSize);
6067
61 Section& s = g_options.pull_section("global");68 auto_log_ = false;
62 auto_log_ = s.get_bool("auto_log", false);
63 if (auto_log_) {
64 showloginbox =
65 new UI::Button(this, "login_dialog", 0, 0, 0, 0, UI::ButtonStyle::kFsMenuSecondary,
66 g_gr->images().get("images/ui_basic/continue.png"), _("Show login dialog"));
67 showloginbox->sigclicked.connect(
68 boost::bind(&FullscreenMenuMultiPlayer::show_internet_login, boost::ref(*this)));
69 }
70 layout();69 layout();
71}70}
7271
73/// called if the showloginbox button was pressed72/// called if the user is not registered
74void FullscreenMenuMultiPlayer::show_internet_login() {73void FullscreenMenuMultiPlayer::show_internet_login() {
75 auto_log_ = false;74 LoginBox lb(*this);
76 internet_login();75 if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk && auto_log_) {
76 auto_log_ = false;
77 internet_login();
78 }
77}79}
7880
79/**81/**
80 * Called if "Internet" button was pressed.82 * Called if "Online Game" button was pressed.
81 *83 *
82 * IF autologin is not set, a LoginBox is shown and, if the user clicks on84 * IF no nickname or a nickname with invalid characters is set, the Online Game Settings
83 * 'login' in it's menu, the data is read from the LoginBox and saved in 'this'85 * are opened.
84 * so wlapplication can read it.86 *
85 *87 * IF at least a name is set, all data is read from the config file
86 * IF autologin is set, all data is read from the config file and saved.88 *
87 * That data will be used for login to the metaserver.89 * This fullscreen menu ends it's modality.
88 *
89 * In both cases this fullscreen menu ends it's modality.
90 */90 */
91void FullscreenMenuMultiPlayer::internet_login() {91void FullscreenMenuMultiPlayer::internet_login() {
92 Section& s = g_options.pull_section("global");92 Section& s = g_options.pull_section("global");
93 if (auto_log_) {93
94 nickname_ = s.get_string("nickname", _("nobody"));94 nickname_ = s.get_string("nickname", "");
95 password_ = s.get_string("password_sha1", "nobody");95 password_ = s.get_string("password_sha1", "no_password_set");
96 register_ = s.get_bool("registered", false);96 register_ = s.get_bool("registered", false);
97 } else {97
98 LoginBox lb(*this);98 // Checks can be done directly in editbox' by using valid_username().
99 if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {99 // This is just to be on the safe side, in case the user changed the password in the config file.
100 nickname_ = lb.get_nickname();100 if (!InternetGaming::ref().valid_username(nickname_)) {
101 /// NOTE: The password is only stored (in memory and on disk) and transmitted (over the101 auto_log_ = true;
102 /// network102 show_internet_login();
103 /// to the metaserver) as cryptographic hash. This does NOT mean that the password is103 return;
104 /// stored
105 /// securely on the local disk. While the password should be secure while transmitted to
106 /// the
107 /// metaserver (no-one can use the transmitted data to log in as the user) this is not the
108 /// case
109 /// for local storage. The stored hash of the password makes it hard to look at the
110 /// configuration
111 /// file and figure out the plaintext password to, e.g., log in on the forum. However, the
112 /// stored hash can be copied to another system and used to log in as the user on the
113 /// metaserver.
114 // Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
115 // passwords on the server are protected by SHA-1 we have to use it here, too
116 password_ = crypto::sha1(lb.get_password());
117 register_ = lb.registered();
118
119 s.set_bool("registered", lb.registered());
120 s.set_bool("auto_log", lb.set_automaticlog());
121 } else {
122 return;
123 }
124 }104 }
125105
126 // Try to connect to the metaserver106 // Try to connect to the metaserver
@@ -141,7 +121,8 @@
141121
142 // Reset InternetGaming and passwort and show internet login again122 // Reset InternetGaming and passwort and show internet login again
143 InternetGaming::ref().reset();123 InternetGaming::ref().reset();
144 s.set_string("password_sha1", "");124 s.set_string("password_sha1", "no_password_set");
125 auto_log_ = true;
145 show_internet_login();126 show_internet_login();
146 }127 }
147}128}
@@ -156,12 +137,8 @@
156137
157 title.set_pos(Vector2i(0, title_y_));138 title.set_pos(Vector2i(0, title_y_));
158139
159 metaserver.set_size(butw_, buth_);
160 if (showloginbox) {
161 showloginbox->set_pos(Vector2i(box_x_ + butw_ + padding_ / 2, box_y_));
162 showloginbox->set_size(buth_, buth_);
163 }
164 metaserver.set_desired_size(butw_, buth_);140 metaserver.set_desired_size(butw_, buth_);
141 showloginbox.set_desired_size(butw_, buth_);
165 lan.set_desired_size(butw_, buth_);142 lan.set_desired_size(butw_, buth_);
166 back.set_desired_size(butw_, buth_);143 back.set_desired_size(butw_, buth_);
167144
168145
=== modified file 'src/ui_fsmenu/multiplayer.h'
--- src/ui_fsmenu/multiplayer.h 2019-05-11 18:50:30 +0000
+++ src/ui_fsmenu/multiplayer.h 2019-05-23 17:11:42 +0000
@@ -53,15 +53,15 @@
5353
54 UI::Textarea title;54 UI::Textarea title;
55 UI::Button metaserver;55 UI::Button metaserver;
56 UI::Button* showloginbox;
57 UI::Button lan;56 UI::Button lan;
57 UI::Button showloginbox;
58 UI::Button back;58 UI::Button back;
5959
60 // Values from internet login window60 // Values from internet login window
61 std::string nickname_;61 std::string nickname_;
62 std::string password_;62 std::string password_;
63 bool auto_log_;
63 bool register_;64 bool register_;
64 bool auto_log_;
65};65};
6666
67#endif // end of include guard: WL_UI_FSMENU_MULTIPLAYER_H67#endif // end of include guard: WL_UI_FSMENU_MULTIPLAYER_H
6868
=== modified file 'src/wui/login_box.cc'
--- src/wui/login_box.cc 2019-05-11 18:50:30 +0000
+++ src/wui/login_box.cc 2019-05-23 17:11:42 +0000
@@ -21,75 +21,89 @@
2121
22#include "base/i18n.h"22#include "base/i18n.h"
23#include "graphic/font_handler.h"23#include "graphic/font_handler.h"
24#include "network/crypto.h"
25#include "network/internet_gaming.h"
24#include "profile/profile.h"26#include "profile/profile.h"
25#include "ui_basic/button.h"27#include "ui_basic/button.h"
26#include "ui_basic/messagebox.h"28#include "ui_basic/messagebox.h"
2729
28LoginBox::LoginBox(Panel& parent)30LoginBox::LoginBox(Panel& parent)
29 : Window(&parent, "login_box", 0, 0, 500, 220, _("Metaserver login")) {31 : Window(&parent, "login_box", 0, 0, 500, 280, _("Online Game Settings")) {
30 center_to_parent();32 center_to_parent();
3133
32 int32_t margin = 10;34 int32_t margin = 10;
3335
34 ta_nickname = new UI::Textarea(this, margin, margin, _("Nickname:"));36 ta_nickname = new UI::Textarea(this, margin, margin, _("Nickname:"));
35 ta_password = new UI::Textarea(this, margin, 40, _("Password:"));37 ta_password = new UI::Textarea(this, margin, 70, _("Password:"));
36 eb_nickname = new UI::EditBox(this, 150, margin, 330, 20, 2, UI::PanelStyle::kWui);38 eb_nickname = new UI::EditBox(this, 150, margin, 330, 20, 2, UI::PanelStyle::kWui);
37 eb_password = new UI::EditBox(this, 150, 40, 330, 20, 2, UI::PanelStyle::kWui);39 eb_password = new UI::EditBox(this, 150, 70, 330, 20, 2, UI::PanelStyle::kWui);
3840
39 pwd_warning =41 cb_register = new UI::Checkbox(this, Vector2i(margin, 40), _("Log in to a registered account."),
40 new UI::MultilineTextarea(this, margin, 65, 505, 50, UI::PanelStyle::kWui,
41 _("WARNING: Password will be shown and saved readable!"));
42
43 cb_register = new UI::Checkbox(this, Vector2i(margin, 110), _("Log in to a registered account"),
44 "", get_inner_w() - 2 * margin);42 "", get_inner_w() - 2 * margin);
45 cb_auto_log = new UI::Checkbox(this, Vector2i(margin, 135),43
46 _("Automatically use this login information from now on."), "",44 register_account = new UI::MultilineTextarea(this, margin, 105, 470, 140, UI::PanelStyle::kWui,
47 get_inner_w() - 2 * margin);45 (boost::format(_("In order to use a registered "
4846 "account, you need an account on the widelands website. "
49 UI::Button* loginbtn = new UI::Button(47 "Please log in at %s and set an online "
48 "gaming password on your profile page."))
49 % "\n\nhttps://widelands.org/accounts/register/\n\n").str());
50
51 loginbtn = new UI::Button(
50 this, "login",52 this, "login",
51 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 :53 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 :
52 (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2,54 (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2,
53 get_inner_h() - 20 - margin, 200, 20, UI::ButtonStyle::kWuiPrimary, _("Login"));55 get_inner_h() - 20 - margin, 200, 20, UI::ButtonStyle::kWuiPrimary, _("Save"));
54 loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));56
55 UI::Button* cancelbtn = new UI::Button(57 cancelbtn = new UI::Button(
56 this, "cancel",58 this, "cancel",
57 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2 :59 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2 :
58 (get_inner_w() / 2 - 200) / 2,60 (get_inner_w() / 2 - 200) / 2,
59 loginbtn->get_y(), 200, 20, UI::ButtonStyle::kWuiSecondary, _("Cancel"));61 loginbtn->get_y(), 200, 20, UI::ButtonStyle::kWuiSecondary, _("Cancel"));
62
63 loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));
60 cancelbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));64 cancelbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));
65 eb_nickname->changed.connect(boost::bind(&LoginBox::change_playername, this));
66 cb_register->clickedto.connect(boost::bind(&LoginBox::clicked_register, this));
6167
62 Section& s = g_options.pull_section("global");68 Section& s = g_options.pull_section("global");
63 eb_nickname->set_text(s.get_string("nickname", _("nobody")));69 eb_nickname->set_text(s.get_string("nickname", _("nobody")));
64 cb_register->set_state(s.get_bool("registered", false));70 cb_register->set_state(s.get_bool("registered", false));
71 eb_password->set_password(true);
72
73 if (registered()) {
74 eb_password->set_text(s.get_string("password_sha1", ""));
75 loginbtn->set_enabled(false);
76 } else {
77 eb_password->set_can_focus(false);
78 ta_password->set_color(UI_FONT_CLR_DISABLED);
79 }
80
65 eb_nickname->focus();81 eb_nickname->focus();
82
83}
84
85/// think function of the UI (main loop)
86void LoginBox::think() {
87 verify_input();
66}88}
6789
68/**90/**
69 * called, if "login" is pressed.91 * called, if "login" is pressed.
70 */92 */
71void LoginBox::clicked_ok() {93void LoginBox::clicked_ok() {
72 // Check if all needed input fields are valid94 Section& s = g_options.pull_section("global");
73 if (eb_nickname->text().empty()) {95 if (cb_register->get_state()) {
74 UI::WLMessageBox mb(96 if (check_password()) {
75 this, _("Empty Nickname"), _("Please enter a nickname!"), UI::WLMessageBox::MBoxType::kOk);97 s.set_string("nickname", eb_nickname->text());
76 mb.run<UI::Panel::Returncodes>();98 s.set_bool("registered", true);
77 return;99 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
78 }100 }
79 if (eb_nickname->text().find(' ') <= eb_nickname->text().size()) {101 } else {
80 UI::WLMessageBox mb(this, _("Space in Nickname"),102 s.set_string("nickname", eb_nickname->text());
81 _("Sorry, but spaces are not allowed in nicknames!"),103 s.set_bool("registered", false);
82 UI::WLMessageBox::MBoxType::kOk);104 s.set_string("password_sha1", "");
83 mb.run<UI::Panel::Returncodes>();105 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
84 return;106 }
85 }
86 if (eb_password->text().empty() && cb_register->get_state()) {
87 UI::WLMessageBox mb(this, _("Empty Password"), _("Please enter your password!"),
88 UI::WLMessageBox::MBoxType::kOk);
89 mb.run<UI::Panel::Returncodes>();
90 return;
91 }
92 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
93}107}
94108
95/// Called if "cancel" was pressed109/// Called if "cancel" was pressed
@@ -97,6 +111,13 @@
97 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);111 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
98}112}
99113
114/// Called when nickname was changed
115void LoginBox::change_playername() {
116 cb_register->set_state(false);
117 eb_password->set_can_focus(false);
118 eb_password->set_text("");
119}
120
100bool LoginBox::handle_key(bool down, SDL_Keysym code) {121bool LoginBox::handle_key(bool down, SDL_Keysym code) {
101 if (down) {122 if (down) {
102 switch (code.sym) {123 switch (code.sym) {
@@ -113,3 +134,80 @@
113 }134 }
114 return UI::Panel::handle_key(down, code);135 return UI::Panel::handle_key(down, code);
115}136}
137
138void LoginBox::clicked_register() {
139 if (cb_register->get_state()) {
140 ta_password->set_color(UI_FONT_CLR_DISABLED);
141 eb_password->set_can_focus(false);
142 eb_password->set_text("");
143 } else {
144 ta_password->set_color(UI_FONT_CLR_FG);
145 eb_password->set_can_focus(true);
146 }
147}
148
149void LoginBox::verify_input() {
150 // Check if all neccessary input fields are valid
151 loginbtn->set_enabled(true);
152 eb_nickname->set_tooltip("");
153 eb_password->set_tooltip("");
154 eb_nickname->set_warning(false);
155
156 if (eb_nickname->text().empty()) {
157 eb_nickname->set_warning(true);
158 eb_nickname->set_tooltip(_("Please enter a nickname!"));
159 loginbtn->set_enabled(false);
160 } else if (!InternetGaming::ref().valid_username(eb_nickname->text())) {
161 eb_nickname->set_warning(true);
162 eb_nickname->set_tooltip(_("Enter a valid nickname. This value may contain only "
163 "English letters, numbers, and @ . + - _ characters."));
164 loginbtn->set_enabled(false);
165 }
166
167 if (eb_password->text().empty() && cb_register->get_state()) {
168 eb_password->set_tooltip(_("Please enter your password!"));
169 loginbtn->set_enabled(false);
170 eb_password->focus();
171 }
172
173 Section& s = g_options.pull_section("global");
174 if (eb_password->has_focus() && eb_password->text() == s.get_string("password_sha1", "")) {
175 eb_password->set_text("");
176 }
177
178 if (cb_register->get_state() && eb_password->text() == s.get_string("password_sha1", "")) {
179 loginbtn->set_enabled(false);
180 }
181}
182
183/// Check password against metaserver
184bool LoginBox::check_password() {
185 // Try to connect to the metaserver
186 Section& s = g_options.pull_section("global");
187 const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());
188 uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);
189 std::string password = crypto::sha1(eb_password->text());
190 InternetGaming::ref().login(get_nickname(), password, true, meta, port);
191
192 if (!InternetGaming::ref().logged_in()) {
193 // something went wrong -> show the error message
194 // idealy it is about the wrong password
195 ChatMessage msg = InternetGaming::ref().get_messages().back();
196 UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);
197 wmb.run<UI::Panel::Returncodes>();
198 InternetGaming::ref().reset();
199 eb_password->set_text("");
200 eb_password->focus();
201 return false;
202 }
203 // NOTE: The password is only stored (in memory and on disk) and transmitted (over the network to the metaserver) as cryptographic hash.
204 // This does NOT mean that the password is stored securely on the local disk.
205 // While the password should be secure while transmitted to the metaserver (no-one can use the transmitted data to log in as the user) this is not the case for local storage.
206 // The stored hash of the password makes it hard to look at the configuration file and figure out the plaintext password to, e.g., log in on the forum.
207 // However, the stored hash can be copied to another system and used to log in as the user on the metaserver.
208 // Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
209 // passwords on the server are protected by SHA-1 we have to use it here, too
210 s.set_string("password_sha1", password);
211 InternetGaming::ref().logout();
212 return true;
213}
116214
=== modified file 'src/wui/login_box.h'
--- src/wui/login_box.h 2019-05-11 18:50:30 +0000
+++ src/wui/login_box.h 2019-05-23 17:11:42 +0000
@@ -29,6 +29,8 @@
29struct LoginBox : public UI::Window {29struct LoginBox : public UI::Window {
30 explicit LoginBox(UI::Panel&);30 explicit LoginBox(UI::Panel&);
3131
32 void think() override;
33
32 std::string get_nickname() {34 std::string get_nickname() {
33 return eb_nickname->text();35 return eb_nickname->text();
34 }36 }
@@ -38,24 +40,26 @@
38 bool registered() {40 bool registered() {
39 return cb_register->get_state();41 return cb_register->get_state();
40 }42 }
41 bool set_automaticlog() {
42 return cb_auto_log->get_state();
43 }
4443
45 /// Handle keypresses44 /// Handle keypresses
46 bool handle_key(bool down, SDL_Keysym code) override;45 bool handle_key(bool down, SDL_Keysym code) override;
4746
48private:47private:
48 void change_playername();
49 void clicked_back();49 void clicked_back();
50 void clicked_ok();50 void clicked_ok();
51 void clicked_register();
52 void verify_input();
53 bool check_password();
5154
55 UI::Button* loginbtn;
56 UI::Button* cancelbtn;
52 UI::EditBox* eb_nickname;57 UI::EditBox* eb_nickname;
53 UI::EditBox* eb_password;58 UI::EditBox* eb_password;
54 UI::Checkbox* cb_register;59 UI::Checkbox* cb_register;
55 UI::Checkbox* cb_auto_log;
56 UI::Textarea* ta_nickname;60 UI::Textarea* ta_nickname;
57 UI::Textarea* ta_password;61 UI::Textarea* ta_password;
58 UI::MultilineTextarea* pwd_warning;62 UI::MultilineTextarea* register_account;
59};63};
6064
61#endif // end of include guard: WL_WUI_LOGIN_BOX_H65#endif // end of include guard: WL_WUI_LOGIN_BOX_H

Subscribers

People subscribed via source and target branches

to status/vote changes: