[Patch] Filter Sign List (by typing a filter string) - v40

Zuu · Post by **Zuu** » 08 Feb 2009 17:23

Have not yet checked on what you did (upgrading my workstation from etch to lenny right now). But regarding the two patches, the widget focus patch is available as a seperate patch on FS #2297. This filter sign list patch, I have not yet made a patch entry for as it currently as awaiting the widget focus patch. At least in my eyes the widget focus patch or something similar is needed before more windows using edit boxes should be introduced in the game. Adding edit boxes to windows in the main menu is still okay as there you only use one window at the time anyways.

Using hg queues I have a filter sign list patch that do not include the widget focus patch, but instead need the widget focus patch to be applied. This patch I have posted some posts ago. But I don't see any big use in it right now so I have not posted it along with all updates.

Roujin · Post by **Roujin** » 10 Feb 2009 11:51

Have not yet checked on what you did (upgrading my workstation from etch to lenny right now).

Here's a short summary:

The changes are in signs_gui.cpp.

The GUIList got a second template type, which is by default char*, and specifies what type is passed to the filter function.
I define a little struct FilterInfo, that contains a char* and a boolean (for the case sensitive decision), and tell that the second template type for the GUIList should be this FilterInfo.

Code: Select all

+struct FilterInfo {
+	const char *string;
+	bool case_sensitive;
+};
+
 struct SignList {
-	typedef GUIList<const Sign *> GUISignList;
+	typedef GUIList<const Sign *, FilterInfo> GUISignList;

Now the filter function gets a FilterInfo passed, instead of a char*. From the filterInfo, I can extract both the string and the boolean with filter_info.string and filter_info.case_sensitive respectively. This I use for making the decision about case sensitiveness here inside the filter...

Code: Select all

+	/** Filter sign list by sign name (case sensitive setting in FilterInfo) */
+	static bool CDECL SignNameFilter(const Sign * const *a, FilterInfo filter_info)
+	{
+		/* Get sign string */
+		char buf1[64];
+		SetDParam(0, (*a)->index);
+		GetString(buf1, STR_SIGN_NAME, lastof(buf1));
+
+		return (filter_info.case_sensitive ? strstr(buf1, filter_info.string) : strcasestr(buf1, filter_info.string)) != NULL;
+	}

... instead of having two seperate filters. And so the FilterSignList function also does not have to set the filter to use anymore, but just create a FilterInfo out of the string and the boolean and pass it to the Filter.

Code: Select all

+	void FilterSignList()
+	{
+		FilterInfo filter_info = {this->filter_string, this->match_case};
+		this->signs.Filter(&SignNameFilter, filter_info);
+	}

That's all I did.

But I don't see any big use in it right now so I have not posted it along with all updates.

Well now that has changed

Congrats again for your widget focus patch.

Zuu · Post by **Zuu** » 26 Feb 2009 18:21

The long awaited update of this patch to current trunk. Or well, actually I believe (but have not tested) that the last stand-alone-patch I published some time ago applies on trunk now that widget focus has been added.

But anyways, here is version 21 which is now only 370 lines and only affects 3 files.

Changes:
* Brought up to r15589 (actually no changes at all to the actual patch, just made an update, removed the widget_focus patch from HG queues and applied my filter patch without any conflicts.

)
* Added the changes to GUIList filter code by Roujin
* When the (blue) selected item gets out of the list the Return key didn't do anything. Now it higlights the first item in the list, as visually nothing is selected.

I think I will rewrite the selected item code so that it use a pointer to selected item instead of a index item, so that it works similar to how the select item code in the content download window works.

2007Alain2007 · Post by **2007Alain2007** » 26 Mar 2009 23:08

NOW IN KGAT PATCHPACK http://www.tt-forums.net/viewtopic.php? ... 32&start=0

2007Alain2007 · Post by **2007Alain2007** » 29 Mar 2009 01:32

C:/msys/1.0/home/Alainspc/ottdsrc/trunk/src/signs_gui.cpp:115: error: multiple definition of `enum SignListWidgets'
C:/msys/1.0/home/Alainspc/ottdsrc/trunk/src/signs_gui.cpp:105: error: previous definition here
make[1]: *** [signs_gui.o] Error 1
make[1]: Leaving directory `/home/Alainspc/ottdsrc/trunk/objs/release'
make: *** [all] Error 2

any idea on what the error is there just updateed the signs_gui.cpp

Zuu · Post by **Zuu** » 29 Mar 2009 19:42

2007Alain2007 wrote:any idea on what the error is there just updateed the signs_gui.cpp

SVN Tortoise told me that there has been two recent changes to signs_gui.cpp, where the first one is related to the error you got. However, I think you should watch your patch tool more carefully for rejects. I got a whole bunch of rejects on last trunk. Ignoring those and trying to compile for sure will give you compile errors.

Thanks for highlighting the conflict with trunk though.

Patch update
I've updated the patch to apply to trunk again according to the changes made to trunk. Also fixed so that up/down arrows don't select previous/next sign when the edit box is not selected. When previous/next sign where selected the list widget was not marked dirty. Somehow this only caused problem for me now, but not before. Might be changes somewhere else related to repainting that brought this problem up to the surface. Or it was just my local copy of the patch version 21 that was broken. Anyway here is version 22 for r15891:

filter_sign_list22_r15891.patch: (12.48 KiB) Downloaded 204 times

2007Alain2007 · Post by **2007Alain2007** » 29 Mar 2009 20:29

thanks for this update

Zuu · Post by **Zuu** » 20 Apr 2009 22:16

Here is version 22 of the filter sign list patch backported for 0.7.0. I've compiled and tested it on MSVC so most obvious things have been checked and fixed. (I first forgot to copy one widget definition resulting in a missing button

- But I have tested the filter functionality too so that should work as well

)

