Log for on 14th November 2015:
Times are UTC Toggle Colours
00:17:36  *** sgtbigman has joined
04:21:43  *** sgtbigman has quit IRC
04:31:55  *** sgtbigman has joined
07:06:55  *** sgtbigman has quit IRC
09:23:42  *** Zuu has joined
09:23:42  *** ChanServ sets mode: +v Zuu
11:21:48  <Zuu> Filter-patch:
11:22:40  <Zuu> It does not add any new GUI. It only changes thet behoviour of online content window when opened from eg. play scenario to only show items of other type than the main type when they have been (auto)selected.
12:31:39  *** Supercheese has quit IRC
12:32:12  *** Supercheese has joined
12:32:12  *** ChanServ sets mode: +v Supercheese
12:42:49  <Zuu> Unless, I'm mistaken, this patch fixes a negoation error in a comment:
12:48:08  *** andythenorth has joined
12:48:08  *** ChanServ sets mode: +v andythenorth
12:49:50  *** Alberth has joined
12:49:50  *** ChanServ sets mode: +v Alberth
12:50:14  <Alberth> hihi, not enough activity here to join every time :)
12:50:28  <Zuu> I have it on auto-join :-)
12:50:30  <Alberth> yep, comment fix seems to be correct
12:51:16  <Alberth> auto joins tends to fail if you don't have interwebz when you log in :p
12:53:24  *** frosch123 has joined
12:53:24  *** ChanServ sets mode: +v frosch123
12:53:29  <Alberth> woep
12:53:36  <Zuu> Hello frosch123
12:53:42  <frosch123> hoi, how rare
12:53:55  <andythenorth> someone is coding something
12:54:46  <Alberth> empty line after class NetworkContentListWindow shouldn't be there
12:54:54  <Zuu> Someone is making a full checkout of svn because his old was of too old format or something.
12:55:53  <Alberth> :)
12:56:06  <frosch123> svn upgrade would be easier :p
12:56:13  <frosch123> "svn upgrade"
12:56:20  <Zuu> That asumes you know svn these days
12:56:30  <Alberth> it doesn't tell you?
12:56:35  <frosch123> it told me on the command line, to run that command
12:57:02  <Alberth> yeah, me too, except then it killed my working-copy :p
12:57:27  <Alberth> perhaps now is a good time to re-align all those variable comments?
12:58:16  <Zuu> Possible, but preferable in a second patch I suppose
12:58:18  <Alberth> TagNameFilter  comment is out of date, probably
12:58:52  <Zuu> What is it missing?
13:00:01  <frosch123> hmm, i thought we already had such filter
13:00:20  <Alberth> hmm, no, code is just confusing in not using the content type
13:02:20  <Alberth> last condition value in TypeOrSelectedFilter  can be simply returned
13:02:36  <frosch123> hmm, ah, the new thing is that it still shows dependencies, also when only showing one type otherwise, right?
13:02:44  <Zuu> frosch123: There is a filter on what content type to include when building the list. Or actuallly in what content _network_content_client provides. What the patch does is hiding content of wrong type unless it has been (auto)selected.
13:03:31  <frosch123> yeah, trunk already can do that for heightmaps, because they have no dependencies
13:04:04  <frosch123> and for newgrf, because they only depend on newgrf
13:04:40  <Zuu> If you open online content in trunk from play scenario, you get all sorts of content but scenarios in top (in English)
13:05:01  <frosch123> hmm, but if i open it from the gs/ai gui, i get gs/ai and the "split" scenario
13:05:25  <frosch123> are you sure it doesn't already work?
13:05:33  <Zuu> Split may still be using the cyclic dependency "hack" and not regular musa depnedencies that only go in one way.
13:05:34  <Alberth> if (this->filter_data.type != CONTENT_TYPE_END) {    <-- do you need this, given if (filter.type == CONTENT_TYPE_END) return true; ?
13:07:13  <Alberth>   /* comment lines like these should be normal english sentences with uppercase and a dot */
13:07:47  <frosch123> bananas code is such a mystery to me
13:08:04  <frosch123> fios_gui calls ShowNetworkContentListWindow(NULL, CONTENT_TYPE_SCENARIO); , why does it show other stuff?
13:08:29  <frosch123> did someone make a hack on the server side to also return other stuff?
13:08:47  <Alberth>  or CONTENT_TYPE_END   <-- Add '#' , like  "or #CONTENT_TYPE_END"  to link to the definition
13:08:59  <frosch123> oh wait, it doesn't display all stuff
13:10:47  <Alberth>  !this->filter_data.string_filter.IsEmpty() || this->filter_data.type != CONTENT_TYPE_END    <-- fold that in a function/method?
13:11:50  <Zuu> <Alberth> if (this->filter_data.type != CONTENT_TYPE_END) {    <-- do you need this, given if (filter.type == CONTENT_TYPE_END) return true; ?   <---- it is only needed for optimization. With this check, we do not even loop all content and return true.
13:12:36  <Alberth> +	 * when a type other than CONTENT_TYPE_END is given,   <-- Missing "W", also, is it part of the @param above it? Please indent it or make it clear otherwise
13:13:34  <Zuu> It is part of the param. I'll indent it.
13:15:05  <Alberth> ForceRebuild implies ForceResort?  If so, you can check for rebuild first
13:15:19  <frosch123> haha, that patch adds the first use of GUIList::SetFilterType :p
13:15:39  <Zuu> frosch123: Yeah I found it interesting it was not used yet. :-)
13:16:09  <frosch123> well, i never used it, because it made no sense to me :)
13:16:26  <frosch123> i always put everything into a single fitler function, even when filtering by multiple criterions
13:16:49  <frosch123> i couldn't see any sense in setting SetFilterType
13:17:39  <Zuu> Alberth: when indenting param text, do you just indent by two(?) spaces? or to get a nice left margin that follow with the first word after @param on the first row?
13:18:14  <Zuu> frosch123: I was wondering if I should just put it in one function too. But then we have this SetFilterType facility.. :-)
13:18:26  <Alberth> +		this->UpdateFilterState();    <-- do 'ScrollToSelected' above it, and then potentially force a rebuild?  isn't it better the other way around?
13:19:07  <frosch123> +	void UpdateFilterState() { <- missing \n
13:19:16  <Alberth> Zuu: don't think we have a code style for that, but not against the left margin at least would be great
13:19:26  <Zuu> A forced rebuild doesn't actually hapen directly. It sets a bit. It is first at OnInvalidateData, the list is actually rebuilt.
13:21:35  <Zuu> Hmm BulidContentList will scroll to selected, so I could UpdateFilterState() first, then check if the rebuild bit is set, and only if not, scroll to selected.
13:23:35  <frosch123> how about adding a bool parameter "void UpdateFilterState(bool editbox_changed)", and also call it from OnEditboxChanged?
13:23:54  <frosch123> that woudl save deduplication of the SetFilterState condition
13:24:17  <Zuu> I updated the patch on /~zuu/
13:24:23  <frosch123> possibly it could also be called from the constructor
13:25:45  <Zuu> It can as well be used in OnEditboxChanged. It would reduce duplication of SetFilterState, but may call ForceRebuild() twice, but that only set a bit so it may not be a big issue.
13:28:06  <frosch123> that's what i mean with the bool parameter
13:28:16  <frosch123> it would force the update, even if the filterstate does not change
13:28:18  <frosch123> same in constructor
13:30:02  <Zuu> Ah, that can be done.
13:30:55  <Zuu> But perhaps rename editbox_changed into force_rebuild if it is used in the constructor too.
13:31:15  <frosch123> yup :)
13:32:45  <Zuu> Or UpdateFilterState could just return a bool if fiter state was updated. Will duplicate a bit more code to rebuild list, but make it more clear what UpdateFilterState does from its name.
13:33:34  <Zuu> Eg. remove the whole rebuilding from UpdateFilterState.
13:40:00  <Zuu> Hmm that actually made sense in code. I think only two places need to check the return value. One need to force rebuild + invalidate and the other only force rebuild.
13:44:43  <Zuu> Updated patch on /~zuu/.
13:47:34  <frosch123> +		if(new_state != old_params.state) { <- missing space after "if"
13:48:45  <frosch123> looks fine :)
13:50:05  <Zuu> too little coffe or so :-)
13:51:49  <Alberth> /me gives another coffee to zuu
13:52:14  <Zuu> I think actually I should get out and catch some daylight before its too late in an hour or so. :-)
13:52:26  <Alberth> makese sense :)
15:18:00  <Zuu> I'm temped to close with my patch. It do put forward an issue about having multiple versions of libraries visible in the main list due to them being dependencis, which my patch doesn't address. On the other hand, do we want to hide things from the main list?
15:22:26  <Zuu> Looking at network_content.cpp it seems the server do just pass back old versions at its own descision. I'm not sure we can detect on the client at the moment which content is 'old' in this sense. On the other hand that doesn't make the FS task invalid just because it is hard to solve.
15:28:23  <Zuu> Hmm.. the user screenshots indicate he/she is using the main list. And that is not dealed with in my patch. So I leave it open.
15:30:24  <Rubidium> the server doesn't make a decision, in the first batch it sends everything that is marked "active" in the database
15:34:23  <Rubidium> in the client on SERVER_INFO it will call CheckDependencyState which will call DownloadContentInfo to download the content info for the dependencies
15:34:33  <Rubidium> so all the dependency handling is client side
15:35:19  <Zuu> So then cilent could set a flag on those dependencies it receive after having received active content.
15:36:58  <Rubidium> there's a requested instance variable that might be useful, but I'm not quite sure
15:41:03  <Zuu> A problem may be how to when receiving content info data from server know how/why it was requested. A global state may be unreliable.
15:41:26  <Rubidium> the server doesn't tell you
15:42:21  <Rubidium> and possibly you can't even really know in the client yourself
15:42:49  <Rubidium> since it's just a stream of contentinfos coming from the server
15:42:57  <Zuu> I found the code that make the requests. In JavaScript it would be easy, but here it may need to be passed along to the server and back again, unless there request key/index passed along with the request and passed back.
15:44:10  <Rubidium> and to make matters worse, if the first thing that is returned in the "content list" has dependencies that are further along in the list, then it will just request them twice
15:44:45  <Zuu> oh
15:44:52  <Rubidium> so the "requests" variable might be misleading
15:46:00  <Zuu> I think I will pass on this issue. The patch deal with content list limited to one type, but is of no help for the general content list.
16:17:50  <frosch123> @services op
16:17:50  *** ChanServ sets mode: +o DorpsGek
16:18:02  <frosch123> @voice Eearslya
16:18:02  *** DorpsGek sets mode: +v Eearslya
17:45:12  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27445 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"
17:50:28  *** sgtbigman has joined
19:01:45  *** Alberth has left
19:16:58  *** sgtbigman has quit IRC
20:04:12  *** sgtbigman has joined
21:53:24  *** andythenorth has left
22:34:43  *** sgtbigman has quit IRC
22:54:53  *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27446 || Logs: || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"

Powered by YARRSTE version: svn-trunk