Merge lp:~widelands-dev/widelands/bug-1827182-sort-client-list into lp:widelands
- bug-1827182-sort-client-list
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 9097 |
Proposed branch: | lp:~widelands-dev/widelands/bug-1827182-sort-client-list |
Merge into: | lp:widelands |
Diff against target: |
186 lines (+42/-16) 6 files modified
src/network/internet_gaming.cc (+10/-11) src/ui_basic/panel.h (+1/-2) src/ui_fsmenu/internet_lobby.cc (+12/-3) src/ui_fsmenu/launch_mpg.cc (+6/-0) src/wui/game_chat_panel.cc (+12/-0) src/wui/game_chat_panel.h (+1/-0) |
To merge this branch: | bzr merge lp:~widelands-dev/widelands/bug-1827182-sort-client-list |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
GunChleoc | Approve | ||
Review via email: mp+366843@code.launchpad.net |
Commit message
sort the lobby client list by client type admin->irc
The admin users will be on top of the list, followed by registered
and unregistered. IRC user are at the bottom as they are already.
IRC user's join/part messages will also be displayed
Set focus to chat input in lobby & mp by default.
Clicking on the chatbox gives focus to input field. Clicking on any
other UI element removes focus.
Description of the change
Toni Förster (stonerl) wrote : | # |
GunChleoc (gunchleoc) wrote : | # |
Define a compare functions for the table column in the UI, using the set_column_compare function. See campaign_select.cc for an example.
Then you will only need 1 vector for the clients without copying anything.
bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4854. State: failed. Details: https:/
Appveyor build 4635. State: success. Details: https:/
Toni Förster (stonerl) wrote : | # |
I found a way to sort clientlist_ alphabetically.
But how do I tell the clientsonline_list_ in internet_lobby.cc that it should sort by client type as default. Currently, it just takes the list and I have to manually click on the "*"-header to sort it.
Toni Förster (stonerl) wrote : | # |
okay found it :D
GunChleoc (gunchleoc) : | # |
Toni Förster (stonerl) wrote : | # |
Addressed your review, and one diff comment.
GunChleoc (gunchleoc) wrote : | # |
LGTM :)
This branch can go in once codecheck has been fixed. If you don't want to trigger a compile for this, you can use
cmake/codecheck
Toni Förster (stonerl) wrote : | # |
Done. :)
GunChleoc (gunchleoc) wrote : | # |
@bunnybot merge
bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4866. State: errored. Details: https:/
Appveyor build 4647. State: success. Details: https:/
bunnybot (widelandsofficial) wrote : | # |
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.
Travis build 4866. State: errored. Details: https:/
Toni Förster (stonerl) wrote : | # |
Sorry I turned this into a lobby/chat cleanup merge request. Gun would you be so kind and have another look, please?
GunChleoc (gunchleoc) wrote : | # |
I an crash this.
1. Host a new LAN game
2. Type something
3. Click on the icon next to your username
widelands: ../src/
Thread 1 "widelands" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/
51 ../sysdeps/
(gdb) backtrace
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/
#1 0x00007ffff434c801 in __GI_abort () at abort.c:79
#2 0x00007ffff433c39a in __assert_fail_base (fmt=0x7ffff44c37d8 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=
file=
function=
#3 0x00007ffff433c412 in __GI___assert_fail (assertion=
function=
#4 0x0000555556af02c4 in UI::Panel:
#5 0x0000555556afc1d0 in UI::EditBox::draw (this=0x61a0002
#6 0x0000555556b2e32d in UI::Panel:
#7 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x61a0002
#8 0x0000555556b2e378 in UI::Panel:
#9 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x61a0002
#10 0x0000555556b2e378 in UI::Panel:
#11 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x7ffffff
#12 0x0000555556b2a015 in UI::Panel::do_run (this=0x7ffffff
#13 0x00005555563898fe in UI::Panel:
#14 0x0000555556a057ad in GameHost::run (this=0x7ffffff
#15 0x0000555556376289 in WLApplication:
#16 0x0000555556374a40 in WLApplication:
#17 0x000055555636ba12 in WLApplication::run (this=0x6110000
#18 0x00005555563677af in main (argc=1, argv=0x7fffffff
Toni Förster (stonerl) wrote : | # |
Hmm, you mean the icon in the client list, where you can choose your colour? Tried this, but I'm unable to crash it...
Toni Förster (stonerl) wrote : | # |
Okay it only crashes in when compiled in debug mode. Release builds don't crash. Weird.
Klaus Halfmann (klaus-halfmann) wrote : | # |
Thats the way how Asserts work ... only in debug bilds.
I dont have that much time today, Ill try to throw it at the debugger.
Toni Förster (stonerl) wrote : | # |
Is this assert even correct? When calling has_focus() I expect either true or false. And it crashes here because it receives a false as answer but expects a true, which doesn't make much sense to me.
Toni Förster (stonerl) wrote : | # |
I'm pretty sure this assert is wrong here. Removed it in r9101.
Klaus Halfmann (klaus-halfmann) wrote : | # |
Mhh, that assert looks valid fro me. Question is why this particular inputfield
does not have the corresponding flag set?
Toni Förster (stonerl) wrote : | # |
If you have any idea let me know. I'm running out of ideas.
Klaus Halfmann (klaus-halfmann) wrote : | # |
I think this is a better fix:
bool has_focus() const {
return (get_can_focus() && parent_->focus_ == this);
}
That can_ / has_focus handling is a bit different then I had expected, well
Works find for me now
Toni Förster (stonerl) wrote : | # |
Can you push your fix directly to this branch? Thanks allot.
Klaus Halfmann (klaus-halfmann) wrote : | # |
Done
Klaus Halfmann (klaus-halfmann) wrote : | # |
Some regression tais fail, but I dont think they are related:
./regression_
FAIL: test/maps/
FAIL: test/maps/
FAIL: test/maps/
Ran 44 tests in 1493.063s
FAILED (failures=3)
Toni Förster (stonerl) wrote : | # |
> Some regression tais fail, but I dont think they are related:
>
Neither do I. Lets wait for travis.
GunChleoc (gunchleoc) wrote : | # |
Tested and working :)
bunnybot (widelandsofficial) wrote : | # |
Continuous integration builds have changed state:
Travis build 4895. State: passed. Details: https:/
Appveyor build 4676. State: success. Details: https:/
Toni Förster (stonerl) wrote : | # |
well :)
@bunnybot merge
Preview Diff
1 | === modified file 'src/network/internet_gaming.cc' | |||
2 | --- src/network/internet_gaming.cc 2019-05-01 20:17:37 +0000 | |||
3 | +++ src/network/internet_gaming.cc 2019-05-05 12:59:18 +0000 | |||
4 | @@ -19,6 +19,7 @@ | |||
5 | 19 | 19 | ||
6 | 20 | #include "network/internet_gaming.h" | 20 | #include "network/internet_gaming.h" |
7 | 21 | 21 | ||
8 | 22 | #include <algorithm> | ||
9 | 22 | #include <memory> | 23 | #include <memory> |
10 | 23 | #include <thread> | 24 | #include <thread> |
11 | 24 | 25 | ||
12 | @@ -527,8 +528,7 @@ | |||
13 | 527 | // Client received the new list of clients | 528 | // Client received the new list of clients |
14 | 528 | uint8_t number = boost::lexical_cast<int>(packet.string()) & 0xff; | 529 | uint8_t number = boost::lexical_cast<int>(packet.string()) & 0xff; |
15 | 529 | std::vector<InternetClient> old = clientlist_; | 530 | std::vector<InternetClient> old = clientlist_; |
18 | 530 | // Push IRC users to a second list and add them to the back | 531 | // Push admins/registred/IRC users to a temporary list and add them back later |
17 | 531 | std::vector<InternetClient> irc; | ||
19 | 532 | clientlist_.clear(); | 532 | clientlist_.clear(); |
20 | 533 | log("InternetGaming: Received a client list update with %u items.\n", number); | 533 | log("InternetGaming: Received a client list update with %u items.\n", number); |
21 | 534 | InternetClient inc; | 534 | InternetClient inc; |
22 | @@ -537,12 +537,7 @@ | |||
23 | 537 | inc.build_id = packet.string(); | 537 | inc.build_id = packet.string(); |
24 | 538 | inc.game = packet.string(); | 538 | inc.game = packet.string(); |
25 | 539 | inc.type = packet.string(); | 539 | inc.type = packet.string(); |
32 | 540 | if (inc.type == INTERNET_CLIENT_IRC) { | 540 | |
27 | 541 | irc.push_back(inc); | ||
28 | 542 | // No "join" or "left" messages for IRC users | ||
29 | 543 | continue; | ||
30 | 544 | } | ||
31 | 545 | // No IRC client | ||
33 | 546 | clientlist_.push_back(inc); | 541 | clientlist_.push_back(inc); |
34 | 547 | 542 | ||
35 | 548 | bool found = | 543 | bool found = |
36 | @@ -554,14 +549,18 @@ | |||
37 | 554 | break; | 549 | break; |
38 | 555 | } | 550 | } |
39 | 556 | } | 551 | } |
41 | 557 | if (!found) | 552 | if (!found) { |
42 | 558 | format_and_add_chat( | 553 | format_and_add_chat( |
43 | 559 | "", "", true, (boost::format(_("%s joined the lobby")) % inc.name).str()); | 554 | "", "", true, (boost::format(_("%s joined the lobby")) % inc.name).str()); |
44 | 555 | } | ||
45 | 560 | } | 556 | } |
47 | 561 | clientlist_.insert(clientlist_.end(), irc.begin(), irc.end()); | 557 | |
48 | 558 | std::sort(clientlist_.begin(), clientlist_.end(), | ||
49 | 559 | [](const InternetClient &left, const InternetClient &right) | ||
50 | 560 | {return (left.name < right.name);}); | ||
51 | 562 | 561 | ||
52 | 563 | for (InternetClient& client : old) { | 562 | for (InternetClient& client : old) { |
54 | 564 | if (client.name.size() && client.type != INTERNET_CLIENT_IRC) { | 563 | if (client.name.size()) { |
55 | 565 | format_and_add_chat( | 564 | format_and_add_chat( |
56 | 566 | "", "", true, (boost::format(_("%s left the lobby")) % client.name).str()); | 565 | "", "", true, (boost::format(_("%s left the lobby")) % client.name).str()); |
57 | 567 | } | 566 | } |
58 | 568 | 567 | ||
59 | === modified file 'src/ui_basic/panel.h' | |||
60 | --- src/ui_basic/panel.h 2019-03-17 11:44:04 +0000 | |||
61 | +++ src/ui_basic/panel.h 2019-05-05 12:59:18 +0000 | |||
62 | @@ -264,8 +264,7 @@ | |||
63 | 264 | return flags_ & pf_can_focus; | 264 | return flags_ & pf_can_focus; |
64 | 265 | } | 265 | } |
65 | 266 | bool has_focus() const { | 266 | bool has_focus() const { |
68 | 267 | assert(get_can_focus()); | 267 | return (get_can_focus() && parent_->focus_ == this); |
67 | 268 | return (parent_->focus_ == this); | ||
69 | 269 | } | 268 | } |
70 | 270 | virtual void focus(bool topcaller = true); | 269 | virtual void focus(bool topcaller = true); |
71 | 271 | 270 | ||
72 | 272 | 271 | ||
73 | === modified file 'src/ui_fsmenu/internet_lobby.cc' | |||
74 | --- src/ui_fsmenu/internet_lobby.cc 2019-04-24 06:01:37 +0000 | |||
75 | +++ src/ui_fsmenu/internet_lobby.cc 2019-05-05 12:59:18 +0000 | |||
76 | @@ -39,9 +39,9 @@ | |||
77 | 39 | namespace { | 39 | namespace { |
78 | 40 | 40 | ||
79 | 41 | // Constants for convert_clienttype() / compare_clienttype() | 41 | // Constants for convert_clienttype() / compare_clienttype() |
81 | 42 | const uint8_t kClientUnregistered = 0; | 42 | const uint8_t kClientSuperuser = 0; |
82 | 43 | const uint8_t kClientRegistered = 1; | 43 | const uint8_t kClientRegistered = 1; |
84 | 44 | const uint8_t kClientSuperuser = 2; | 44 | const uint8_t kClientUnregistered = 2; |
85 | 45 | // 3 was INTERNET_CLIENT_BOT which is not used | 45 | // 3 was INTERNET_CLIENT_BOT which is not used |
86 | 46 | const uint8_t kClientIRC = 4; | 46 | const uint8_t kClientIRC = 4; |
87 | 47 | } // namespace | 47 | } // namespace |
88 | @@ -162,6 +162,9 @@ | |||
89 | 162 | // try to connect to the metaserver | 162 | // try to connect to the metaserver |
90 | 163 | if (!InternetGaming::ref().error() && !InternetGaming::ref().logged_in()) | 163 | if (!InternetGaming::ref().error() && !InternetGaming::ref().logged_in()) |
91 | 164 | connect_to_metaserver(); | 164 | connect_to_metaserver(); |
92 | 165 | |||
93 | 166 | // set focus to chat input | ||
94 | 167 | chat.focus_edit(); | ||
95 | 165 | } | 168 | } |
96 | 166 | 169 | ||
97 | 167 | void FullscreenMenuInternetLobby::layout() { | 170 | void FullscreenMenuInternetLobby::layout() { |
98 | @@ -191,6 +194,10 @@ | |||
99 | 191 | if (InternetGaming::ref().update_for_games()) { | 194 | if (InternetGaming::ref().update_for_games()) { |
100 | 192 | fill_games_list(InternetGaming::ref().games()); | 195 | fill_games_list(InternetGaming::ref().games()); |
101 | 193 | } | 196 | } |
102 | 197 | // unfocus chat window when other UI element has focus | ||
103 | 198 | if (!chat.has_focus()) { | ||
104 | 199 | chat.unfocus_edit(); | ||
105 | 200 | } | ||
106 | 194 | } | 201 | } |
107 | 195 | 202 | ||
108 | 196 | void FullscreenMenuInternetLobby::clicked_ok() { | 203 | void FullscreenMenuInternetLobby::clicked_ok() { |
109 | @@ -302,6 +309,7 @@ | |||
110 | 302 | } | 309 | } |
111 | 303 | prev_clientlist_len_ = clients->size(); | 310 | prev_clientlist_len_ = clients->size(); |
112 | 304 | } | 311 | } |
113 | 312 | clientsonline_list_.sort(); | ||
114 | 305 | } | 313 | } |
115 | 306 | 314 | ||
116 | 307 | /// called when an entry of the client list was doubleclicked | 315 | /// called when an entry of the client list was doubleclicked |
117 | @@ -324,12 +332,13 @@ | |||
118 | 324 | 332 | ||
119 | 325 | temp += text; | 333 | temp += text; |
120 | 326 | chat.set_edit_text(temp); | 334 | chat.set_edit_text(temp); |
122 | 327 | chat.focus(); | 335 | chat.focus_edit(); |
123 | 328 | } | 336 | } |
124 | 329 | } | 337 | } |
125 | 330 | 338 | ||
126 | 331 | /// called when an entry of the server list was selected | 339 | /// called when an entry of the server list was selected |
127 | 332 | void FullscreenMenuInternetLobby::server_selected() { | 340 | void FullscreenMenuInternetLobby::server_selected() { |
128 | 341 | // remove focus from chat | ||
129 | 333 | if (opengames_list_.has_selection()) { | 342 | if (opengames_list_.has_selection()) { |
130 | 334 | const InternetGame* game = &opengames_list_.get_selected(); | 343 | const InternetGame* game = &opengames_list_.get_selected(); |
131 | 335 | if (game->connectable) | 344 | if (game->connectable) |
132 | 336 | 345 | ||
133 | === modified file 'src/ui_fsmenu/launch_mpg.cc' | |||
134 | --- src/ui_fsmenu/launch_mpg.cc 2019-04-26 11:33:10 +0000 | |||
135 | +++ src/ui_fsmenu/launch_mpg.cc 2019-05-05 12:59:18 +0000 | |||
136 | @@ -228,6 +228,7 @@ | |||
137 | 228 | delete chat_; | 228 | delete chat_; |
138 | 229 | chat_ = new GameChatPanel(this, get_w() * 3 / 80, get_h() * 17 / 30 + 0.5 * label_height_, | 229 | chat_ = new GameChatPanel(this, get_w() * 3 / 80, get_h() * 17 / 30 + 0.5 * label_height_, |
139 | 230 | get_w() * 53 / 80, get_h() * 11 / 30, chat, UI::PanelStyle::kFsMenu); | 230 | get_w() * 53 / 80, get_h() * 11 / 30, chat, UI::PanelStyle::kFsMenu); |
140 | 231 | chat_->focus_edit(); | ||
141 | 231 | } | 232 | } |
142 | 232 | 233 | ||
143 | 233 | /** | 234 | /** |
144 | @@ -372,6 +373,11 @@ | |||
145 | 372 | ctrl_->think(); | 373 | ctrl_->think(); |
146 | 373 | } | 374 | } |
147 | 374 | refresh(); | 375 | refresh(); |
148 | 376 | |||
149 | 377 | // unfocus chat window when other UI element has focus | ||
150 | 378 | if (!chat_->has_focus()) { | ||
151 | 379 | chat_->unfocus_edit(); | ||
152 | 380 | } | ||
153 | 375 | } | 381 | } |
154 | 376 | 382 | ||
155 | 377 | /** | 383 | /** |
156 | 378 | 384 | ||
157 | === modified file 'src/wui/game_chat_panel.cc' | |||
158 | --- src/wui/game_chat_panel.cc 2019-04-24 06:01:37 +0000 | |||
159 | +++ src/wui/game_chat_panel.cc 2019-05-05 12:59:18 +0000 | |||
160 | @@ -119,3 +119,15 @@ | |||
161 | 119 | editbox.set_text(""); | 119 | editbox.set_text(""); |
162 | 120 | aborted(); | 120 | aborted(); |
163 | 121 | } | 121 | } |
164 | 122 | |||
165 | 123 | /** | ||
166 | 124 | * The mouse was clicked on this chatbox | ||
167 | 125 | */ | ||
168 | 126 | bool GameChatPanel::handle_mousepress(const uint8_t btn, int32_t, int32_t) { | ||
169 | 127 | if (btn == SDL_BUTTON_LEFT && get_can_focus()) { | ||
170 | 128 | focus_edit(); | ||
171 | 129 | return true; | ||
172 | 130 | } | ||
173 | 131 | |||
174 | 132 | return false; | ||
175 | 133 | } | ||
176 | 122 | 134 | ||
177 | === modified file 'src/wui/game_chat_panel.h' | |||
178 | --- src/wui/game_chat_panel.h 2019-03-17 11:44:04 +0000 | |||
179 | +++ src/wui/game_chat_panel.h 2019-05-05 12:59:18 +0000 | |||
180 | @@ -54,6 +54,7 @@ | |||
181 | 54 | editbox.set_text(text); | 54 | editbox.set_text(text); |
182 | 55 | } | 55 | } |
183 | 56 | 56 | ||
184 | 57 | bool handle_mousepress(uint8_t btn, int32_t x, int32_t y) override; | ||
185 | 57 | void focus_edit(); | 58 | void focus_edit(); |
186 | 58 | void unfocus_edit(); | 59 | void unfocus_edit(); |
187 | 59 | 60 |
I do need some help here. This is good as it is now, but I'd like to sort the irc/superuser/ registered list alphabetically before inserting them into clientlist_. Can some tell me how to do this.