Patch : Network game list name filter
Moderator: OpenTTD Developers
Patch : Network game list name filter
Hello,
This patch provides the basic text filter functionality to network game. (FS #3852) Although there are many other criteria I can filter on (number of clients, companies, map size, etc...), I thought it would be better to start small and expand. I'll add as soon as I finish the rest. I hope someone finds this useful.
-handrake
This patch provides the basic text filter functionality to network game. (FS #3852) Although there are many other criteria I can filter on (number of clients, companies, map size, etc...), I thought it would be better to start small and expand. I'll add as soon as I finish the rest. I hope someone finds this useful.
-handrake
Last edited by handrake on 30 Mar 2012 07:09, edited 1 time in total.
- stevenh
- TTDPatch Developer
- Posts: 759
- Joined: 24 Jul 2005 05:07
- Location: Canberra, Australia
- Contact:
Re: Patch : Network game list name filter
Great idea... but does it need to be a popup?
A "Filter: [___________]" box up the top next to Connection Type would be ideal.
Saving your previous filter and putting an "[X]" button to clear the box would be cool too.
A "Filter: [___________]" box up the top next to Connection Type would be ideal.
Saving your previous filter and putting an "[X]" button to clear the box would be cool too.
Re: Patch : Network game list name filter
I believe the popup has something to do with the fact that there can be only one text input thingy in a window. The player name already takes this position.
Don't ask me the details on this though
Maybe it's better to move the player name to a popup, as I think one changes that less often than a filter.
I agree that it would be useful to see somewhere that a filter is applied, what that filter is, and an option to remove it. Other than that I can only applaud the effort, well done!
Don't ask me the details on this though
Maybe it's better to move the player name to a popup, as I think one changes that less often than a filter.
I agree that it would be useful to see somewhere that a filter is applied, what that filter is, and an option to remove it. Other than that I can only applaud the effort, well done!
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Hi folks,
the first time using the network server list I also was dumbstrucked by the long list without any filter capability. Every time I log on to a multiplayer game, I wish to have a filter edit box there as well, because most time I know at least part of the server's name I want to join.
Having now read this thread and the flyspray entry, I thought I could make that better. Your approach with the additional popup box, handrake, however, didn't suite me (nothing against you, though ). Thus, I am quite in line with Stevenh. Frosch already mentioned (in FS) the main problem on the matter is that "there is no focus". Wanting to solve this issue with minimal impact to existing coding (there are too many consumers of class QueryStringBaseWindow - making an error there could have devastating side-effects), I tried to allow emulate multiple edit boxes in a window by switching a single edit box following the current focus that the user has set. Still, you are able to access the edit boxes in the coding seperately, if you are willing to use an enhanced interface which is provided.
As I thought that this issue could occur on other windows as well, I tried to encapsulate this approach in a dedicated class called MultipleQueryStringBaseWindow.
Find attached the patch file how I tried to solve it: Here's a screenshot on how this patch visualizes:
PS: "Clear" button and "history of filters" (like you know from a browser's address bar) is not available yet. The visualization that the list is filtered is done natively, as you see that there is something in the filter edit box.
the first time using the network server list I also was dumbstrucked by the long list without any filter capability. Every time I log on to a multiplayer game, I wish to have a filter edit box there as well, because most time I know at least part of the server's name I want to join.
Having now read this thread and the flyspray entry, I thought I could make that better. Your approach with the additional popup box, handrake, however, didn't suite me (nothing against you, though ). Thus, I am quite in line with Stevenh. Frosch already mentioned (in FS) the main problem on the matter is that "there is no focus". Wanting to solve this issue with minimal impact to existing coding (there are too many consumers of class QueryStringBaseWindow - making an error there could have devastating side-effects), I tried to allow emulate multiple edit boxes in a window by switching a single edit box following the current focus that the user has set. Still, you are able to access the edit boxes in the coding seperately, if you are willing to use an enhanced interface which is provided.
As I thought that this issue could occur on other windows as well, I tried to encapsulate this approach in a dedicated class called MultipleQueryStringBaseWindow.
Find attached the patch file how I tried to solve it: Here's a screenshot on how this patch visualizes:
PS: "Clear" button and "history of filters" (like you know from a browser's address bar) is not available yet. The visualization that the list is filtered is done natively, as you see that there is something in the filter edit box.
- Attachments
-
- mpfilter-crop.png (18.2 KiB) Viewed 792 times
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Adding the "Clear" button was much easier than I thought initially.
However, I really don't have a clue how we could add something like the address bar drop down you know from your web browser.
Anyway, here's an updated patch for it (against same rev. than before) which provides the clear button
However, I really don't have a clue how we could add something like the address bar drop down you know from your web browser.
Anyway, here's an updated patch for it (against same rev. than before) which provides the clear button
- Attachments
-
- mpfilter-with-clear-against-24634.patch
- patch for filtering the network list against r24634 including "clear" button as well
- (23.4 KiB) Downloaded 128 times
Last edited by Eagle_rainbow on 28 Oct 2012 21:30, edited 1 time in total.
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Adopting my learnings from a code review in this thread, I have adjusted the patch in this thread here as well.
Here's the new version:
Here's the new version:
- Attachments
-
- mpfilter-with-clear-v2-against-24634.patch
- (23.48 KiB) Downloaded 116 times
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Here are some more adjustments to the coding style to make the patch more compliant to the rules.
- Attachments
-
- mpfilter-with-clear-v3-against-24638.patch
- current patch version
* further coding style adjustments (no change in logic)
* update of revision; now: 24638 - (23.8 KiB) Downloaded 341 times
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
In today's #openttd.dev session, there was a debate on this patch. I would like to summarize the result here briefly:
- The approach with class MultipleQueryStringBaseWindow to allow multiple edit boxes in a window was rejected. Instead the developers intend to enable the GUI framework to allow multiple edit boxes in general; that is a bigger task and won't be there short-term.
- For the time being the player name available on the network list window should go to a separate window (with a button or something like this, or query string window), whilst retaining the filter on the network list window. The player name should still be visible on the window to see which name has been set. Furthermore, Yexo asked to split these two items (change player name and filtering) into two distinct patches.
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Here's the first patch (draft) as discussed above: It will remove the player's name edit box and move the change option behind a button into a QueryStringWindow to allow adding a filter edit box in a second step (patch in prep).
On implementing it, I detected the following things which I think are worthwhile discussing in the upcoming code review:
- When trying to get rid of the constant WID_NG_CLIENT (the edit box for the player's name on the screen) I detected references in script/api/script_window.hpp and thus also in script/api/game/game_window_hpp_sq. Having a short alignment with Yexo (see also IRC log at the very bottom), I also deleted these references. Thus, also the auto-generated file will be shown as changed in the patch - I think it would be better to regenerate that file (seems to be auto-generated via script/api/squirrel_export.sh).
Furthermore, Yexo was also puzzled about the publishing in sq and thus created a FS entry for that. - When writing the changed player's name, I reused the original coding to do so. There, I think I found a glitch: A comment tells that only letters would be allowed in player names. However, the coding only checks if the first letter isn't a whitespace. Digits are not checked; in the original version furthermore it is possible to also enter symbols.
I have marked the location with a FIXME comment, because I think that either the comment is wrong or the coding.
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Though I did not receive any feedback on this patch, I have updated the patch to the most current revision of trunk: Furthermore, I was able to finish up the patch for the filter edit box: Combining those two patches, the multiplayer screen now looks like this:Eagle_rainbow wrote:Here's the first patch (draft) as discussed above: It will remove the player's name edit box and move the change option behind a button into a QueryStringWindow to allow adding a filter edit box in a second step (patch in prep).
- Attachments
-
- mpfilter.png (17.08 KiB) Viewed 786 times
- MinchinWeb
- Traffic Manager
- Posts: 225
- Joined: 01 Feb 2011 12:41
- Contact:
Re: Patch : Network game list name filter
You have a growing collection of very neat patched Eagle Rainbow!
I wonder here if the tooltip cannot be simplified. What about if you were to use "Filter available network games" instead?
I wonder here if the tooltip cannot be simplified. What about if you were to use "Filter available network games" instead?
Alberta Town Names - 1500+ real names from 'Acme' to 'Zama City'
MinchinWeb's Random Town Name Generator - providing 2 million plus names...
WmDOT v13 - An AI that doubles as your highway department
MinchinWeb's Random Town Name Generator - providing 2 million plus names...
WmDOT v13 - An AI that doubles as your highway department
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Thanks for your compliment.
Concerning the tool tip label - I see what you mean. Though, personally text labels are quite smoke and mirrors to me, as long as I am able to understand what is meant, it still may be important for many others
I have changed the label as you suggested.
Here are the new patches version which are now also against the most recent trunk revision: Is there further feedback which I may incorporate on that topic here?
Concerning the tool tip label - I see what you mean. Though, personally text labels are quite smoke and mirrors to me, as long as I am able to understand what is meant, it still may be important for many others
I have changed the label as you suggested.
Here are the new patches version which are now also against the most recent trunk revision: Is there further feedback which I may incorporate on that topic here?
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Frosch just commited a set of patches to trunk which enables the GUI framework to support multiple edit boxes on the same window. Making use of that feature, I have now adjusted the multiplayer filter patch to bypass the "remove_player_name". The window now looks like this: And here's the patch against the most recent revision of trunk:Eagle_rainbow wrote:
- The approach with class MultipleQueryStringBaseWindow to allow multiple edit boxes in a window was rejected. Instead the developers intend to enable the GUI framework to allow multiple edit boxes in general; that is a bigger task and won't be there short-term.
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Here's an updated version of the patch after a code review in #openttd.dev
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Here's the next version...
- Attachments
-
- mpfiltermultieditbox-v5-against-r24751.patch
- adds a filter edit box to the multiplayer screen
against trunk r24751 - (13.5 KiB) Downloaded 124 times
Re: Patch : Network game list name filter
Finally I have a test-patch for multiple editboxes! (there are still some issues with the OSK.)
About this patch: I fixed some issues, mostly about the window layout and drawing filtered list.
However, there are still several bugs with the list while it is filtered. Mostly everything using "list_pos" fails while the list is filtered. E.g. ScrollToSelectedServer or the keyboard navigation.
About this patch: I fixed some issues, mostly about the window layout and drawing filtered list.
However, there are still several bugs with the list while it is filtered. Mostly everything using "list_pos" fails while the list is filtered. E.g. ScrollToSelectedServer or the keyboard navigation.
- Attachments
-
- mpfiltermultieditbox-v51-r24760.diff
- (21.18 KiB) Downloaded 110 times
⢇⡸⢸⠢⡇⡇⢎⡁⢎⡱⢸⡱⢸⣭⠀⢸⢜⢸⢸⣀⢸⣀⢸⣭⢸⡱⠀⢰⠭⡆⣫⠰⣉⢸⢸⠀⢰⠭⡆⡯⡆⢹⠁⠀⢐⠰⡁
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Thanks for testing my stuff.frosch wrote: However, there are still several bugs with the list while it is filtered. Mostly everything using "list_pos" fails while the list is filtered. E.g. ScrollToSelectedServer or the keyboard navigation.
I will have a look on the "list_pos" thingy soon and revert back to this thread with updates.
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Here we go with my update.Eagle_rainbow wrote: I will have a look on the "list_pos" thingy soon and revert back to this thread with updates.
In the previous patches 'list_pos' is not handled properly. This leads to a very strange (not to say erratic ) behaviour when using the cursor keys for example. Also sorting could happen to become quite strange. Having had a second thought, I started to dislike my initial approach pushing down the filtering concept to the _network_game_list. This linked list is more or less a technical thing whilst sorting, filtering and list_pos stuff tend to be "lightweight" GUI-related things. This gets even more apparent, if you are looking at 'GUIGameServerList' which also has a capability of filtering.
I now reimplemented the entire patch to make use of FilterFunctions in the server list. This had the side-effect that BuildNetworkGameList became a bit more powerful, as it now has to handle sorting as well (which IMHO isn't a bad idea either, because it makes life easier keeping the list consistent). As there is a clear difference between _network_game_list and the servers vector now, I also changed the name of BuildNetworkGameList to BuildGUINetworkGameList to stress that difference (previously, the two lists where kept in sync, especially with regard to sorting which isn't worthwhile anymore due to filtering). This resolved a lot of issues which otherwise I would have had to "code around" and would have been evil to understand later on (for example: changing sort order when list is filtered).
Please find attached the newest version of this patch: However, during testing this version, I found one thing that still looks inconsistent to me: Though the cursor keys are working properly now, the effect of pressing Home or End now depends on the current focus: If the focus is with one of the edit boxes, you'll change the cursor position within the edit box. If the focus is with the matrix, you'll navigate through the server's list. What makes it really a mess, is the fact, that the PageUp/Down keys are working regardless of the current focus (and will change the list_pos) . My first thought on this was to disable the Home and End keys for the server list entirely - but that is most likely due to the fact that I don't use them at all (and before having had read the coding, didn't even know that this feature existed ). However, ignorance isn't a good reasoning after all. The other alternative is to disable PageUp/Down, if the focus is with the edit boxes. Yet, I don't have a preference on how to resolve that.
Do you have another good idea?
PS: We might need to have another round of reviews on it later, as several things have changed again. Yet, I didn't have a full passthrough on the diff myself, so there still could be some quirks in there (hopefully coding style only).
Re: Patch : Network game list name filter
While having a look at your patch, I found a way to trigger an assert:
I do however not see it as a problem. Yes, someone may wonder why page up/down + arrow keys works but not home/up to scroll the list while not being aware of the edit box being focused. However, I don't see any good fix for that. And in that situation, I think it is better to let arrows+page up/down work than not.
(however, other devs may have a different opinion)
- (press Find servers to fill list, unless you have enough manual servers)
- Select an item in the list
- Type a filter that causes the selected item to disappear
- Press arrow down/up
- => assertion failed
This would also apply to arrow up/down if you think it is a problem.Eagle_rainbow wrote:What makes it really a mess, is the fact, that the PageUp/Down keys are working regardless of the current focus (and will change the list_pos)
I do however not see it as a problem. Yes, someone may wonder why page up/down + arrow keys works but not home/up to scroll the list while not being aware of the edit box being focused. However, I don't see any good fix for that. And in that situation, I think it is better to let arrows+page up/down work than not.
(however, other devs may have a different opinion)
My OpenTTD contributions (AIs, Game Scripts, patches, OpenTTD Auto Updater, and some sprites)
Junctioneer (a traffic intersection simulator)
Junctioneer (a traffic intersection simulator)
-
- Engineer
- Posts: 59
- Joined: 13 Oct 2012 20:27
- Location: Germany
Re: Patch : Network game list name filter
Thanks for pointing that out. Here's a new version which fixes this problem: It also incorporates some minor enhancements (mainly on comments).Zuu wrote:While having a look at your patch, I found a way to trigger an assert:
- (press Find servers to fill list, unless you have enough manual servers)
- Select an item in the list
- Type a filter that causes the selected item to disappear
- Press arrow down/up
- => assertion failed
Who is online
Users browsing this forum: No registered users and 48 guests