Nickman · Post by **Nickman** » 06 Oct 2009 22:25

After a request from Ammler (and other people

) I decided to update this patch to the latest revision with the new NestedWidget system.
In the attachement you can find the patch for revision r17733.

Nothing functional has changed, only some small code changes to fit the new widget system have been applied.
If you have any request/suggestion/ideas for this patch, I will try to take a look at them, just post them here.

---------

Bug in trunk currently causes loading of games to fail, will post an update when fixed

.

Zuu · Post by **Zuu** » 07 Oct 2009 14:42

Haven't looked on what you have done yet, but thanks for updating it.

I don't know if the nested widget system makes that impossible, but if it is possible I would suggest making the edit box one pixel taller so that the black line below it is removed.

Nickman · Post by **Nickman** » 07 Oct 2009 15:32

I updated the patch to the latest revision and loading of savegames works again.
I also managed to get rid of the black line underneath the edit box

.

Ammler · Post by **Ammler** » 07 Oct 2009 19:39

Many thanks for this update Nickman, I found a small glitch, try to close the sign list with ESC or to switch focus to chat input while this is open.

Greets
Ammler

Nickman · Post by **Nickman** » 07 Oct 2009 21:00

If you mean that the chat window won't open when you press "return" and that escape doesnt close the window like it used to, this is not my glitch but it is functionality from the original patch

.

The esc button now clears the filter string and the return button shows you the location of the first sign in the list.
If you dont want/like these features, they are easily removable and it would indeed make the window more consistent to the regular behaviour of other windows.

2007Alain2007 · Post by **2007Alain2007** » 07 Oct 2009 21:02

I think the less it changes the better it be

Zuu · Post by **Zuu** » 08 Oct 2009 09:27

Nickman wrote:If you mean that the chat window won't open when you press "return" and that escape doesnt close the window like it used to, this is not my glitch but it is functionality from the original patch .

That is not a functionality of Filter Sign List but of the Widget Focus patch which got merged with trunk 9-12 months ago or so.

Nickman wrote:The esc button now clears the filter string and the return button shows you the location of the first sign in the list.
If you dont want/like these features, they are easily removable and it would indeed make the window more consistent to the regular behaviour of other windows.

If these features are removed from this patch you will still not be able to use the enter/esc-key to control the chat window while the edit box of the sign window is focused.

Code: Select all

	virtual void OnOpenOSKWindow(int wid)
	{
		ShowOnScreenKeyboard(this, wid, SLW_CLOSEBOX, 0);
	}

Any particular reason for adding this code? You only need to implement OnOpenOSKWindow if you don't want the last two ShowOnScreenKeyboard parameters to be 0, 0. Adding SLW_CLOSEBOX will not make the sign list window close when a user hit the OK-button on the OSK-window since if you read the OSKWindow code you'll see that it will call Window::OnClick(widget) which does not handle the close box events since those are handled in the window system code. On top of that I would suggest not closing the sign list window when a user cancels editing the filter string by pressing the cancel button on the OSK window.

I see though that for future work it might be an idea to combine the concept of confirming/canceling text edit input by keys and by OSK-buttons to one single interface for windows. Eg. Window::OnEditBoxConfirm(widget) and Window::OnEditBoxCancel(widget). But that lays outside the scope of this patch.

Nickman · Post by **Nickman** » 09 Oct 2009 15:29

I have updated the patch to the latest revision and made some small changes.

- Removed the "OnOpenOSKWindow" function, it was indeed not needed, I porbably added it because I was having problems before

.
- Minor changes to the SelectNextSign/SelectPrevSign functions
- Fixed a minor "glitch" when you had a sign selected and entered a new filter causing the list to shrink. In this case the selected_sign index was not changed to the biggest possible number.

See attachement

.

Terkhen · Post by **Terkhen** » 27 Oct 2009 21:38

Thank you for your work updating this patch, Nickman

I have attached a diff file updated to current trunk. No changes besides correcting a minor reject caused by changes in r17853.

Edit: Another update for a new reject.

Zuu · Post by **Zuu** » 01 Jan 2010 13:25

Since FS#3256 was included to trunk yesterday, I've decided to update the Filter Sign List patch.

