Minetest logo

IRC log for #minetest-dev, 2016-10-26

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
00:06 AnotherBrick joined #minetest-dev
00:33 Taoki joined #minetest-dev
00:52 paramat est31 sofar how about we close #3623 ? the commits will still be preserved and findable if someone wants to take over
00:52 ShadowBot https://github.com/minetest/minetest/issues/3623 -- Postprocessing (WIP) by RealBadAngel
00:55 AcidNinjaFWHR joined #minetest-dev
01:16 Tmanyo joined #minetest-dev
01:24 Taoki joined #minetest-dev
01:25 froike joined #minetest-dev
01:26 ElectronLibre joined #minetest-dev
01:34 BrandonReese joined #minetest-dev
01:53 Grandolf joined #minetest-dev
01:53 halt_ joined #minetest-dev
01:54 Grandolf joined #minetest-dev
01:54 halt_ joined #minetest-dev
02:04 torgdor joined #minetest-dev
02:13 hiradur joined #minetest-dev
02:15 turtleman joined #minetest-dev
02:28 Grandolf joined #minetest-dev
02:50 Zeno` joined #minetest-dev
03:08 halt_ joined #minetest-dev
03:11 paramat Zeno` is it possible you could check #4530 for memory leaks? btw if it makes you feel better i doubt gcu will be working on this anymore
03:11 ShadowBot https://github.com/minetest/minetest/issues/4530 -- Fix superflous shader setting updates by gregorycu
03:15 paramat hm i can't find the comment about a possible memory leak, this seems fairly close to mergeable
03:15 Zeno` The comment was made my hmmmm 29 days ago
03:16 Zeno` new ShaderCallback(setter_factories),
03:16 paramat ah that
03:16 paramat yeah as sfan5 commented
03:21 Zeno` well, I'll run it
03:21 Zeno` I dunno why gcu doesn't answer questions though :/
03:22 paramat yeah frustrating. we're finding it difficult to find anyone who can work on it
03:23 Zeno` well, I personally don't want to touch it with a 50km long pole
03:23 Zeno` seeing what valgrind says about it
03:25 Zeno` might take awhile... I should have generated the world before running valgrind lol
03:26 Zeno` I'm gonna abort and try with an existing world, like I should have in the first place :D
03:26 paramat heh ok i'll continue to bug hmmmmm about it
03:27 Zeno` geez
03:27 Zeno` even without the world generated I get heaps of shader errors
03:27 Zeno` if (std::equal(m_sent, m_sent + count, value) && has_been_set) <--- depends on unitialised value(s)
03:27 Zeno` about 20 of them
03:29 Zeno` new ShaderCallback(setter_factories),   <--- definately lost memory (3 of those)
03:29 Zeno` oops 4
03:29 Zeno` actually when was the last time anyone checked for leaks etc?
03:29 Zeno` but most of them are shader anyway
03:29 paramat for this PR? possibly never
03:29 Zeno` let me see if they're there before the PR
03:30 paramat ah
03:32 Zeno` I'm more worried about the unitialized values then the memory "leak"
03:32 Zeno` (valgrind still running... I'll be back in a few)
03:34 Zeno` there are leaks before the PR
03:34 Zeno` but different ones lol
03:34 Zeno` but before the PR there is none of the  depends on unitialised value(s)
03:34 paramat =/
03:35 Zeno` so those unitialized things need to be fixed
03:35 Zeno` I'll generate a proper report
03:35 paramat thanks, this is useful
03:39 Zeno` the PR introduces 1 *new* memory leak that wasn't there before
03:39 Zeno` the others might actually be a bug in irrlicht, but the new one isn't (the one that hmmmm pointed out)
03:40 hmmmm well first of all
03:40 hmmmm i think this entire factory setup is confusing and i just don't understand the need for it
03:40 Zeno` hmmmm, that's why I can't work out where the unit var. is coming from without looking at the code... it's confusing
03:40 Zeno` normally it's "yeah, it was declared he and used there", but this is all over the place
03:41 hmmmm i need to spend a bit of time on trying to understand the interfaces irrlicht provides for us to work with, understand the problem this PR is solving, how it's been solved, and what a better way to do it is
03:41 hmmmm so
03:41 hmmmm i love the fact that this PR boosts performance for quite a few people a significant amount
03:41 hmmmm but this code is frankly crap
03:41 Zeno` the backtrace isn't much help either because it goes through the factory through a template through a... I dunno what it's doing lol
03:42 hmmmm i'm absolutely certain there's a cleaner way to do this without memory leaks, so that it's understandable
03:42 hmmmm i really, really hate to say this because there's a huge benefit but
03:42 hmmmm i code review -1 that PR for the mentioned reason
03:42 hmmmm the structure of it is just a total mess
03:42 hmmmm i would recommend re-solving the problem with an entirely new PR
03:42 est31 can the uninit thing be put on the github log of the PR as well?
03:43 est31 <Zeno`> if (std::equal(m_sent, m_sent + count, value) && has_been_set) <--- depends on unitialised value(s)
03:43 est31 <Zeno`> about 20 of them
03:43 Zeno` est31, yeah I'm getting a proper report now for them
03:43 hmmmm so
03:43 hmmmm what do we say
03:43 Zeno` it's shader.h line 98 btew
03:43 hmmmm re-write the effect of the PR?
03:43 hmmmm or try to fix it and merge that
03:43 hmmmm we'll be stuck with code we're not happy with
03:44 paramat higher quality is worth a wait
03:45 paramat perhaps the 0.4.15 milestone can be removed
03:50 Zeno` well I remove my approval from it at least heh
03:52 Zeno` hmmmm, well I've never looked much at the shader code but I just don't understand this new "style"
03:52 Zeno` it seems more complicated than it should be to me
03:52 Zeno` I dunno
03:52 hmmmm it's way more complicated than it should be
03:52 hmmmm you see that double pointer to Sky?
03:52 hmmmm I literally can't see where Sky is being modified
03:52 Zeno` the one that he doesn't know why it's a double pointer?
03:53 hmmmm but then again, i haven't really dug into the code a significant amount
03:53 hmmmm i feel like you shouldn't have to fully immerse yourself in the code in order to understand it
03:54 hmmmm if code requires you to put that much effort into understanding what it does and how it works, i think the developer completely failed at an important aspect of writing software
03:54 hmmmm not to mention that we've already confirmed it's hiding several memory leaks
03:54 hmmmm this makes it much more difficult to review and more error prone
03:55 Zeno` the backtrace for those unitialized vars leading to shader.h line 98 is obscene lol
03:55 hmmmm so yeah
03:55 hmmmm we'll dump it and start a new one
03:55 Zeno` I added it to the PR
03:56 Zeno` the entire atrocity that I am having trouble parsing properly in the source code
03:56 hmmmm our ultimate solution for reducing shader variable setting will use the solution this PR proposes, but implemented in its own way
03:56 Zeno` those are *all* from this PR btw. The memory leaks are a different matter...
03:59 Zeno` Are the benefits as huge as they're made out to be?
04:00 Zeno` I might profile it if my headache goes away
04:00 Zeno` but yeah I think it might be better started from scratch in any case
04:01 Zeno` I don't really know what's going on... I can't keep up with where I am in the new code
04:01 Zeno` 'cause of the template and factory "pattern"
04:01 Zeno` it seems to jump back and forth... *sigh*
04:01 hmmmm yeah it really doesn't help
04:02 hmmmm the jumping back and forth is the bane of my existence
04:02 hmmmm it reminds me of java code
04:07 Zeno` the sky pointer (where it's pointing to) isn't changed in C++
04:08 Zeno` even if it was something Lua related there'd have to be something in C++ to actually make the change and it doesn't exist
04:09 Zeno` weird
04:11 Zeno` hang on
04:12 Zeno` gcu changed it to a pointer to a pointer
04:12 Zeno` and he doesn't know why?
04:12 Zeno` that seems a bit strange
04:13 paramat that may have been ShadowNinja in #3770 "with some changes like replacing SkyBackgroundColorProvider with a pointer to a pointer"
04:13 ShadowBot https://github.com/minetest/minetest/issues/3770 -- Fix superflous shader setting updates by ShadowNinja
04:13 hmmmm look if that's not a sign that says "abandon all hope ye who enter here" then i don't know what is
04:13 Zeno` it's used in exactly the same 3 places but as a ** not a *
04:14 Zeno` paramat, is 3770 in master?
04:14 Zeno` oh I see
04:14 Zeno` it's a modified version of 3770
04:14 paramat yes
04:15 Zeno` I wonder why he did that
04:15 Zeno` close it!
04:17 Zeno` I have abandoned all hope and am exiting there hehe
04:18 sofar paramat: I don't mind closing old PRs like that, but maybe label them "not merged" so that they're easy to find later on?
04:18 sofar or some other label that is easily usable
04:19 Zeno` i meant close 4530 as hmmmm suggested
04:19 Zeno` but yeah I agree
04:20 paramat for #3623
04:20 ShadowBot https://github.com/minetest/minetest/issues/3623 -- Postprocessing (WIP) by RealBadAngel
04:21 Zeno` i see why it's done btw
04:22 Zeno` it's because the sky background colour can be changed before sky exists... but I just fixed it (without using a **) by adding an if (m_sky) { } around the offending code
04:23 Zeno` which is a whole lot clearer IMO than having a ** to deal with something that happens once during program init
04:23 paramat well i'm in no rush to close 3623
04:24 hmmmm Zeno`:  ha good on you, but is knowing that going to fix the template shit and the factory shit?
04:26 hmmmm you know what, lately i've been -1ing more PRs than +1ing
04:26 Zeno` hmmmm, no. But I now know that the sky** is a kludge :)
04:26 hmmmm maybe i'm too negative of a person, but maybe a lot of these PRs aren't really that great
04:27 Zeno` it's called once
04:28 Zeno` why would you solve the problem by using a ** heh
04:28 Zeno` oh well
04:28 * Zeno` puts it on his list of weird things he's seen today
04:28 * Zeno` gets out of the branch now and deletes it from his local repo
04:33 Zeno` delayed server shutdown
04:34 Zeno` ok, -1 on that idea :)
04:38 Zeno` closed #3354
04:38 ShadowBot https://github.com/minetest/minetest/issues/3354 -- Add ring buffer by dandelion-james
04:38 Zeno` it doesn't do anything
04:39 sofar was there even unit tests in that?
04:39 Zeno` no
04:39 Zeno` my comment on closing is "If it's intended "to be used with ProfilerGraph, to replace deque m_log" then that should be done and this change part of that."
04:39 sofar well that's surely a sign that it doesn't do anything :)
04:39 Zeno` as that was the author's justification for the PR
04:43 Zeno` hehe
04:44 Zeno` well, yeah. Not much point in adding dead code before it even lives
04:44 garywhite joined #minetest-dev
05:08 Hunterz joined #minetest-dev
05:18 nerzhul_ joined #minetest-dev
05:46 paramat joined #minetest-dev
05:58 paramat discovered i left an unnecessary alpha channel in acacia tree texture, fixing
06:08 paramat game#1344
06:08 ShadowBot https://github.com/minetest/minetest_game/issues/1344 -- Default: Remove alpha channel from acacia tree textures by paramat
06:10 paramat will merge in a moment
06:14 paramat merging
06:17 paramat complete
06:18 jin_xi joined #minetest-dev
06:47 red-001 joined #minetest-dev
06:59 paramat left #minetest-dev
07:18 Foghrye4_ joined #minetest-dev
07:41 werwerwer joined #minetest-dev
08:49 Zeno` closed #4530
08:49 ShadowBot https://github.com/minetest/minetest/issues/4530 -- Fix superflous shader setting updates by gregorycu
09:12 troller joined #minetest-dev
09:49 Zeno` I'm sick of gcu's accusations against me
09:49 Zeno` apparently I am trying to sabotage his work (according to his crazed mind)
09:50 est31 joined #minetest-dev
09:51 Zeno` I didn't "close" his stupid and shitty PR (apart from closing it)
09:51 Zeno` it was closed at the point that the "Won't add" label was added
09:51 Zeno` geez
09:53 Zeno` and I had nothing to do with that apart from participating in a discussion
09:53 Zeno` ugh
09:58 blaze joined #minetest-dev
10:02 Fritigern joined #minetest-dev
10:21 Zeno` someone should add "the result of a comparison is discarded in the case one of the operands is uninitialised" to the topic
10:41 jin_xi as far as i can tell the result of this comparison is discarded based on wether a bool is set
10:50 Zeno` no, it's checked
10:50 Zeno` i.e. accessed
10:50 Zeno` and the value of that is undefined
10:53 jin_xi but that value is never used? so this code allows for some undefined behaviour but does not rely on it being anything specific at all
10:53 jin_xi well, idk. thats what i get from a quick glance
10:54 Zeno` it's used in a conditional comparison (if)
11:05 jin_xi i'd say its used in a comparison (&&) the result of which is always well defined due to has_been_set
11:07 Zeno` no, because it's && both have to be initialised
11:07 Zeno` even if it's || both *should* be initialised to be safe
11:08 Zeno` if one of the operands involved in an && is not initisialised then the entire expression is undefined
11:08 jin_xi but the comparison of random memory just gives non reliable bool, not uninitialized
11:10 Zeno` no, the result of the expression is undefined because the evaluation of the expression relied on unitialised (random) "memory"
11:10 Zeno` so the expression could be true, or it could be false. Neither is correct because the result of the && is undefined
11:12 Zeno` int a = 0, b; int c = a && b;
11:12 Zeno` the value of c is undefined because 'b' is not initialised
11:12 Zeno` it might be 0, or it might be 1. Neither is more correct
11:13 Zeno` and that's what's happening (in essence, but in an if statement) in the PR
11:14 Zeno` who knows what the value of 'b' is?
11:14 Zeno` I don't
11:14 AcidNinjaFWHR joined #minetest-dev
11:15 Zeno` common sense would say that b == 42, but *shrug*... that's not defined
12:08 Fixer joined #minetest-dev
12:21 proller__ joined #minetest-dev
12:43 Fixer https://github.com/minetest/minetest/issues/4335#issuecomment-256335199
12:55 red-001 joined #minetest-dev
13:35 garywhite joined #minetest-dev
13:45 STHGOM joined #minetest-dev
14:15 Calinou https://github.com/minetest/minetest/issues/4335#issuecomment-256360437
14:15 Calinou ran a benchmark, please review
14:15 Calinou even on my hardware, I lose 20 FPS with shaders enabled and no special features, 30 FPS more when enabling all features
14:15 Calinou at this point Terasology runs faster… :P
14:16 Calinou so there's a lot of optimization work to do, even more so with shaders
14:16 Calinou Minecraft performs quite better, last time I played it
14:20 sfan5 it depends on the machine how MT & MC perform
14:21 Calinou sfan5: Minecraft almost universally performs better in my experience, too :/
14:21 Calinou even on mediocre hardware
14:21 Calinou they really did optimize it a lot
14:34 Fixer same for me
14:34 Fixer MC runs a lot faster and smoother
14:51 est31 joined #minetest-dev
15:17 Fixer joined #minetest-dev
15:17 hmmmm joined #minetest-dev
15:22 est31 kahrl really knows his C++
15:22 est31 kudos
15:39 Fixer_ joined #minetest-dev
16:09 blaze joined #minetest-dev
16:10 Hunterz joined #minetest-dev
16:34 jin_xi joined #minetest-dev
16:44 Foghrye4__ joined #minetest-dev
17:01 AcidNinjaFWHR joined #minetest-dev
17:02 Krock joined #minetest-dev
17:02 Krock joined #minetest-dev
17:38 proller__ joined #minetest-dev
17:58 nrzkt joined #minetest-dev
18:08 torgdor joined #minetest-dev
18:27 est31 joined #minetest-dev
19:32 torgdor joined #minetest-dev
19:46 Anchakor_ joined #minetest-dev
19:58 nrzkt sfan5, a user reports this in serverlist +> https://lut.im/mWND9f1wXm/7i0EPRLKiTR9U1tJ.png can you do something please ?
20:00 sfan5 well they can name their servers however they want
20:01 sfan5 also individual server banning is not implemented yet
20:01 sfan5 i could only ban the whole of mthosting.com right now
20:01 garywhite You could try emailing minetesthosting
20:02 sfan5 i could
20:02 est31 and if they don't ban it you ban the whole site :)
20:04 proller joined #minetest-dev
20:10 indriApollo joined #minetest-dev
20:13 Tmanyo joined #minetest-dev
20:13 red-001 Its not as if they would lose a paying customer
20:14 proller joined #minetest-dev
20:15 red-001 I don't think they would mind taking it down
20:16 sfan5 well send them a mail
20:16 sfan5 it's not my responsibility to do so
20:27 Jellonator joined #minetest-dev
20:34 garywhite Emailed, he said:
20:34 garywhite https://www.irccloud.com/pastebin/9sAWvDMo/
20:36 Fixer_ i don't see it in serverlist currently
20:36 garywhite The guy in charge of minetesthosting shut it down
20:40 Person joined #minetest-dev
20:43 DI3HARD139 joined #minetest-dev
20:57 torgdor joined #minetest-dev
20:58 Hijiri nrzkt: in https://github.com/minetest/minetest/pull/4669#issuecomment-256161297 you meant that making it reliable would cause lots of lag, right?
20:58 Hijiri the "sorry but i'm wrong" at the beginning confuses me
20:59 Hijiri I was thinking to instead append the stuff that normally gets sent in a TOSERVER_PLAYERPOS to the end of interact packets, and then use that to set player control stuff
20:59 Hijiri does that sound good?
21:11 proller joined #minetest-dev
21:43 AcidNinjaFWHR joined #minetest-dev
21:49 twoelk joined #minetest-dev
21:52 kaeza joined #minetest-dev
22:16 Zeno` joined #minetest-dev
22:45 proller joined #minetest-dev
22:48 Hunterz joined #minetest-dev
22:49 ElectronLibre joined #minetest-dev
22:53 troller joined #minetest-dev
22:56 ssieb joined #minetest-dev
23:16 Zeno` joined #minetest-dev
23:24 Tmanyo joined #minetest-dev
23:24 twoelk|2 joined #minetest-dev
23:36 Zeno` est31, am I using "Won't Add" tag incorrectly?
23:55 est31 Zeno`: ??
23:56 Zeno` it's just that you removed the two I added yesterday
23:56 Zeno` so I thought/think I must be using it wrong
23:57 Zeno` I probably misunderstand what the label is supposed to be used for
23:57 paramat joined #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext