Merge lp:~widelands-dev/widelands/bug-1827786-metaserver-login-box-clean-start into lp:widelands
- bug-1827786-metaserver-login-box-clean-start
- Merge into trunk
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 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
GunChleoc | Approve | ||
kaputtnik (community) | testing | Approve | |
Review via email:
|
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
Description of the change

kaputtnik (franku) wrote : Posted in a previous version of this proposal | # |

Toni Förster (stonerl) wrote : Posted in a previous version of this proposal | # |
Your're right. Chnaged it.

bunnybot (widelandsofficial) wrote : Posted in a previous version of this proposal | # |
Continuous integration builds have changed state:
Travis build 4915. State: passed. Details: https:/
Appveyor build 4696. State: success. Details: https:/

GunChleoc (gunchleoc) : Posted in a previous version of this proposal | # |

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.

bunnybot (widelandsofficial) wrote : Posted in a previous version of this proposal | # |
Continuous integration builds have changed state:
Travis build 4925. State: errored. Details: https:/
Appveyor build 4706. State: success. Details: https:/

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

GunChleoc (gunchleoc) wrote : Posted in a previous version of this proposal | # |
Changing editbox styles is implemented in https:/

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.

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.

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.

Toni Förster (stonerl) wrote : Posted in a previous version of this proposal | # |
This one gets replaced by the following one:

GunChleoc (gunchleoc) wrote : | # |
I have pushed a small tweak and added some more comments.

bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4956. State: passed. Details: https:/
Appveyor build 4737. State: success. Details: https:/

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

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

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

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

Toni Förster (stonerl) wrote : | # |
I made the "show login dialog" button a proper button. kaputtnik has second thoughts, though.
here is a screenshot:

bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4976. State: errored. Details: https:/
Appveyor build 4757. State: success. Details: https:/

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.

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

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 removenickname &password from the config.

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

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

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"

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

bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4988. State: passed. Details: https:/
Appveyor build 4769. State: success. Details: https:/

Toni Förster (stonerl) wrote : | # |
Hmm, I'm still uncertain. What about this?
https:/
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

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:/
[2]https:/
- 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

kaputtnik (franku) wrote : | # |
Ordering the buttons like here https:/
All the rest is fine for me :-)

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

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.

GunChleoc (gunchleoc) wrote : | # |
Buttons use Title Case, so:
Online Game
Online Game Settings
- 9126. By Toni Förster
-
uppercase G

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
If feasible the warning about wrong password should appear also in step 2.
I am fine with the wording though :-)
- 9127. By Toni Förster
-
addressed review comments

Toni Förster (stonerl) wrote : | # |
Can you give it another try?
- 9128. By Toni Förster
-
check for enabled checkbox

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

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

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.

Toni Förster (stonerl) wrote : | # |
Also, this needs to be resolved on the Metaserver side.

kaputtnik (franku) wrote : | # |
Ok, we may need to file a bug against the metaserver then.
This branch is ok for me now :-)

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:/
Preview Diff
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 |
It would be good to have the registered checkbox above the password field. It would be a more logical ordering, imho.