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
9107. By GunChleoc

Tweaked a string and reformatted a comment so that clang-format can generate better line breaks.

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.

9108. By Toni Förster

editbox can be a password field

edibox show asterisk instead off testinput when set to pasword field

9109. By Toni Förster

added valid_username() to editboxes

9110. By Toni Förster

disable and empty password field when username is changed

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.

9111. By Toni Förster

added text_to_asterisk()

caret is now properly aligned in password fields

9112. By Toni Förster

make text_to_asterisk() privat

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

Carets are also fixed.

9113. By Toni Förster

changed icon for login-box button

9114. By Toni Förster

made >show login dialog< a normal button

9115. By Toni Förster

proper resizing

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

9116. By Toni Förster

revert changes to buttons

9117. By Toni Förster

moved valid_username() to internet_gaming.cc

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

9118. By Toni Förster

Added log out button to internet_lobby

When no registered account is used the log in box will show up before
every login. Registered users can logout from the lobby. This will remove

nickname &password from the config.

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.

9119. By Toni Förster

when username is set, focus login button

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?

9120. By Toni Förster

rename login box title - save instead of login

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

9121. By Toni Förster

rename back to log out

9122. By Toni Förster

add more space

9123. By Toni Förster

autologin first time users or on wrong password/username

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"?

9124. By Toni Förster

rename back button to Leave Lobby

9125. By Toni Förster

rename Internet game to Online game anf move buttons in multiplayer

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

9126. By Toni Förster

uppercase G

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
9127. By Toni Förster

addressed review comments

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

Can you give it another try?

9128. By Toni Förster

check for enabled checkbox

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.

9129. By Toni Förster

save registered only if password change was successfull

9130. By Toni Förster

