Times are UTC Toggle Colours
00:17:36 *** sgtbigman has joined #openttd.dev 04:21:43 *** sgtbigman has quit IRC 04:31:55 *** sgtbigman has joined #openttd.dev 07:06:55 *** sgtbigman has quit IRC 09:23:42 *** Zuu has joined #openttd.dev 09:23:42 *** ChanServ sets mode: +v Zuu 11:21:48 <Zuu> Filter-patch: https://devs.openttd.org/~zuu/online_content/10-filter-by-type-or-selected.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 #openttd.dev 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: https://devs.openttd.org/~zuu/online_content/comment-fix.patch 12:48:08 *** andythenorth has joined #openttd.dev 12:48:08 *** ChanServ sets mode: +v andythenorth 12:49:50 *** Alberth has joined #openttd.dev 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 #openttd.dev 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 https://bugs.openttd.org/task/5470 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: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking" 17:50:28 *** sgtbigman has joined #openttd.dev 19:01:45 *** Alberth has left #openttd.dev 19:16:58 *** sgtbigman has quit IRC 20:04:12 *** sgtbigman has joined #openttd.dev 21:53:24 *** andythenorth has left #openttd.dev 22:34:43 *** sgtbigman has quit IRC 22:54:53 *** DorpsGek changes topic to "OpenTTD Dev Channel || Latest SVN: r27446 || Logs: http://webster.openttdcoop.org/?channel=openttd.dev || Voice (talk-right) upon request via #openttd; make sure you are registered to NickServ before asking"