One thing I don't understand in both last Nickman's and Terken's patch. Why do you change the method used to calculate the minimal size of the sign list window? If you have a better idea of how that should be done, that is IMHO not the scope of this patch to do since that could be done as a patch of trunk. I've not compiled Terken's patch, but as I understand it it actually changes the minimum size limit to one sign. Something that I think is irrelevant of adding filtering capabilities to the window.

I could be wrong, but I don't see why these code changes needs to be part of this patch. Please enlighten me if you think I am wrong.

filter_sign_list_r18283.patch wrote:

Code: Select all

@@ -165,11 +385,8 @@

 	{
 		switch (widget) {
 			case SLW_LIST: {
-				Dimension spr_dim = GetSpriteSize(SPR_COMPANY_ICON);
-				this->text_offset = WD_FRAMETEXT_LEFT + spr_dim.width + 2; // 2 pixels space between icon and the sign text.
 				resize->height = max<uint>(FONT_HEIGHT_NORMAL, GetSpriteSize(SPR_COMPANY_ICON).height);
-				Dimension d = {this->text_offset + MAX_LENGTH_SIGN_NAME_PIXELS + WD_FRAMETEXT_RIGHT, WD_FRAMERECT_TOP + 5 * resize->height + WD_FRAMERECT_BOTTOM};
-				*size = maxdim(*size, d);
+				/* Minimal width of SLW_LIST is the maximal length of a sign + its offset. */
 			} break;
 
 			case SLW_CAPTION:
@@ -196,20 +413,26 @@

 	}
 };
 
+const int SignListWindow::text_offset = 22;
+
 static const NWidgetPart _nested_sign_list_widgets[] = {
 	NWidget(NWID_HORIZONTAL),
 		NWidget(WWT_CLOSEBOX, COLOUR_GREY),
 		NWidget(WWT_CAPTION, COLOUR_GREY, SLW_CAPTION), SetDataTip(STR_SIGN_LIST_CAPTION, STR_TOOLTIP_WINDOW_TITLE_DRAG_THIS),
 		NWidget(WWT_STICKYBOX, COLOUR_GREY),
 	EndContainer(),
-	NWidget(NWID_HORIZONTAL),
-		NWidget(WWT_PANEL, COLOUR_GREY, SLW_LIST), SetMinimalSize(WD_FRAMETEXT_LEFT + 16 + MAX_LENGTH_SIGN_NAME_PIXELS + WD_FRAMETEXT_RIGHT, 50),
-							SetResize(1, 10), SetFill(1, 0), EndContainer(),
+  	NWidget(NWID_HORIZONTAL),
+		NWidget(WWT_PANEL, COLOUR_GREY, SLW_LIST), SetMinimalSize(WD_FRAMERECT_LEFT + SignListWindow::text_offset + MAX_LENGTH_SIGN_NAME_PIXELS + WD_FRAMERECT_RIGHT, 54), SetResize(1, 10), SetFill(true, true), EndContainer(),

Nickman · Post by **Nickman** » 01 Jan 2010 15:21

I think that in the "_nested_sign_list_widgets" part, the only change that was made was to remove a number from the calculation and just use the static constant that was defined.

But I follow your reasoning that this patch is not focused on changing that type of calculation, I think I probably changed it according to my own idea not looking at the old/existing piece of code. So it could just as well be changed back to the old calculation method.

Happy new year btw!

Zuu · Post by **Zuu** » 01 Jan 2010 15:45

Update

Based on the filter_signs_r17747.diff patch by Nickman.

Changes:

Made the widget definition more adopted to the features of nested widgets. No minimum size of the buttons and the edit box is now resizable so that the window can be resized.
Reverted to the in trunk code for determining the minimum size of the list widget.
Use Window::UnfocusFocusedWidget that was introduced in r18675
Don't paint the edit box widget when the window is shaded
Removed spaces/tabs at the end of lines added / changed by the patch
The clear filter button is now disabled when the filter string is zero-length. (when a non-zero length filter string is entered, the button become enabled)

filter_sign_list27_r18678.patch: (12.15 KiB) Downloaded 166 times

Code review
I'm not really happy with the InitializeFilterTextWidget() function being called both for initializing the text widget and for clearing the text in it. I don't know enough about the string system in OpenTTD to make something really good here, so if someone has better knowledge some help on this matter would be welcome.

Other comments on the code is also welcome. If anyone on the development team is interested to review the code for a possible future trunk inclusion I can split the patch into two parts. One containing the basic filter sign list part and the other one containing the code for selecting a sign in the result using the arrow keys (and then view that sign when the user press the return key).

Other ideas I have is to leave the sign window as it is and introduce a new window that contains all signs, station names, towns etc. which will have the filtering feature of this patch as well as buttons at the top for enabling/disabling the different sources of strings.

Transport Tycoon Forums

[Patch] Filter Sign List (by typing a filter string) - v40

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Re: [Patch] Filter Sign List (by typing a filter string)

Who is online