only save on successfull changes

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
1=== modified file 'src/network/internet_gaming.cc'
2--- src/network/internet_gaming.cc 2019-05-13 02:25:58 +0000
3+++ src/network/internet_gaming.cc 2019-05-23 17:11:42 +0000
4@@ -947,3 +947,14 @@
5 ingame_system_chat_.push_back(c);
6 }
7 }
8+
9+/**
10+ * Check for vaild username characters.
11+ */
12+bool InternetGaming::valid_username(std::string username) {
13+ if (username.empty() || username.find_first_not_of("abcdefghijklmnopqrstuvwxyz"
14+ "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890@.+-_") <= username.size()) {
15+ return false;
16+ }
17+ return true;
18+}
19
20=== modified file 'src/network/internet_gaming.h'
21--- src/network/internet_gaming.h 2019-05-11 22:02:09 +0000
22+++ src/network/internet_gaming.h 2019-05-23 17:11:42 +0000
23@@ -183,6 +183,8 @@
24 bool system,
25 const std::string& msg);
26
27+ bool valid_username(std::string);
28+
29 private:
30 InternetGaming();
31
32
33=== modified file 'src/ui_basic/editbox.cc'
34--- src/ui_basic/editbox.cc 2019-05-13 02:25:58 +0000
35+++ src/ui_basic/editbox.cc 2019-05-23 17:11:42 +0000
36@@ -19,7 +19,9 @@
37
38 #include "ui_basic/editbox.h"
39
40+#include <algorithm>
41 #include <limits>
42+#include <string>
43
44 #include <SDL_keycode.h>
45 #include <boost/format.hpp>
46@@ -86,7 +88,8 @@
47 m_(new EditBoxImpl),
48 history_active_(false),
49 history_position_(-1),
50- warning_(false) {
51+ password_(false),
52+ warning_(false) {
53 set_thinks(false);
54
55 m_->background_style = g_gr->styles().editbox_style(style);
56@@ -388,7 +391,8 @@
57 const int max_width = get_w() - 2 * kMarginX;
58
59 std::shared_ptr<const UI::RenderedText> rendered_text =
60- UI::g_fh->render(as_editorfont(m_->text, m_->fontsize));
61+ UI::g_fh->render(as_editorfont(m_->text, m_->fontsize));
62+
63
64 const int linewidth = rendered_text->width();
65 const int lineheight = m_->text.empty() ? text_height(m_->fontsize) : rendered_text->height();
66@@ -400,31 +404,44 @@
67 UI::center_vertically(lineheight, &point);
68
69 // Crop to max_width while blitting
70- if (max_width < linewidth) {
71- // Fix positioning for BiDi languages.
72- if (UI::g_fh->fontset()->is_rtl()) {
73- point.x = 0.f;
74- }
75- // We want this always on, e.g. for mixed language savegame filenames
76- if (i18n::has_rtl_character(m_->text.c_str(), 100)) { // Restrict check for efficiency
77- // TODO(GunChleoc): Arabic: Fix scrolloffset
78- rendered_text->draw(dst, point, Recti(linewidth - max_width, 0, linewidth, lineheight));
79- } else {
80- if (m_->align == UI::Align::kRight) {
81+ if (!password_) {
82+ if (max_width < linewidth) {
83+ // Fix positioning for BiDi languages.
84+ if (UI::g_fh->fontset()->is_rtl()) {
85+ point.x = 0.f;
86+ }
87+ // We want this always on, e.g. for mixed language savegame filenames
88+ if (i18n::has_rtl_character(m_->text.c_str(), 100)) { // Restrict check for efficiency
89 // TODO(GunChleoc): Arabic: Fix scrolloffset
90- rendered_text->draw(
91- dst, point, Recti(point.x + m_->scrolloffset + kMarginX, 0, max_width, lineheight));
92+ rendered_text->draw(dst, point, Recti(linewidth - max_width, 0, linewidth, lineheight));
93 } else {
94- rendered_text->draw(dst, point, Recti(-m_->scrolloffset, 0, max_width, lineheight));
95+ if (m_->align == UI::Align::kRight) {
96+ // TODO(GunChleoc): Arabic: Fix scrolloffset
97+ rendered_text->draw(
98+ dst, point, Recti(point.x + m_->scrolloffset + kMarginX, 0, max_width, lineheight));
99+ } else {
100+ rendered_text->draw(dst, point, Recti(-m_->scrolloffset, 0, max_width, lineheight));
101+ }
102 }
103+ } else {
104+ rendered_text->draw(dst, point, Recti(0, 0, max_width, lineheight));
105 }
106 } else {
107- rendered_text->draw(dst, point, Recti(0, 0, max_width, lineheight));
108+ std::shared_ptr<const UI::RenderedText> password_text =
109+ UI::g_fh->render(as_editorfont(text_to_asterisk(), m_->fontsize));
110+ password_text->draw(dst, point, Recti(0, 0, max_width, lineheight));
111 }
112
113 if (has_focus()) {
114 // Draw the caret
115- std::string line_to_caret = m_->text.substr(0, m_->caret);
116+ std::string line_to_caret;
117+
118+ if (password_) {
119+ line_to_caret = text_to_asterisk().substr(0, m_->caret);
120+ } else {
121+ line_to_caret = m_->text.substr(0, m_->caret);
122+ }
123+
124 // TODO(GunChleoc): Arabic: Fix cursor position for BIDI text.
125 int caret_x = text_width(line_to_caret, m_->fontsize);
126
127@@ -471,4 +488,15 @@
128 m_->scrolloffset = 0;
129 }
130 }
131+
132+/**
133+ * Return text as asterisks.
134+ */
135+std::string EditBox::text_to_asterisk() {
136+ std::string asterisk;
137+ for (int i = 0; i < int (m_->text.size()); i++) {
138+ asterisk.append("*");
139+ }
140+ return asterisk;
141+}
142 } // namespace UI
143
144=== modified file 'src/ui_basic/editbox.h'
145--- src/ui_basic/editbox.h 2019-05-11 22:02:09 +0000
146+++ src/ui_basic/editbox.h 2019-05-23 17:11:42 +0000
147@@ -72,18 +72,28 @@
148
149 void draw(RenderTarget&) override;
150
151+ void set_password(bool pass) {
152+ password_ = pass;
153+ }
154+
155 void set_warning(bool warn) {
156 warning_ = warn;
157 }
158
159+ bool is_password() {
160+ return password_;
161+ }
162+
163 private:
164 std::unique_ptr<EditBoxImpl> m_;
165
166 void check_caret();
167+ std::string text_to_asterisk();
168
169 bool history_active_;
170 int16_t history_position_;
171 std::string history_[CHAT_HISTORY_SIZE];
172+ bool password_;
173 bool warning_;
174 };
175 } // namespace UI
176
177=== modified file 'src/ui_fsmenu/internet_lobby.cc'
178--- src/ui_fsmenu/internet_lobby.cc 2019-05-13 02:25:58 +0000
179+++ src/ui_fsmenu/internet_lobby.cc 2019-05-23 17:11:42 +0000
180@@ -78,7 +78,7 @@
181 hostgame_(this,
182 "host_game",
183 get_w() * 17 / 25,
184- get_h() * 81 / 100,
185+ get_h() * 73 / 100,
186 butw_,
187 buth_,
188 UI::ButtonStyle::kFsMenuSecondary,
189@@ -90,7 +90,7 @@
190 butw_,
191 buth_,
192 UI::ButtonStyle::kFsMenuSecondary,
193- _("Back")),
194+ _("Leave Lobby")),
195
196 // Edit boxes
197 edit_servername_(
198@@ -165,6 +165,7 @@
199
200 // set focus to chat input
201 chat.focus_edit();
202+
203 }
204
205 void FullscreenMenuInternetLobby::layout() {
206
207=== modified file 'src/ui_fsmenu/multiplayer.cc'
208--- src/ui_fsmenu/multiplayer.cc 2019-05-11 18:50:30 +0000
209+++ src/ui_fsmenu/multiplayer.cc 2019-05-23 17:11:42 +0000
210@@ -21,7 +21,6 @@
211
212 #include "base/i18n.h"
213 #include "graphic/text_constants.h"
214-#include "network/crypto.h"
215 #include "network/internet_gaming.h"
216 #include "profile/profile.h"
217 #include "random/random.h"
218@@ -36,9 +35,10 @@
219
220 // Buttons
221 metaserver(
222- &vbox_, "metaserver", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Internet game")),
223- showloginbox(nullptr),
224+ &vbox_, "metaserver", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Online Game")),
225 lan(&vbox_, "lan", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("LAN / Direct IP")),
226+ showloginbox(
227+ &vbox_, "lan", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Online Game Settings")),
228 back(&vbox_, "back", 0, 0, butw_, buth_, UI::ButtonStyle::kFsMenuMenu, _("Back")) {
229 metaserver.sigclicked.connect(
230 boost::bind(&FullscreenMenuMultiPlayer::internet_login, boost::ref(*this)));
231@@ -47,6 +47,9 @@
232 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,
233 boost::ref(*this), FullscreenMenuBase::MenuTarget::kLan));
234
235+ showloginbox.sigclicked.connect(
236+ boost::bind(&FullscreenMenuMultiPlayer::show_internet_login, boost::ref(*this)));
237+
238 back.sigclicked.connect(
239 boost::bind(&FullscreenMenuMultiPlayer::end_modal<FullscreenMenuBase::MenuTarget>,
240 boost::ref(*this), FullscreenMenuBase::MenuTarget::kBack));
241@@ -54,73 +57,50 @@
242 title.set_fontsize(fs_big());
243
244 vbox_.add(&metaserver, UI::Box::Resizing::kFullSize);
245+ vbox_.add(&showloginbox, UI::Box::Resizing::kFullSize);
246+ vbox_.add_inf_space();
247 vbox_.add(&lan, UI::Box::Resizing::kFullSize);
248 vbox_.add_inf_space();
249+ vbox_.add_inf_space();
250+ vbox_.add_inf_space();
251 vbox_.add(&back, UI::Box::Resizing::kFullSize);
252
253- Section& s = g_options.pull_section("global");
254- auto_log_ = s.get_bool("auto_log", false);
255- if (auto_log_) {
256- showloginbox =
257- new UI::Button(this, "login_dialog", 0, 0, 0, 0, UI::ButtonStyle::kFsMenuSecondary,
258- g_gr->images().get("images/ui_basic/continue.png"), _("Show login dialog"));
259- showloginbox->sigclicked.connect(
260- boost::bind(&FullscreenMenuMultiPlayer::show_internet_login, boost::ref(*this)));
261- }
262+ auto_log_ = false;
263 layout();
264 }
265
266-/// called if the showloginbox button was pressed
267+/// called if the user is not registered
268 void FullscreenMenuMultiPlayer::show_internet_login() {
269- auto_log_ = false;
270- internet_login();
271+ LoginBox lb(*this);
272+ if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk && auto_log_) {
273+ auto_log_ = false;
274+ internet_login();
275+ }
276 }
277
278 /**
279- * Called if "Internet" button was pressed.
280- *
281- * IF autologin is not set, a LoginBox is shown and, if the user clicks on
282- * 'login' in it's menu, the data is read from the LoginBox and saved in 'this'
283- * so wlapplication can read it.
284- *
285- * IF autologin is set, all data is read from the config file and saved.
286- * That data will be used for login to the metaserver.
287- *
288- * In both cases this fullscreen menu ends it's modality.
289+ * Called if "Online Game" button was pressed.
290+ *
291+ * IF no nickname or a nickname with invalid characters is set, the Online Game Settings
292+ * are opened.
293+ *
294+ * IF at least a name is set, all data is read from the config file
295+ *
296+ * This fullscreen menu ends it's modality.
297 */
298 void FullscreenMenuMultiPlayer::internet_login() {
299 Section& s = g_options.pull_section("global");
300- if (auto_log_) {
301- nickname_ = s.get_string("nickname", _("nobody"));
302- password_ = s.get_string("password_sha1", "nobody");
303- register_ = s.get_bool("registered", false);
304- } else {
305- LoginBox lb(*this);
306- if (lb.run<UI::Panel::Returncodes>() == UI::Panel::Returncodes::kOk) {
307- nickname_ = lb.get_nickname();
308- /// NOTE: The password is only stored (in memory and on disk) and transmitted (over the
309- /// network
310- /// to the metaserver) as cryptographic hash. This does NOT mean that the password is
311- /// stored
312- /// securely on the local disk. While the password should be secure while transmitted to
313- /// the
314- /// metaserver (no-one can use the transmitted data to log in as the user) this is not the
315- /// case
316- /// for local storage. The stored hash of the password makes it hard to look at the
317- /// configuration
318- /// file and figure out the plaintext password to, e.g., log in on the forum. However, the
319- /// stored hash can be copied to another system and used to log in as the user on the
320- /// metaserver.
321- // Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
322- // passwords on the server are protected by SHA-1 we have to use it here, too
323- password_ = crypto::sha1(lb.get_password());
324- register_ = lb.registered();
325-
326- s.set_bool("registered", lb.registered());
327- s.set_bool("auto_log", lb.set_automaticlog());
328- } else {
329- return;
330- }
331+
332+ nickname_ = s.get_string("nickname", "");
333+ password_ = s.get_string("password_sha1", "no_password_set");
334+ register_ = s.get_bool("registered", false);
335+
336+ // Checks can be done directly in editbox' by using valid_username().
337+ // This is just to be on the safe side, in case the user changed the password in the config file.
338+ if (!InternetGaming::ref().valid_username(nickname_)) {
339+ auto_log_ = true;
340+ show_internet_login();
341+ return;
342 }
343
344 // Try to connect to the metaserver
345@@ -141,7 +121,8 @@
346
347 // Reset InternetGaming and passwort and show internet login again
348 InternetGaming::ref().reset();
349- s.set_string("password_sha1", "");
350+ s.set_string("password_sha1", "no_password_set");
351+ auto_log_ = true;
352 show_internet_login();
353 }
354 }
355@@ -156,12 +137,8 @@
356
357 title.set_pos(Vector2i(0, title_y_));
358
359- metaserver.set_size(butw_, buth_);
360- if (showloginbox) {
361- showloginbox->set_pos(Vector2i(box_x_ + butw_ + padding_ / 2, box_y_));
362- showloginbox->set_size(buth_, buth_);
363- }
364 metaserver.set_desired_size(butw_, buth_);
365+ showloginbox.set_desired_size(butw_, buth_);
366 lan.set_desired_size(butw_, buth_);
367 back.set_desired_size(butw_, buth_);
368
369
370=== modified file 'src/ui_fsmenu/multiplayer.h'
371--- src/ui_fsmenu/multiplayer.h 2019-05-11 18:50:30 +0000
372+++ src/ui_fsmenu/multiplayer.h 2019-05-23 17:11:42 +0000
373@@ -53,15 +53,15 @@
374
375 UI::Textarea title;
376 UI::Button metaserver;
377- UI::Button* showloginbox;
378 UI::Button lan;
379+ UI::Button showloginbox;
380 UI::Button back;
381
382 // Values from internet login window
383 std::string nickname_;
384 std::string password_;
385+ bool auto_log_;
386 bool register_;
387- bool auto_log_;
388 };
389
390 #endif // end of include guard: WL_UI_FSMENU_MULTIPLAYER_H
391
392=== modified file 'src/wui/login_box.cc'
393--- src/wui/login_box.cc 2019-05-11 18:50:30 +0000
394+++ src/wui/login_box.cc 2019-05-23 17:11:42 +0000
395@@ -21,75 +21,89 @@
396
397 #include "base/i18n.h"
398 #include "graphic/font_handler.h"
399+#include "network/crypto.h"
400+#include "network/internet_gaming.h"
401 #include "profile/profile.h"
402 #include "ui_basic/button.h"
403 #include "ui_basic/messagebox.h"
404
405 LoginBox::LoginBox(Panel& parent)
406- : Window(&parent, "login_box", 0, 0, 500, 220, _("Metaserver login")) {
407+ : Window(&parent, "login_box", 0, 0, 500, 280, _("Online Game Settings")) {
408 center_to_parent();
409
410 int32_t margin = 10;
411
412 ta_nickname = new UI::Textarea(this, margin, margin, _("Nickname:"));
413- ta_password = new UI::Textarea(this, margin, 40, _("Password:"));
414+ ta_password = new UI::Textarea(this, margin, 70, _("Password:"));
415 eb_nickname = new UI::EditBox(this, 150, margin, 330, 20, 2, UI::PanelStyle::kWui);
416- eb_password = new UI::EditBox(this, 150, 40, 330, 20, 2, UI::PanelStyle::kWui);
417-
418- pwd_warning =
419- new UI::MultilineTextarea(this, margin, 65, 505, 50, UI::PanelStyle::kWui,
420- _("WARNING: Password will be shown and saved readable!"));
421-
422- cb_register = new UI::Checkbox(this, Vector2i(margin, 110), _("Log in to a registered account"),
423+ eb_password = new UI::EditBox(this, 150, 70, 330, 20, 2, UI::PanelStyle::kWui);
424+
425+ cb_register = new UI::Checkbox(this, Vector2i(margin, 40), _("Log in to a registered account."),
426 "", get_inner_w() - 2 * margin);
427- cb_auto_log = new UI::Checkbox(this, Vector2i(margin, 135),
428- _("Automatically use this login information from now on."), "",
429- get_inner_w() - 2 * margin);
430-
431- UI::Button* loginbtn = new UI::Button(
432+
433+ register_account = new UI::MultilineTextarea(this, margin, 105, 470, 140, UI::PanelStyle::kWui,
434+ (boost::format(_("In order to use a registered "
435+ "account, you need an account on the widelands website. "
436+ "Please log in at %s and set an online "
437+ "gaming password on your profile page."))
438+ % "\n\nhttps://widelands.org/accounts/register/\n\n").str());
439+
440+ loginbtn = new UI::Button(
441 this, "login",
442 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 :
443 (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2,
444- get_inner_h() - 20 - margin, 200, 20, UI::ButtonStyle::kWuiPrimary, _("Login"));
445- loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));
446- UI::Button* cancelbtn = new UI::Button(
447+ get_inner_h() - 20 - margin, 200, 20, UI::ButtonStyle::kWuiPrimary, _("Save"));
448+
449+ cancelbtn = new UI::Button(
450 this, "cancel",
451 UI::g_fh->fontset()->is_rtl() ? (get_inner_w() / 2 - 200) / 2 + get_inner_w() / 2 :
452 (get_inner_w() / 2 - 200) / 2,
453 loginbtn->get_y(), 200, 20, UI::ButtonStyle::kWuiSecondary, _("Cancel"));
454+
455+ loginbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_ok, boost::ref(*this)));
456 cancelbtn->sigclicked.connect(boost::bind(&LoginBox::clicked_back, boost::ref(*this)));
457+ eb_nickname->changed.connect(boost::bind(&LoginBox::change_playername, this));
458+ cb_register->clickedto.connect(boost::bind(&LoginBox::clicked_register, this));
459
460 Section& s = g_options.pull_section("global");
461 eb_nickname->set_text(s.get_string("nickname", _("nobody")));
462 cb_register->set_state(s.get_bool("registered", false));
463+ eb_password->set_password(true);
464+
465+ if (registered()) {
466+ eb_password->set_text(s.get_string("password_sha1", ""));
467+ loginbtn->set_enabled(false);
468+ } else {
469+ eb_password->set_can_focus(false);
470+ ta_password->set_color(UI_FONT_CLR_DISABLED);
471+ }
472+
473 eb_nickname->focus();
474+
475+}
476+
477+/// think function of the UI (main loop)
478+void LoginBox::think() {
479+ verify_input();
480 }
481
482 /**
483 * called, if "login" is pressed.
484 */
485 void LoginBox::clicked_ok() {
486- // Check if all needed input fields are valid
487- if (eb_nickname->text().empty()) {
488- UI::WLMessageBox mb(
489- this, _("Empty Nickname"), _("Please enter a nickname!"), UI::WLMessageBox::MBoxType::kOk);
490- mb.run<UI::Panel::Returncodes>();
491- return;
492- }
493- if (eb_nickname->text().find(' ') <= eb_nickname->text().size()) {
494- UI::WLMessageBox mb(this, _("Space in Nickname"),
495- _("Sorry, but spaces are not allowed in nicknames!"),
496- UI::WLMessageBox::MBoxType::kOk);
497- mb.run<UI::Panel::Returncodes>();
498- return;
499- }
500- if (eb_password->text().empty() && cb_register->get_state()) {
501- UI::WLMessageBox mb(this, _("Empty Password"), _("Please enter your password!"),
502- UI::WLMessageBox::MBoxType::kOk);
503- mb.run<UI::Panel::Returncodes>();
504- return;
505- }
506- end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
507+ Section& s = g_options.pull_section("global");
508+ if (cb_register->get_state()) {
509+ if (check_password()) {
510+ s.set_string("nickname", eb_nickname->text());
511+ s.set_bool("registered", true);
512+ end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
513+ }
514+ } else {
515+ s.set_string("nickname", eb_nickname->text());
516+ s.set_bool("registered", false);
517+ s.set_string("password_sha1", "");
518+ end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kOk);
519+ }
520 }
521
522 /// Called if "cancel" was pressed
523@@ -97,6 +111,13 @@
524 end_modal<UI::Panel::Returncodes>(UI::Panel::Returncodes::kBack);
525 }
526
527+/// Called when nickname was changed
528+void LoginBox::change_playername() {
529+ cb_register->set_state(false);
530+ eb_password->set_can_focus(false);
531+ eb_password->set_text("");
532+}
533+
534 bool LoginBox::handle_key(bool down, SDL_Keysym code) {
535 if (down) {
536 switch (code.sym) {
537@@ -113,3 +134,80 @@
538 }
539 return UI::Panel::handle_key(down, code);
540 }
541+
542+void LoginBox::clicked_register() {
543+ if (cb_register->get_state()) {
544+ ta_password->set_color(UI_FONT_CLR_DISABLED);
545+ eb_password->set_can_focus(false);
546+ eb_password->set_text("");
547+ } else {
548+ ta_password->set_color(UI_FONT_CLR_FG);
549+ eb_password->set_can_focus(true);
550+ }
551+}
552+
553+void LoginBox::verify_input() {
554+ // Check if all neccessary input fields are valid
555+ loginbtn->set_enabled(true);
556+ eb_nickname->set_tooltip("");
557+ eb_password->set_tooltip("");
558+ eb_nickname->set_warning(false);
559+
560+ if (eb_nickname->text().empty()) {
561+ eb_nickname->set_warning(true);
562+ eb_nickname->set_tooltip(_("Please enter a nickname!"));
563+ loginbtn->set_enabled(false);
564+ } else if (!InternetGaming::ref().valid_username(eb_nickname->text())) {
565+ eb_nickname->set_warning(true);
566+ eb_nickname->set_tooltip(_("Enter a valid nickname. This value may contain only "
567+ "English letters, numbers, and @ . + - _ characters."));
568+ loginbtn->set_enabled(false);
569+ }
570+
571+ if (eb_password->text().empty() && cb_register->get_state()) {
572+ eb_password->set_tooltip(_("Please enter your password!"));
573+ loginbtn->set_enabled(false);
574+ eb_password->focus();
575+ }
576+
577+ Section& s = g_options.pull_section("global");
578+ if (eb_password->has_focus() && eb_password->text() == s.get_string("password_sha1", "")) {
579+ eb_password->set_text("");
580+ }
581+
582+ if (cb_register->get_state() && eb_password->text() == s.get_string("password_sha1", "")) {
583+ loginbtn->set_enabled(false);
584+ }
585+}
586+
587+/// Check password against metaserver
588+bool LoginBox::check_password() {
589+ // Try to connect to the metaserver
590+ Section& s = g_options.pull_section("global");
591+ const std::string& meta = s.get_string("metaserver", INTERNET_GAMING_METASERVER.c_str());
592+ uint32_t port = s.get_natural("metaserverport", kInternetGamingPort);
593+ std::string password = crypto::sha1(eb_password->text());
594+ InternetGaming::ref().login(get_nickname(), password, true, meta, port);
595+
596+ if (!InternetGaming::ref().logged_in()) {
597+ // something went wrong -> show the error message
598+ // idealy it is about the wrong password
599+ ChatMessage msg = InternetGaming::ref().get_messages().back();
600+ UI::WLMessageBox wmb(this, _("Error!"), msg.msg, UI::WLMessageBox::MBoxType::kOk);
601+ wmb.run<UI::Panel::Returncodes>();
602+ InternetGaming::ref().reset();
603+ eb_password->set_text("");
604+ eb_password->focus();
605+ return false;
606+ }
607+ // NOTE: The password is only stored (in memory and on disk) and transmitted (over the network to the metaserver) as cryptographic hash.
608+ // This does NOT mean that the password is stored securely on the local disk.
609+ // 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.
610+ // 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.
611+ // However, the stored hash can be copied to another system and used to log in as the user on the metaserver.
612+ // Further note: SHA-1 is considered broken and shouldn't be used anymore. But since the
613+ // passwords on the server are protected by SHA-1 we have to use it here, too
614+ s.set_string("password_sha1", password);
615+ InternetGaming::ref().logout();
616+ return true;
617+}
618
619=== modified file 'src/wui/login_box.h'
620--- src/wui/login_box.h 2019-05-11 18:50:30 +0000
621+++ src/wui/login_box.h 2019-05-23 17:11:42 +0000
622@@ -29,6 +29,8 @@
623 struct LoginBox : public UI::Window {
624 explicit LoginBox(UI::Panel&);
625
626+ void think() override;
627+
628 std::string get_nickname() {
629 return eb_nickname->text();
630 }
631@@ -38,24 +40,26 @@
632 bool registered() {
633 return cb_register->get_state();
634 }
635- bool set_automaticlog() {
636- return cb_auto_log->get_state();
637- }
638
639 /// Handle keypresses
640 bool handle_key(bool down, SDL_Keysym code) override;
641
642 private:
643+ void change_playername();
644 void clicked_back();
645 void clicked_ok();
646+ void clicked_register();
647+ void verify_input();
648+ bool check_password();
649
650+ UI::Button* loginbtn;
651+ UI::Button* cancelbtn;
652 UI::EditBox* eb_nickname;
653 UI::EditBox* eb_password;
654 UI::Checkbox* cb_register;
655- UI::Checkbox* cb_auto_log;
656 UI::Textarea* ta_nickname;
657 UI::Textarea* ta_password;
658- UI::MultilineTextarea* pwd_warning;
659+ UI::MultilineTextarea* register_account;
660 };
661
662 #endif // end of include guard: WL_WUI_LOGIN_BOX_H

Subscribers

People subscribed via source and target branches

to status/vote changes: