Time Nick Message 00:16 paramat updated game#1076 00:16 ShadowBot https://github.com/minetest/minetest_game/issues/1076 -- Fire: Enable permanent flame above coalblock by paramat 00:57 paramat will merge game#1049 in a moment 00:58 ShadowBot https://github.com/minetest/minetest_game/issues/1049 -- Fix '-' glitch and add creative.formspec_add by tenplus1 01:02 paramat merging .. 01:13 OldCoder rubenwardy, up early or late? 01:19 paramat merged 01:20 paramat all game milestones cleared 01:23 KaadmY release on the 11th? 01:24 KaadmY why not the 13th, DOOM4's releasing that date, too :] 01:27 paramat maybe 11th, depends on issues 01:27 Zeno` yeah we could shadow their release because everyone will be looking at ours! 01:27 KaadmY yeah, totally 01:28 KaadmY a reboot of the grandfather of FPSes from over 20 years ago, or a small, uninteresting sandbox block-based game? 04:38 Zeno` nobody objected to #4090 so I'll merge it within 30 minutes 04:38 ShadowBot https://github.com/minetest/minetest/issues/4090 -- Escapes fix by Ekdohibs 04:41 Zeno` is it just me or do the milestone issues just keep increasing in number? 04:44 hmmmm lol. 04:44 hmmmm i have a quick question 04:44 hmmmm would it be more appropriate for getDropDownValues to return a const ref instead? 04:45 Zeno` hmm 04:45 hmmmm it doesn't really matter all that much since it's UI code, but this is very inefficiently done 04:46 Zeno` nah it should 04:46 Zeno` I think 04:46 * Zeno` looks where it's used 04:47 hmmmm adding the fields to the map isn't much better lol 04:47 hmmmm so there's an alloc for a temporary std::vector, and then there's an alloc for each string that's in the vector it's being copied from, and then all of the std::strings are copied 04:48 hmmmm temp rvalue created 04:48 hmmmm gets passed along (by value?? that's another copy!) to push_back, when then copies it a third time 04:48 Zeno` which line were you referring to in your first question? 04:48 hmmmm just... 04:48 hmmmm the whole thing 04:48 hmmmm the whole thing lol 04:49 Zeno` I can't see where a reference should be returned instead (in the *changed* code) 04:49 hmmmm guiFormSpecMenu.cpp:243, i'd make that return a const std::vector & 04:50 Zeno` I am scared of the GUI code 04:50 Zeno` things go out of scope all the time lol 04:50 hmmmm then at :906-913, i'd push_back a blank std::pair and then get a ref to the last item you just added, and push it onto that constructed vector 04:51 hmmmm well 04:51 hmmmm nevermind about scope, since the lifetime of your dropdowns is the lifetime of the GUIFormSpecMenu itself 04:51 Zeno` I'll leave it for now, think more and discuss with nore about it 04:51 hmmmm you don't need to worry about data races either since all the GUI code is on one thread 04:51 hmmmm right, that's the thing though 04:52 Zeno` seems that my comment was deleted anyway 04:52 hmmmm making this kind of change would no doubt be technically better, but you have to think more about it, you have to loop back with the original developer, re-review, etc. 04:52 Zeno` wtf 04:52 Zeno` I spoke the other night and commented I'm sure 04:52 hmmmm whereas with this version of the commit you can commit it immediately with no involvement or extra brainpower 04:52 Zeno` maybe I *have* gone insane (properly) 04:53 hmmmm at the cost of having horribly inefficient STL code 04:53 hmmmm so.. it's up to you 04:53 hmmmm that's my comment on it 04:53 Zeno` nah it's cool 04:53 Zeno` I can't even see my own comment 04:56 Zeno` meh +O3 will fix it :-D 04:56 * Zeno` hides 05:31 nore hmmmm: maybe it's inefficient, but it never contains more than a few values anyway; and I was trying to match the style of the surrounding code 05:33 nore Especially, using a vector of pairs instead of a map is because there are is no comparison operator for FieldSpec I think 05:40 * nore gets up and updates his code anyway 05:59 nore Zeno`: hmmmm: updated 06:00 nore it now returns a pointer to the vector, which is NULL if the dropdown is not found; this prevents crashes if the dropdown is not found 06:00 nore also, I implemented hmmmm's suggestion to avoid copies 06:08 Zeno` return NULL 06:09 Zeno` I can handle the function prototype default values being = 0 because it's how it is in the rest of the code, but not there :p 06:09 Zeno` other than that I looks good and I think you should merge 06:10 nore ah, indeed 06:10 * nore again copied the code above it 06:10 Zeno` yeah I'd change it anyway 06:10 Zeno` just so it at least gradually gets fixed heh 06:11 nore will merge after the change to NULL if you think it's good 06:11 nore should I squash the commits? 06:11 nore (or leave the first two separate from one another, since they fix different issues) 06:12 Zeno` I think two is appropriate 06:12 Zeno` not 3 though :P 06:12 nore of course :) 06:13 Zeno` heh 06:13 hmmmm yeah looks good to me 06:14 hmmmm but how do you know that dropdown_values has a valid 'selected' index 06:15 nore hmmmm: I don't check it :/ 06:15 hmmmm if (dropdown_values && dropdown_values->size() > selected) { .. 06:15 nore I assume if the dropdown is found, it's ok 06:15 nore but I should maybe change that indeed 06:16 hmmmm you use an unsigned int for pushing back the dropdown values but you use a u32 for iterating through the dropdown list 06:16 hmmmm shouldn't it be consistent? 06:19 Zeno` it depends. Perhaps inconsistent *is* consistent in that file 06:19 nore let me see 06:19 nore anyway, I added the check 06:19 hmmmm no wait 06:19 hmmmm don't cast size() to a signed integer 06:19 nore why? 06:19 hmmmm because what if selected == -1 or something due to some convention 06:20 nore yeah, that's why I added the selected >= 0 check too 06:20 hmmmm oh 06:20 hmmmm hmm 06:20 hmmmm in either case you're relying on ub 06:20 nore ub? 06:20 hmmmm if you cast selected to unsigned you're still relying on ub but you'll be able to cut it down to one comparison instead of two 06:20 hmmmm undefined behavior 06:21 nore ah 06:21 hmmmm i think it's reasonable to assume we're always going to be running on a two's complement machine though 06:21 nore what does rely on ub in my code? 06:21 hmmmm the conversion between signed and unsigned and vice versa 06:21 Zeno` that's defined behaviour hmmmm 06:21 nore I mean, if there's no overflow, it's defined 06:22 nore am I wrong? 06:22 Zeno` yes, that's what I mean as well 06:22 hmmmm no you're right, if there's no overflow 06:22 hmmmm idk 06:22 hmmmm are you worried about handling overflow cases? 06:22 nore well, I think we can safely assume a dropdown has less than 2^31 choices :) 06:23 hmmmm perhaps 06:23 hmmmm sorry i always automatically think of edge cases when reviewing code 06:23 nore I do too, I just try to decide if they make sense too :) 06:24 nore (and with GUI code, there are a lot of ones that don't) 06:24 nore so, does it look good now? 06:27 Zeno` I know you're a mathematics expert and that 0 <= selected < (s32)dropdown_values->size() is how you would write it on paper, but... 06:27 Zeno` dropdown_values && 0 <= selected is perhaps confusing because of the line split :) 06:27 nore Zeno`: you prefer selected >= 0 then? 06:27 Zeno` I think so, in this case 06:28 Zeno` I mean it's the same of course, I'm just thinking of scrolling quickly and the line break making the expression seem confusing when it's not 06:28 Zeno` if that makes sense 06:29 Zeno` if it weren't for the line break I honestly wouldn't mind either way :) 06:29 nore Zeno`: changed, can I push it now? 06:29 Zeno` ok with me 06:29 nore hmmmm: is it ok with you too? 06:30 hmmmm uhh sure 06:30 nore ok, pushing 06:30 nore done 06:30 Zeno` actually I think it was better how it was .... 0 <= a < b 06:31 Zeno` just joking! :D 06:31 Zeno` I was actually very tempted to review the entire GUI 06:31 hmmmm oh shit 06:31 hmmmm whoops 06:32 hmmmm https://github.com/minetest/minetest/commit/cafbc28db7bce6d533f747bc335997447df49686#diff-65f34680878a6bd86f3a59ebc0c06c6dR2744 06:32 Zeno` but I got depressed when I saw how much it depends on irrlicht :( 06:32 hmmmm why didn't i notice this before nore pushed 06:32 Zeno` just fix it quickly now 06:32 Zeno` can rewrite history 06:32 nore yep 06:32 hmmmm 5 minute rule 06:32 hmmmm you can still eat it if it's been on the floor under 5 minutes 06:33 nore force-pushed 06:35 * Zeno` checks the atomic clock 06:35 Zeno` phew, well within time 06:36 nore updated comment as well 06:36 nore Zeno`: if I couldn't have done it in time, I would have removed the commits, changed it, and pushed it again ;) 06:36 Zeno` my atomic clock is actually slow so... 06:36 Zeno` heh 06:37 Zeno` I used the wrong element 06:37 * nore rebases his colored chat pull now 06:37 * nore bets there will be a conflict 11:03 Zeno` will merge #3979 soon 11:03 ShadowBot https://github.com/minetest/minetest/issues/3979 -- Android settings optimization by MoNTE48 11:04 Zeno` because the discussion will never have an agreement and it works better than current settings 11:05 Zeno` for those *guessing* how it performs without testing... please at least test :/ 11:26 Zeno` pushed 11:29 VanessaE Zeno`: break anything yet? :) 11:44 Zeno` VanessaE, heaps 11:45 * Zeno` break dances 12:31 est31 Why do we need separate bug and bugfix labels 12:32 Zeno` I dunno. But tbh adding the "bug" label to a bug fix PR has always bugged me 12:40 * nore agrees with Zeno` 12:43 nore est31: about static_text.cpp 12:43 nore it is an Irrlicht file 12:43 nore that we modified 12:43 nore so it's normal it violates style :/ 12:44 nore what should I do with it then? 12:44 est31 what precisely did you modify 12:45 est31 do you have a diff 12:46 nore the constructor, I added a class member, and changed a lot of things in BreakText to use EnrichedString instead 12:46 nore I do not have a diff, since I made the changes on the already-modified file that I got from the rebase 12:48 nore and I added what is from line 169 to the end of the file in static_text.h 12:53 est31 well shrug 12:54 nore IIRC we haven't changed code style in other library files we modified 12:54 est31 but I wonder why statictext and enrichedstring is really neccessary 12:55 est31 i havent looked at the code well enough though to suggest alternatives 12:55 est31 or to be really sure that i dont like it 12:55 nore est31: statictext adds another irrlicht GUI element 12:56 nore that behaves like text, but that is able to render colored text 12:56 nore (more precisely, text that can change color) 12:57 nore and EnrichedString is a class that reprensents such text 12:57 nore in order to separate the escape sequence code from the rendering one 17:48 paramat #3979 needed a small change before merge https://github.com/minetest/minetest/pull/3979#issuecomment-217655378 but we can do this in a new PR 17:48 ShadowBot https://github.com/minetest/minetest/issues/3979 -- Android settings optimization by MoNTE48 17:50 paramat #4092 can be merged 17:50 ShadowBot https://github.com/minetest/minetest/issues/4092 -- Keep scroll position constant in ChatBuffer::deleteOldest() by kahrl 18:03 paramat Zeno` > "is it just me or do the milestone issues just keep increasing in number?" it's just you multiplying 18:08 Wayward_One so, now after following the new apk build instructions, the build seems to be searching a bunch of directories, then often hangs at this: http://paste.ubuntu.com/16285724/ 18:08 Wayward_One and when it gets past that, it fails with http://paste.ubuntu.com/16285694/ 18:08 * Wayward_One misses ant 18:20 paramat maybe ask donbatman in project channel or monte48 in github? 18:56 paramat wow issues keep arising it's scary 18:58 est31 O_o 18:58 sofar people are actually testing 18:58 est31 over 100 milestone issues 18:58 est31 and > 90% closed 18:58 est31 good job 18:58 sofar that's what freeze is for ;) 19:10 paramat Wayward_One, please open an issue 19:12 Wayward_One paramat, already doing so ;) 19:13 paramat github did it again, 5 pages of open PRs 'update 13hrs ago' 19:14 Wayward_One #4099 19:14 ShadowBot https://github.com/minetest/minetest/issues/4099 -- Minetest apk build failure (again) 19:15 paramat ok 19:33 paramat i doubt we'll release on the 11th with all these issues, it would be better to release at the weekend anyway, more devs around 19:38 paramat nore PilzAdam sfan5 any reviews for game#1077 ? 19:39 ShadowBot https://github.com/minetest/minetest_game/issues/1077 -- fix error when setting creative from false to true by tenplus1 19:41 sfan5 paramat: pretty trivial, lgtm 19:41 paramat yes, just needs a correction 19:44 est31 paramat, with the release date you are almost like a republican senator member about appointing a new supreme court judge 19:44 est31 delay delay delay :) 19:44 paramat weird how it works by cacheing 19:44 est31 https://www.youtube.com/watch?v=-pTsmHoppLs 19:44 paramat heh i object to being compared to a republican 19:45 est31 I was joking :) 19:47 paramat i know :) 19:48 est31 you never know with this irc stuff, message may be misunderstood 19:54 paramat not sure about game 1077, caching seems wrong 19:54 paramat (even if it fixes the bug) 19:55 paramat maybe the problem is being able to switch to creative in-game? 19:57 nore paramat: I'd merge #1077 for the release as a temp workaround only 19:57 ShadowBot https://github.com/minetest/minetest/issues/1077 -- Dirt ridges / trenches in dirt, at chunk borders when lowering terrain in mgv6 19:57 nore and this will need to be properly fixed after it 19:58 paramat ok 19:59 paramat so you +1? 19:59 paramat once the '== true' is foxed 19:59 paramat (fixed) 20:01 paramat yes i guess that's obvious 20:07 nore paramat: +1 indeed if something is left on github as a remainder 20:07 nore (I guess, an issue with milestone 0.4.15) 20:11 paramat i can't agree to this. caching will probably break other things 20:12 paramat well hmm .. 20:15 paramat if it's merged then no crash, but caching provides false information if the creative setting is changed, causing other bugs. if it's not merged then crash if setting changed, but creative shouldn't be switched in-game anyway 20:18 paramat i'm flummoxed 20:23 paramat i oppose for now and won't merge it 20:38 kaeza I've been having a weird issue: if an error occurs in an `on_player_receive_fields` callback, the entire window goes blank (actually, gray), and does nothing else. it still responds as Windows does not show the "Not responding" dialog, but gets stuck on that state 20:38 kaeza haven't tried other callbacks 20:40 kaeza can be reproduced with this: https://gist.github.com/kaeza/372b74e509e0772b87b0a96d1338ac0c 23:38 paramat now merging game#1077 23:38 ShadowBot https://github.com/minetest/minetest_game/issues/1077 -- fix error when setting creative from false to true by tenplus1 23:50 paramat merged