Merge lp:~widelands-dev/widelands/bug-1827182-sort-client-list into lp:widelands

Proposed by Toni Förster
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
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.

To post a comment you must log in.
Revision history for this message
Toni Förster (stonerl) wrote :

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.

Revision history for this message
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.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4854. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/527360011.
Appveyor build 4635. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827182_sort_client_list-4635.

9091. By Toni Förster

sort clientlist_ alphabetically

Revision history for this message
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.

9092. By Toni Förster

presort admin->irc

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

okay found it :D

Revision history for this message
GunChleoc (gunchleoc) :
9093. By Toni Förster

removed unused vectors

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

Addressed your review, and one diff comment.

Revision history for this message
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/CodeCheck.py src/* | grep -v "src/third_party"

review: Approve
9094. By Toni Förster

addressed Codecheck issues

9095. By Toni Förster

merged trunk

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

Done. :)

Revision history for this message
GunChleoc (gunchleoc) wrote :

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4866. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/527644741.
Appveyor build 4647. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827182_sort_client_list-4647.

Revision history for this message
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://travis-ci.org/widelands/widelands/builds/527644741.

9096. By Toni Förster

 show part message for IRC users

9097. By Toni Förster

set focus to chat input box

When entering the lobby focus is set to the input box so user can start
chatting right away.

After double clicking a username from the client list the input box will
get focus to write the private message without extra clicking.

Revision history for this message
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?

9098. By Toni Förster

in muliplayer setup set focus on chat input

9099. By Toni Förster

removed whitespace

9100. By Toni Förster

clicking on the chatbox gives focus to input field
when clicking somewhere else in the UI focus gets removed

Revision history for this message
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/ui_basic/panel.h:267: bool UI::Panel::has_focus() const: Assertion `get_can_focus()' failed.

Thread 1 "widelands" received signal SIGABRT, Aborted.
__GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
51 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) backtrace
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#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=assertion@entry=0x5555576043e0 "get_can_focus()",
    file=file@entry=0x5555576043a0 "../src/ui_basic/panel.h", line=line@entry=267,
    function=function@entry=0x55555760cd20 <UI::Panel::has_focus() const::__PRETTY_FUNCTION__> "bool UI::Panel::has_focus() const") at assert.c:92
#3 0x00007ffff433c412 in __GI___assert_fail (assertion=0x5555576043e0 "get_can_focus()", file=0x5555576043a0 "../src/ui_basic/panel.h", line=267,
    function=0x55555760cd20 <UI::Panel::has_focus() const::__PRETTY_FUNCTION__> "bool UI::Panel::has_focus() const") at assert.c:101
#4 0x0000555556af02c4 in UI::Panel::has_focus (this=0x61a00023e590) at ../src/ui_basic/panel.h:267
#5 0x0000555556afc1d0 in UI::EditBox::draw (this=0x61a00023e590, dst=...) at ../src/ui_basic/editbox.cc:364
#6 0x0000555556b2e32d in UI::Panel::do_draw_inner (this=0x61a00023e590, dst=...) at ../src/ui_basic/panel.cc:755
#7 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x61a00023e590, dst=...) at ../src/ui_basic/panel.cc:788
#8 0x0000555556b2e378 in UI::Panel::do_draw_inner (this=0x61a00023e280, dst=...) at ../src/ui_basic/panel.cc:759
#9 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x61a00023e280, dst=...) at ../src/ui_basic/panel.cc:788
#10 0x0000555556b2e378 in UI::Panel::do_draw_inner (this=0x7fffffff5af0, dst=...) at ../src/ui_basic/panel.cc:759
#11 0x0000555556b2ea07 in UI::Panel::do_draw (this=0x7fffffff5af0, dst=...) at ../src/ui_basic/panel.cc:788
#12 0x0000555556b2a015 in UI::Panel::do_run (this=0x7fffffff5af0) at ../src/ui_basic/panel.cc:195
#13 0x00005555563898fe in UI::Panel::run<FullscreenMenuBase::MenuTarget> (this=0x7fffffff5af0) at ../src/ui_basic/panel.h:104
#14 0x0000555556a057ad in GameHost::run (this=0x7fffffff7470) at ../src/network/gamehost.cc:608
#15 0x0000555556376289 in WLApplication::mainmenu_multiplayer (this=0x611000000180) at ../src/wlapplication.cc:1275
#16 0x0000555556374a40 in WLApplication::mainmenu (this=0x611000000180) at ../src/wlapplication.cc:1113
#17 0x000055555636ba12 in WLApplication::run (this=0x611000000180) at ../src/wlapplication.cc:466
#18 0x00005555563677af in main (argc=1, argv=0x7fffffffde88) at ../src/main.cc:44

Revision history for this message
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...

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

Okay it only crashes in when compiled in debug mode. Release builds don't crash. Weird.

Revision history for this message
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.

Revision history for this message
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.

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

I'm pretty sure this assert is wrong here. Removed it in r9101.

Revision history for this message
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?

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

If you have any idea let me know. I'm running out of ideas.

Revision history for this message
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

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

Can you push your fix directly to this branch? Thanks allot.

9101. By Klaus Halfman \<<email address hidden>\>

relax asert in has_focus

9102. By Klaus Halfman \<<email address hidden>\>

merged diverged branches

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Done

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Some regression tais fail, but I dont think they are related:

./regression_test.py -b ./widelands

FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_ship_before_picking_up_transporting_ware.lua

FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_portdock_with_worker_and_ware_in_transit.lua

FAIL: test/maps/ship_transportation.wmf/scripting/test_rip_ports_with_worker_in_transit.lua

Ran 44 tests in 1493.063s

FAILED (failures=3)

9103. By Toni Förster

remove unneeded ste_thinks()

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

> Some regression tais fail, but I dont think they are related:
>

Neither do I. Lets wait for travis.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Tested and working :)

review: Approve
Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4895. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/528399156.
Appveyor build 4676. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827182_sort_client_list-4676.

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

well :)

@bunnybot merge

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-01 20:17:37 +0000
3+++ src/network/internet_gaming.cc 2019-05-05 12:59:18 +0000
4@@ -19,6 +19,7 @@
5
6 #include "network/internet_gaming.h"
7
8+#include <algorithm>
9 #include <memory>
10 #include <thread>
11
12@@ -527,8 +528,7 @@
13 // Client received the new list of clients
14 uint8_t number = boost::lexical_cast<int>(packet.string()) & 0xff;
15 std::vector<InternetClient> old = clientlist_;
16- // Push IRC users to a second list and add them to the back
17- std::vector<InternetClient> irc;
18+ // Push admins/registred/IRC users to a temporary list and add them back later
19 clientlist_.clear();
20 log("InternetGaming: Received a client list update with %u items.\n", number);
21 InternetClient inc;
22@@ -537,12 +537,7 @@
23 inc.build_id = packet.string();
24 inc.game = packet.string();
25 inc.type = packet.string();
26- if (inc.type == INTERNET_CLIENT_IRC) {
27- irc.push_back(inc);
28- // No "join" or "left" messages for IRC users
29- continue;
30- }
31- // No IRC client
32+
33 clientlist_.push_back(inc);
34
35 bool found =
36@@ -554,14 +549,18 @@
37 break;
38 }
39 }
40- if (!found)
41+ if (!found) {
42 format_and_add_chat(
43 "", "", true, (boost::format(_("%s joined the lobby")) % inc.name).str());
44+ }
45 }
46- clientlist_.insert(clientlist_.end(), irc.begin(), irc.end());
47+
48+ std::sort(clientlist_.begin(), clientlist_.end(),
49+ [](const InternetClient &left, const InternetClient &right)
50+ {return (left.name < right.name);});
51
52 for (InternetClient& client : old) {
53- if (client.name.size() && client.type != INTERNET_CLIENT_IRC) {
54+ if (client.name.size()) {
55 format_and_add_chat(
56 "", "", true, (boost::format(_("%s left the lobby")) % client.name).str());
57 }
58
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 return flags_ & pf_can_focus;
64 }
65 bool has_focus() const {
66- assert(get_can_focus());
67- return (parent_->focus_ == this);
68+ return (get_can_focus() && parent_->focus_ == this);
69 }
70 virtual void focus(bool topcaller = true);
71
72
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 namespace {
78
79 // Constants for convert_clienttype() / compare_clienttype()
80-const uint8_t kClientUnregistered = 0;
81+const uint8_t kClientSuperuser = 0;
82 const uint8_t kClientRegistered = 1;
83-const uint8_t kClientSuperuser = 2;
84+const uint8_t kClientUnregistered = 2;
85 // 3 was INTERNET_CLIENT_BOT which is not used
86 const uint8_t kClientIRC = 4;
87 } // namespace
88@@ -162,6 +162,9 @@
89 // try to connect to the metaserver
90 if (!InternetGaming::ref().error() && !InternetGaming::ref().logged_in())
91 connect_to_metaserver();
92+
93+ // set focus to chat input
94+ chat.focus_edit();
95 }
96
97 void FullscreenMenuInternetLobby::layout() {
98@@ -191,6 +194,10 @@
99 if (InternetGaming::ref().update_for_games()) {
100 fill_games_list(InternetGaming::ref().games());
101 }
102+ // unfocus chat window when other UI element has focus
103+ if (!chat.has_focus()) {
104+ chat.unfocus_edit();
105+ }
106 }
107
108 void FullscreenMenuInternetLobby::clicked_ok() {
109@@ -302,6 +309,7 @@
110 }
111 prev_clientlist_len_ = clients->size();
112 }
113+ clientsonline_list_.sort();
114 }
115
116 /// called when an entry of the client list was doubleclicked
117@@ -324,12 +332,13 @@
118
119 temp += text;
120 chat.set_edit_text(temp);
121- chat.focus();
122+ chat.focus_edit();
123 }
124 }
125
126 /// called when an entry of the server list was selected
127 void FullscreenMenuInternetLobby::server_selected() {
128+ // remove focus from chat
129 if (opengames_list_.has_selection()) {
130 const InternetGame* game = &opengames_list_.get_selected();
131 if (game->connectable)
132
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 delete chat_;
138 chat_ = new GameChatPanel(this, get_w() * 3 / 80, get_h() * 17 / 30 + 0.5 * label_height_,
139 get_w() * 53 / 80, get_h() * 11 / 30, chat, UI::PanelStyle::kFsMenu);
140+ chat_->focus_edit();
141 }
142
143 /**
144@@ -372,6 +373,11 @@
145 ctrl_->think();
146 }
147 refresh();
148+
149+ // unfocus chat window when other UI element has focus
150+ if (!chat_->has_focus()) {
151+ chat_->unfocus_edit();
152+ }
153 }
154
155 /**
156
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 editbox.set_text("");
162 aborted();
163 }
164+
165+/**
166+ * The mouse was clicked on this chatbox
167+ */
168+bool GameChatPanel::handle_mousepress(const uint8_t btn, int32_t, int32_t) {
169+ if (btn == SDL_BUTTON_LEFT && get_can_focus()) {
170+ focus_edit();
171+ return true;
172+ }
173+
174+ return false;
175+}
176
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 editbox.set_text(text);
182 }
183
184+ bool handle_mousepress(uint8_t btn, int32_t x, int32_t y) override;
185 void focus_edit();
186 void unfocus_edit();
187

Subscribers

People subscribed via source and target branches

to status/vote changes: