Time  Nick        Message
07:21 hmmmm       ugh
07:22 hmmmm       does anybody have objections to placing object files into src/CMakeFiles/<target>.dir/<cmake_build_type> subdirectory?
07:22 hmmmm       this way you don't have to recompile the whole thing when you change the build type
07:24 TriBlade9   I approve?
07:49 nrzkt       Hello all !
07:52 hmmmm       smooth day to night transition seems like it's broken again
07:55 nrzkt       #2209 #2212 and #2214 need to be merged !
07:55 ShadowBot   https://github.com/minetest/minetest/issues/2209 -- Little performance improvement: use getPlayer(peer_id) instead of getPlayer(playername) by nerzhul
07:55 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
07:55 ShadowBot   https://github.com/minetest/minetest/issues/2214 -- Performance improvement -> Craftdef.cpp: Improve loop and mathematics for CraftDefinitionShaped::check by nerzhul
09:42 rubenwardy  Is this a mistake? https://github.com/minetest/minetest_game/blob/master/mods/default/nodes.lua#L895
09:47 nrzkt       i don't see anything special
09:48 rubenwardy  What does "buildable_to" mean?
09:48 rubenwardy  Oh, nevermind
12:01 kilbith     kahrl_, what are you waiting to merge 2185 ?
12:10 kahrl_      kilbith: end of the feature freeze
12:12 Zeno`       smooth day/night is broken?
12:22 Zeno`       kahrl_, what are your opinions on #2212
12:22 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
12:23 Zeno`       it (the problem) obviously needs to be address, and the PR seems to do that
12:24 Zeno`       It was merged before (in a different format) that caused old maps to fail loading. This PR addressed that issue
12:24 Zeno`       addresses*
12:33 kilbith     kahrl_: your PR is not a feature, but a fix
12:48 Zeno`       #2209 is fine
12:48 ShadowBot   https://github.com/minetest/minetest/issues/2209 -- Little performance improvement: use getPlayer(peer_id) instead of getPlayer(playername) by nerzhul
12:50 kilbith     50000% faster ? :p
12:53 kahrl_      kilbith: yeah but the issue has been there for like forever, so there is no need to hurry
12:53 kahrl_      so in that sense it's really more like a feature
12:54 kahrl       especially since graphics fixes have a tendency to always cause new bugs on different drivers :/
12:54 Zeno`       2212 seems ok though
12:55 kahrl       Zeno`: without having thought all the consequences through (compatibility is hard), I tend to agree
12:56 Hunterz     vote for issue #1072
12:56 ShadowBot   https://github.com/minetest/minetest/issues/1072 -- Clientside prediction needs to be improved
12:58 Zeno`       does anyone have an old world they can test with #2212 merged?
12:58 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
12:58 kahrl       Zeno`: about # 2212, I don't understand why the check is mapblock.cpp is removed
12:59 kahrl       s/is/in
12:59 nrzkt       kahrl
12:59 nrzkt       if you look at handling of _INIT packet
13:00 nrzkt       it sets client->setPendingSerializationVersion(deployed);
13:00 nrzkt       and it's the version passed into MapBlock
13:00 kahrl       yeah
13:00 nrzkt       then if we check at init
13:00 kahrl       so clients can't trigger a SerializationError anymore
13:00 nrzkt       the version isn't modified elsewhere
13:00 nrzkt       :)
13:01 nrzkt       then PR is good
13:01 kahrl       but some coder in the future might accidentally cause something to pass in a version < 24
13:01 nrzkt       this commits mustn't be accepted
13:01 kahrl       this check signals that this is a severe error
13:01 nrzkt       we need to manage the version properly
13:01 kahrl       nrzkt: your first PR about this was accepted too, so...
13:01 nrzkt       i'm working on networking rework and it' unacceptable to have a protocol version < now
13:02 nrzkt       yes. But it modify a production value
13:02 Zeno`       I accepted that first PR without looking at all the side-effects
13:02 nrzkt       here we have an alternate value
13:02 Zeno`       I do know that I had issues about a month ago with supporting version 0 of the protocol (when it's pretty pointless)
13:02 nrzkt       i don't know that version may be different than serialization, it's not clean in code, but i must be clear for 1.0.0 release
13:03 nrzkt       here we accept client serial version != map serial version
13:03 nrzkt       but i think for 1.0.0 we must break those things and use a migration mode
13:03 nrzkt       maintain 2 years old version is a pain
13:04 kahrl       there isn't a "map serial version"
13:04 kahrl       every mapblock is its own version
13:04 nrzkt       okay, i understand better now
13:04 nrzkt       i think we must have a conversion mode for next release, and we mustn't keep there old maps which block the minetest evolution
13:04 kahrl       what's a pain about it? most of the cruft is in deSerialize_pre22 and doesn't need to be touched
13:05 Zeno`       As I said yesterday, I think the main thing for 0.4.12 is to close the potential DoS
13:05 nrzkt       no for this feature freeze.
13:05 nrzkt       this fix the DoS, or i'll use it on many servers :p
13:05 Zeno`       a client connecting with incorrect version should NOT "crash" the server
13:06 kahrl       I think coding a migration layer is more work than simply updating the serialization code
13:06 kahrl       and it causes more work for every user too
13:06 Zeno`       so what is the easiest way to avoid the crash *right now*? Even if it's not the best way?
13:06 nrzkt       it's the only way Zeno
13:07 nrzkt       if we don't handle the input the DoS is easy
13:07 kahrl       Zeno`: I vote for # 2212 without the check in mapblock.cpp removed
13:07 kahrl       assuming that works in all the cases
13:07 nrzkt       it's impossible that test match
13:07 kahrl       nrzkt: yeah. It's defensive code
13:07 nrzkt       please audit the code instead of telling this
13:08 nrzkt       a stupid test which stay there and add cpu cycles and 1 year later stay there and nobody knows why.
13:08 Zeno`       nrzkt, add a comment: TODO: This condition can never be true; review this code
13:08 kahrl       and who will do the audit?
13:08 nrzkt       ME
13:08 Zeno`       just so that it can be merged during the feature freeze
13:08 nrzkt       now, do it
13:08 kahrl       ok, it can be removed after the audit
13:08 Zeno`       after the feature freeze larger changes can be made
13:09 nrzkt       it's not a larger thing
13:09 kahrl       assuming you promise to audit every future change to serialization versions too ;)
13:09 nrzkt       i tell you that there is code to remove, after the release everybody forget this
13:09 Zeno`       it's erring on the side of caution
13:09 Zeno`       your first PR after feature freeze can be to delete it
13:09 Zeno`       and I'll merge it myself :P
13:09 nrzkt       i know what i am doing, i'm very conservative with code
13:10 Zeno`       yep, not debating that
13:10 kahrl       meh, what's a few cycles versus writing to disk or sending stuff over the network
13:10 nrzkt       in fact, CPU cycle vs ... nothing is clear
13:10 nrzkt       look at the code, look at version input
13:10 Zeno`       This is only for a few days
13:10 nrzkt       now i'm doing audit for you in live.
13:10 nrzkt       client->setPendingSerializationVersion(deployed);*
13:10 Zeno`       Surely you will not forget before next week :)
13:11 nrzkt       clientiface.h: 	/* set expected serialization version */
13:11 nrzkt       void setPendingSerializationVersion(u8 version)
13:11 nrzkt       { m_pending_serialization_version = version; }
13:11 nrzkt       later in the code:
13:11 nrzkt       void confirmSerializationVersion()
13:11 nrzkt       { serialization_version = m_pending_serialization_version; }
13:12 nrzkt       we now have RemoteClient->serialization_version (u8)
13:13 gregorycu   "which is dumb, because not even spread_light uses 8% of the CPU time; but a single map lookup did?"
13:13 gregorycu   there was another issue where adding a callback to "cache" the enable_fog led to reducing the main (cleint) game loop by 8%
13:13 gregorycu   If the profiler reports that 8% of the time is being spent there, 8% of the time is getting spent there
13:14 gregorycu   It doesn't have to make sense, it's just cold hard facts
13:14 gregorycu   @ Zeno`
13:14 nrzkt       ok then
13:15 nrzkt       void Server::SendBlockNoLock(u16 peer_id, MapBlock *block, u8 ver, u16 net_proto_version)
13:15 nrzkt       which is called by void Server::SendBlocks(float dtime)
13:15 nrzkt       uses as argument 2: client->serialization_version
13:15 nrzkt       line 3924 (server.cpp)
13:16 nrzkt       then if client->serialization_version is used, and it's verified and set to a value >= 24 in _INIT it's impossible to match this try
13:16 nrzkt       then this is DEAD CODE
13:16 nrzkt       CQFD.
13:16 nrzkt       if forget to say: SendBlockNoLock call lock->serialize(os, ver, false); which is the function we are looking for
13:17 nrzkt       now stop discuss about cutting the thing, you have the demonstration.
13:17 kahrl       ok, please don't do any audits
13:18 kahrl       because you missed other places that call MapBlock::serialize
13:18 nrzkt       tell me
13:18 kahrl       no I won't, I don't have time to audit
13:18 nrzkt       my grep doesn't show nothing.
13:18 kahrl       there's an obvious one in map.cpp
13:19 nrzkt       right
13:19 nrzkt       u8 version = SER_FMT_VER_HIGHEST_WRITE;
13:19 nrzkt       block->serialize(o, version, true);
13:19 nrzkt       it's good.
13:19 Zeno`       gregorycu, what is the time complexity of a map lookup?
13:20 gregorycu   Are you talking theory, when we have actual statistics?
13:20 Zeno`       I am talking theory
13:20 gregorycu   O(logn)
13:21 Zeno`       ok, what it the time complexity of, say, unspreadLight()?
13:21 gregorycu   The time complexity?
13:21 kahrl       so assuming we remove the check
13:21 Zeno`       yeah
13:21 Zeno`       in big-O notation
13:21 kahrl       where is the documentation for serialize() that documents the precondition version >= 24?
13:22 gregorycu   O(n^3)
13:22 Zeno`       what I am suggesting is that your empirical data is either wrong or you're misinterpreting it
13:22 gregorycu   ...
13:22 gregorycu   Neither are those are true
13:22 gregorycu   Firstly, the latter happens in a different thread
13:22 gregorycu   And only when emerging, or light changes
13:23 nrzkt       thanks kahrl
13:23 kahrl       nrzkt: btw, are you saying we should remove all assert() calls from minetest?
13:23 kahrl       because they are surely dead code
13:23 Zeno`       you're saying that a single map lookup consumes 8% of the CPU in the loop
13:23 Zeno`       well, they kind of are
13:23 Zeno`       kahrl, that's my next mission heh
13:23 nrzkt       no kharl, asserts are good to prevent some coding error
13:24 gregorycu   A map lookup, and a conversion from a string to a bool
13:24 gregorycu   Yes
13:24 nrzkt       if you look at my networking PR i'm using it to prevent packet reading error in the new stream interface :)
13:24 gregorycu   About 4% each
13:24 kahrl       nrzkt: right
13:24 Zeno`       4% each of TOTAL program time?
13:24 gregorycu   The render loop, which standing still is about 90% of the application
13:25 kahrl       nrzkt: so can we agree to document version >= 24 as a precondition, remove the if and instead add an assert that verifies the precondition?
13:25 Zeno`       if that's the case please add // precondition to then end of the assert
13:25 T4im        asserts are supposed to be dead code… they shall error the moment they are accidentally not anymore :P
13:26 gregorycu   It was 8% of the 90% (or about 7.2% overall)
13:26 nrzkt       if you want, but i think it's useless, the programming error is handled
13:26 Zeno`       so I don't have to recheck it when I removed the redefinition of assert
13:26 nrzkt       maybe a comment ?
13:26 Zeno`       s/removed/remove
13:26 Zeno`       A comment is fine
13:26 kahrl       nrzkt: compatibility code is very hard as we've seen a lot in the past, I think an actual assert is needed
13:27 nrzkt       in fact kahrl assert was already here
13:27 nrzkt       because this error trigger and assert on servers
13:27 nrzkt       trigger an*
13:27 nrzkt       and it's why i handle it
13:27 kahrl       yeah but that's fixed now, isn't it
13:28 kahrl       I mean with the rest of 2212
13:28 nrzkt       of course, because we are handling the packet input, then the exception/assert can be removed because condition is verified and handled
13:28 gregorycu   Zeno`: I'm glad I could clear this up for you, now I'm going to go back to ignoring you
13:28 Zeno`       gregorycu, I'm not sure you understand
13:29 Zeno`       gregorycu, and what, please, have I done to so greatly offend you?
13:29 nrzkt       in fact the new SER_FMT_CLIENT_VER_LOWEST guarantee a compatibility to old mapblocks which are using SER_FMT_CLIENT_VER_LOWEST
13:29 nrzkt       SER_FMT_VER_LOWEST* (2nd occurence)
13:30 Zeno`       gregorycu, If you will not discuss pull requests with me or other devs (unless you choose to) then your pull requests are useless
13:31 gregorycu   I have updated #2156 as per comments
13:31 ShadowBot   https://github.com/minetest/minetest/issues/2156 -- Optimise MapBlockMesh functions by gregorycu
13:32 Zeno`       gregorycu, is there a reason it's not static?
13:33 Zeno`       look, if you can't answer the question I'll just close it
13:34 nrzkt       then kahrl, #2212 is right ?
13:34 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
13:35 kahrl       nrzkt: I don't see the assert you mean
13:36 Zeno`       closed
13:38 kahrl       nrzkt: neither do I see any comment about the precondition
13:38 nrzkt       in fact kahl, i don't found it too, it's just assert(0) when it's triggered
13:38 nrzkt       no code line :(
13:38 nrzkt       but i can trigger it on you server, give me an IP and i connect with my modified client and the assert will be triggered
13:39 nrzkt       and the exception show
13:39 kahrl       127.0.0.1
13:39 nrzkt       xD
13:39 gregorycu   For some reason #2156 was closed
13:39 ShadowBot   https://github.com/minetest/minetest/issues/2156 -- Optimise MapBlockMesh functions by gregorycu
13:40 kahrl       gregorycu: you would know the reason if you hadn't ignored Zeno` :P
13:40 kilbith     ^
13:40 gregorycu   Apparently: "Closing. Author will not discuss the PR on IRC and therefore questions about it cannot be asked."
13:40 gregorycu   I wasn't aware IRC was required to get work merged
13:40 kilbith     don't expect your PRs merged if you ignores a concerned core-dev
13:40 kilbith     simply as that
13:41 gregorycu   There are no concerns mentioned in the PR
13:41 gregorycu   (Apart from the ones I've addressed)
13:42 Zeno`       gregorycu, why is VoxelManipulator::ContentIgnoreNode not static?
13:42 Zeno`       I see no reason for it to have external linkage
13:43 Zeno`       ok, that settles it then I guess :/
13:43 nrzkt       gregorycu ?
13:43 nrzkt       are you ignoring Zeno on IRC ?
13:43 gregorycu   Yes?
13:43 gregorycu   Yes
13:44 nrzkt       then you can't discuss with minetest PR merger
13:44 nrzkt       it's why your PR is blocked
13:44 gregorycu   I'll respond to any concerns he has in the PR
13:44 gregorycu   I just find him a frustrating person to speak to, so it's best this way
13:44 nrzkt       he ask you: gregorycu, why is VoxelManipulator::ContentIgnoreNode not static?
13:44 gregorycu   It is...
13:45 gregorycu   You don't have to be a go-between, I'm sure if has concerns, he'll address them in the PR
13:48 nrzkt       gregory: const MapNode VoxelManipulator::ContentIgnoreNode = MapNode(CONTENT_IGNORE); in voxel.cpp
13:49 gregorycu   Yeah...
13:49 Zeno`       look, I won't open it again
13:49 gregorycu   It's declared static, therefore it is static
13:49 gregorycu   I'm pretty sure it's a compile error to put static there too
13:49 Zeno`       If gregorycu wants stuff merged he has to discuss things
13:49 Zeno`       and it's not declared static at all
13:49 gregorycu   (And not required, I'll check)
13:50 Zeno`       so it has external linkage for no apparent reason at all
13:50 Zeno`       err
13:50 Zeno`       wait, the code has changed since the other day
13:50 Zeno`       oh well
13:50 nrzkt       :)
13:50 gregorycu   error C2720: 'VoxelManipulator::ContentIgnoreNode' : 'static ' storage-class specifier illegal on members
13:50 nrzkt       he said he changes the code
13:51 Zeno`       pity it cannot be discussed :)
13:51 nrzkt       but i think your browser cache is wrong Zeno
13:51 nrzkt       :)
13:51 nrzkt       greg, Zeno i opened to discussion if you unignore him :
13:52 Zeno`       all he had to say was "hey, look! I changed it since the other day"
13:53 Zeno`       but, no, he had to ignore me
13:53 gregorycu   It's quite alright nrzkt, there are many people, I'm not sure he should be closing PRs for no technical reason
13:53 gregorycu   And indeed, closing them without reading comments
13:53 Zeno`       there is no technical reason now. It's a political reason
13:54 Zeno`       And it was, really, a political reason why I made it WIP as well
13:54 Zeno`       If devs cannot ask questions then *shrug*
13:54 nrzkt       in fact minetest devel can go faster if political goes to trash
13:54 nrzkt       :p
13:54 Zeno`       not by ignoring people with though :/
13:54 nrzkt       minetest is not a profit enterprise
13:55 nrzkt       ofc Zeno
13:55 Zeno`       I don't even know why he ignored me in the first place
13:55 Zeno`       in the last few weeks I've rebased his commits, fixed his broken repo, tutored him in git usage, etc, etc
13:56 Zeno`       makes no sense
13:56 Zeno`       I've done nothing but support him
13:57 Zeno`       so, at this point my response to him is "ok, if that's the way you want it, then fuck you"
13:59 Zeno`       All I can think of is that I denied one of his PRs because it caused a deadlock on Linux
13:59 shadowzone  Whose?
14:00 Zeno`       Since that comment he's been nothing but aggressive towards me :/
14:00 kahrl       wasn't that me who was concerned about that?
14:01 kahrl       why isn't he ignoring me instead then?
14:01 kahrl       shadowzone: gregorycu's
14:01 shadowzone  Oh
14:02 shadowzone  I've never even had a PR merged or even considered, so he should just settle down.
14:02 gregorycu   You guys are talking about me, I can sense it somehow, kahrl, I don't have a problem with you
14:02 gregorycu   And I'm settled, but removing frustrating elements out of my life, I've streamlined my process
14:03 shadowzone  Well that was uneventful, took apart my tablet plugged the battery and plugged it back in, nothing.
14:03 kahrl       gregorycu: yes we are, you can read it in the logs (see topic), but you'd have to read Zeno's comments (the horror!)
14:03 gregorycu   I'd rather not, would defeat the purpose you see
14:03 kahrl       gregorycu: see, and Zeno` is removing frustrating elements out of his life by closing your PRs
14:04 gregorycu   I wasn't aware the minetest repo was his plaything to satisfy his life enjoyment
14:05 kahrl       neither is it yours
14:06 gregorycu   No, it's not. But I'm the one contributing to it, not the one closing legitimate PRs
14:06 shadowzone  To be honest, I never expect my PRs to get merged and if they do Hooray, but I really don't expect them to.
14:06 kahrl       what is legitimate about it if its author doesn't want to discuss it?
14:07 shadowzone  I don't say "Merge this merge that, why'd you close that?"  My dad who develops apps n stuff told me "if they don't like it work harder and make something they will like"
14:07 gregorycu   I am quite willing, I'm responding to comments in PRs...
14:08 kilbith     shadowzone: #minetest-dev is not a place to talk about your life, your feelings and other anedoctic things...
14:08 shadowzone  Okay
14:11 kahrl       gregorycu: well, if you arbitrarily want to make the conversation about your PR inefficient by limiting it to github, then so be it
14:12 gregorycu   A conversation can't happen if the PRs get closed, they are off peoples' radars before there is a chance to look at them.
14:13 gregorycu   It's not arbitrary, I'm more than willing to discuss these things with people
14:13 kahrl       except the people that actually care?
14:14 kahrl       you can always ask Zeno to open it again, he's here, you know
14:15 gregorycu   Zeno` made a mistake in closing it, or at least I assume so, as there is no legitimate reason specified in the PR
14:15 gregorycu   I'm sure he will come to his senses without my prompting
14:17 kahrl       let's stop spamming the channel with this stupid BS now
14:17 gregorycu   Ok
14:19 kahrl       let's rather fix the segfault in sqlite3_finalize when the singleplayer server rejects the singleplayer client (because of on_prejoinplayer for example)
14:20 nrzkt       #2212 is poping again for netherstorm
14:20 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
14:20 nrzkt       where are we kahrl ?
14:22 kahrl       nrzkt: I'm having trouble testing it because of what I just mentioned
14:25 Zeno`       gregorycu, although I haven't reviewed it properly, yet another time, the PR probably has no outstanding *technical* issues
14:25 Zeno`       night all
14:30 kahrl       #2219
14:30 ShadowBot   https://github.com/minetest/minetest/issues/2219 -- Singleplayer segfaults if server rejects client
14:30 nrzkt       kahrl you want an assert ?
14:30 kahrl       nrzkt: yes
14:30 nrzkt       ok, but after release i explode it :p
14:30 nrzkt       right ? :)
14:30 kahrl       why?
14:32 nrzkt       because it's useless, but if you want to be sure for this release, we made it
14:33 kahrl       it's not useless
14:33 nrzkt       on next release if nobody has assert it's because it's useless :)
14:34 kahrl       nrzkt, you assume everyone keeps everything about the minetest source code in mind at all times
14:34 kahrl       nrzkt: and that nobody ever makes mistakes
14:35 nrzkt       nobody musn't do mistake, but reviews are there to don't have those mistakes
14:35 kahrl       Zeno reviewed your first PR and still pushed it
14:36 gregorycu   Mistakes happen, and two people can make the same mistake, even two smart people
14:36 nrzkt       you are right, but we forget the input into map.cpp, as you said
14:36 gregorycu   Code reviews are not perfect
14:36 nrzkt       ofc
14:38 nrzkt       #2209 can be reviewed ? it's more and more simpler
14:38 ShadowBot   https://github.com/minetest/minetest/issues/2209 -- Little performance improvement: use getPlayer(peer_id) instead of getPlayer(playername) by nerzhul
14:38 nrzkt       and doesn't trigger asserts or exceptions :)
14:39 kahrl       nrzkt: after the release
14:39 nrzkt       can you use milestones on github ? it will be better to manage project with milestones no ?
14:40 kahrl       sure, we have milestones
14:40 nrzkt       then PR will not be abandonned because milestone MUST be reached for release
14:40 nrzkt       https://github.com/minetest/minetest/milestones/0.4.12
14:40 kahrl       it's not a guarantee
14:41 nrzkt       milestones must be uses properly to manage the release engineering, it help the developer to focus their effort
14:41 kahrl       the decision to start a feature freeze is done without regards to whether there are open PRs in the milestone
14:42 kahrl       and if the devs who know about the topic of the open PRs are not around before and during the feature freeze, they won't be merged
14:42 nrzkt       why not be agile, creating a milestone with a maximum date (for example every 6 months), do a feature freeze 1 month before release
14:43 nrzkt       and do efforts on cleaning up bugfix PR and resolve issues in this month and then release ?
14:43 nrzkt       milestone is here for that. In minetest it's... nothing
14:43 kahrl       I don't know what "agile" means anymore
14:43 nrzkt       sorry it's a french term
14:43 nrzkt       do you know ITIL ?
14:43 kahrl       it's thrown around a lot in software engineering and everyone means something different by it
14:44 kahrl       do you mean we should do scrums every morning?
14:44 kahrl       because that won't happen :P
14:44 gregorycu   Standups are a great waste of manhours
14:44 nrzkt       no, i mean the project must be more organized in term of release engineering
14:44 gregorycu   When the person leading them doesn't know how to do them
14:44 nrzkt       a release date must be set months before, not days before
14:45 kahrl       I don't know ITIL
14:45 nrzkt       ok
14:45 kahrl       well there was some discussion about release engineering
14:45 kahrl       it evolved into bikeshedding about version numbers :P
14:45 nrzkt       agile = development based on little modifications reviewed by many people and frequent releases (6 month max)
14:46 nrzkt       version numbering is difficult to handle
14:46 nrzkt       because there are many ways.
14:47 nrzkt       I think there must be real branches in minetest. 1.0 (which is after 0.4.12 if i correctly follows) must add a new devel cycle with a branching model for compat
14:47 nrzkt       old things must stay in 0.4.x branch, where minetest dev provide and backport fixes
14:47 kahrl       the problem with setting release dates a long time in advance is that you don't know if the people who can do the release will be available at the time
14:48 kahrl       it happens to some extent with 0.4.11
14:48 kahrl       happened*
14:48 nrzkt       it's not a problem
14:48 kahrl       it will be worse if it's done several months in advance
14:49 nrzkt       feature freeze stop the innovation and we fix problems. If the dev are not available and needed fixes are critical, stay in FF a little bit more. Release must be stopped if the problem is blocking
14:49 nrzkt       but not if all milestone blocking fixes have been done
14:53 nrzkt       kahrl, #2212 updates as you like
14:53 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
14:58 kahrl       nrzkt: I think making branches during feature freeze was one of the ideas in the release engineering discussion
14:58 kahrl       not sure what happened to it, as I said, it devolved
14:58 nrzkt       maybe minetest project need hands
14:59 nrzkt       and then i could help into release engineering (it's my job)
14:59 kahrl       I think you should talk to hmmmm about it, he was the one who proposed it IIRC
15:00 nrzkt       no problem, it's interesting. I don't thing minetest can evolve if minetest code get three years old compat
15:01 kahrl       three years is nothing
15:01 kahrl       other software has compatibility layers for decades worth of stuff and still manages to evolve
15:02 nrzkt       we aren't X11
15:02 nrzkt       we are a game
15:02 nrzkt       compat with three years old client is bad for innovation
15:02 kahrl       I don't mean compatibility with clients
15:02 kahrl       but for maps it is important
15:03 nrzkt       ofc course
15:03 nrzkt       when release was finished and my network rework done i will look at map store
15:03 nrzkt       if you said it's a version per mapblock maybe there is something to do
15:04 nrzkt       we cannot let old map, and thinking about an upgrading process is important in a rolling developpement
15:04 nrzkt       development*
15:06 kahrl       I don't see the point of forcing every user to do some extra stuff each release
15:06 kahrl       when a little piece of code in the engine can do it incrementally
15:07 nrzkt       maintenance of the code
15:07 kahrl       (in fact, I hate how every web application on the planet forces me to babysit its upgrade process and can't deal with an old DB schema on its own)
15:08 nrzkt       i'm using ~50 free & opensource web applications and upgrading it. Each application has an upgrade process. Why ? because of schema modification, new features, performance improvements...
15:09 nrzkt       i understand your conservative mind, but if the map needs new features, then upgrade it. Old code can be remove and there is less maintain to do on it
15:09 kahrl       mapblock serialization is changed so rarely nowadays that I don't see how it is a huge maintenance burden
15:09 nrzkt       if it's not changed, why not then
15:10 nrzkt       but why not migrate old users to have a convergent model ?
15:10 kahrl       they are
15:10 kahrl       incrementally as the map is loaded
15:10 nrzkt       this permit in the future to improve mapblock serialization by removing old stuff
15:11 nrzkt       i do'nt talk about new records, but oldest
15:11 kahrl       I sometimes download maps from the 0.3 era and check them out
15:12 nrzkt       i agree with you for this core
15:12 kahrl       if I had to keep around old builds for that or go through a length migration process, I wouldn't
15:12 kahrl       lengthy*
15:12 kahrl       so that would decrease my enjoyment of the game
15:12 kahrl       and I'm sure it would for others too
15:12 nrzkt       and if a developper propose a patch to migrate old database structures ?
15:13 kahrl       what would be the difference to how it is now?
15:13 nrzkt       what is the problem with changing serialization to unify methods ?
15:13 nrzkt       the different is that the algo will be must simpler to maintain
15:13 kahrl       you would still have the same migration code, just in a different place
15:13 kahrl       how would it be simpler?
15:13 nrzkt       migration code is a simple code call in one point sometimes. Managing different mapblocks is called many times
15:14 nrzkt       add new mapblock format need a new ugly condition to manage, think about old mapblock version etc... if you add a migrating mode you don't think about old mapblock versions, you only think current and migrate users to current one time.
15:17 kahrl       well, I still don't get how you would get around having to write deserialization code for every past serialization version
15:17 kahrl       which is pretty much the only semi-difficult thing about it
15:18 nrzkt       it's an idea, doing it will be a little more difficult, yes, but when it's done, you don't have to do it another time :)
15:19 kahrl       you will, every time you change the version you have to add code to handle the "new" old version
15:20 nrzkt       old course
15:20 nrzkt       ofc*
15:21 nrzkt       but, in fact, it's only a code move. You move deserialize from core to updater, you read, modify and serialize with new format (which is the current), and remove old serialize format from core.
15:21 T4im        schema migration frameworks exists for that, that are able to transform old schemata to the newest before import… but I'm not aware of open source implementations for c/c++ :/
15:21 nrzkt       it needs a little more devel process but permit to have a homogeneous db
15:23 nrzkt       T4im, this works very well for PgSQL, MySQL, Sqlite, but for Redis and leveldb, don't know
15:23 nrzkt       forum is down
15:24 kahrl       schema migration doesn't really work in this case, as the changes usually affect the compressed blob
15:26 kahrl       something like #1845 would be the only exception to that
15:26 ShadowBot   https://github.com/minetest/minetest/issues/1845 -- Split block position into separate fields in SQLite3 database by ShadowNinja
15:37 celeron55   < nrzkt> forum is down <- noted
15:38 hmmmm       https://github.com/minetest/minetest/pull/1845
15:38 hmmmm       I'm not a huge fan of this
15:38 hmmmm       you either change all databases or one
15:38 hmmmm       or none i mean
15:39 hmmmm       the point is to have a consistent scheme across each because the only difference between the databases is their interface, not the data contained
15:40 hmmmm       what would be better imo would be to fix the "position hash" thing to pack the 12-bit block position values into a 36 bit integer using bitwise airthmetic, not that odd factor/addition thing that currently exists
15:41 hmmmm       what currently exists seems more difficult to me to prove that it's one-to-one
15:42 celeron55   i'm voting for this change
15:42 celeron55   shadowninja's one, i mean
15:42 celeron55   i've been asking for it for like years
15:42 hmmmm       why would you want to change sqlite but none of the others though
15:42 hmmmm       that is so inconsistent
15:42 celeron55   well for example redis is maintained by sfan5 on the terms that it can be dropped if nobody is willing to update it when needed
15:42 celeron55   we just don't have a /contrib/ directory for it
15:42 hmmmm       is redis bad or something?
15:43 celeron55   no, it's just generally unnecessary
15:43 hmmmm       i'll agree with that.  but i thought it was supposed to be the fastest of the three
15:43 celeron55   it has a different architecturer so it cannot be compared (it needs a separate database server)
15:43 celeron55   -r
15:44 celeron55   or, can, but doing it is relatively pointless
15:44 celeron55   i mean, when it's useful, it's useful because of its architecture, not because of speed
15:46 celeron55   and the only program that is ever going to support all of these at once is going to be MT itself as long as we don't publish a library for doing it, in which case it's the library
15:46 gregorycu   hmmmm: I addressed your concerns with #2156, are you able to confirm and +1 ?
15:46 ShadowBot   https://github.com/minetest/minetest/issues/2156 -- Optimise MapBlockMesh functions by gregorycu
15:47 Zeno`       gregorycu, I've asked you to speak to me for 3 days
15:47 Zeno`       you will not answer
15:48 Zeno`       If you refuse to talk to people why should the PR be trusted?
15:48 kahrl       nrzkt: I'm okay with #2212 now. I would prefer to have a comment in mapblock.h about the precondition version >= SER_FMT_CLIENT_VER_LOWEST, and perhaps keep the comment in .cpp that <24 won't work because of dynamic node IDs, but both of these aren't strictly necessary
15:48 ShadowBot   https://github.com/minetest/minetest/issues/2212 -- Fix a crash (assert) when client set serial version < 24 in INIT and by nerzhul
15:49 Zeno`       If I merge the PR and something goes wrong, how can it be discussed?
15:49 Zeno`       This is silly
15:50 kahrl       perhaps the comment about <24 can be moved above the definition of SER_FMT_CLIENT_VER_LOWEST
15:50 nrzkt       ok kahrl, i'm releasing our product at work, then i cannot doing it now, can this be done at merge by you or Zeno ?
15:51 kahrl       sure
15:51 Zeno`       yeah, I'm ok with that
15:51 Zeno`       I can't merge atm (in Windows ;3)
15:51 Zeno`       but if kahrl is happy then I agree
15:51 nrzkt       thanks !
15:52 hmmmm       gregorycu, I don't think I can unless you address Zeno`'s concerns
15:53 hmmmm       sorry (:
15:53 gregorycu   What are those?
15:53 hmmmm       I don't know, whatever he's asking
15:53 gregorycu   He wants me to talk to him in IRC, I think he is lonely or something
15:53 hmmmm       Did you put him on ignore or something?
15:53 gregorycu   Yes
15:53 hmmmm       That's not very friendly
15:53 gregorycu   No, he isn't
15:53 gregorycu   err... neither is putting him on ignore
15:53 hmmmm       =/
15:54 Zeno`       Show me once where I was not friendly :/
15:54 hmmmm       I think it's childish but whatever.  Zeno, did you post your concerns on github??
15:54 hmmmm       in the PR
15:54 Zeno`       I cannot in good faith reopen that PR when I am put on ignore for no apparent reason
15:54 Zeno`       hmmmm, I have :)
15:54 Zeno`       also in the logs
15:55 hmmmm       well
15:55 hmmmm       there's no real way of communicating between both of you
15:55 Zeno`       nope
15:55 celeron55   gregorycu: 17:48:41 <+Zeno`> If you refuse to talk to people why should the PR be trusted? 17:49:42 <+Zeno`> If I merge the PR and something goes wrong, how can it be discussed?
15:55 gregorycu   You probably want me to address those two points
15:55 celeron55   gregorycu: kill the ignore right now, Zeno` is a core developer
15:56 hmmmm       a level of communication is expected with the rest of the team
15:56 kahrl       nrzkt, Zeno`: https://gist.github.com/kahrl/7a4584b58164545d3288
15:56 gregorycu   Is IRC a requirement to contribute?
15:56 kahrl       okay to push?
15:57 nrzkt       okay
15:57 Zeno`       kahrl, looking
15:57 celeron55   gregorycu: it kind of is, unless you refuse to use IRC altogether
15:57 Zeno`       kahrl, yeah I think this is ok. The preconditions are now documented and the assert is there
15:57 gregorycu   So, if I leave IRC and don't return, can I get my PR reopened?
15:57 hmmmm       it isn't, but cowboy coding is frowned upon
15:58 gregorycu   I am willing to listen to everybodies concerns about my code
15:58 celeron55   gregorycu: at least state in your pull requests and issues if you are on IRC, but do not want to discuss it on IRC
15:58 celeron55   people assume that if you are here, you are willing to discuss things here
15:58 hmmmm       ughg
15:58 ShadowNinja hmmmm: About the SQLite3 pull: The other DBs are KV stores, so they'll have to be done in a different way.  If they work with binary keys I'd enode the position into a string and use that as the key insteak of the current ASCII-decimal-hash format.
15:59 gregorycu   I can do that
15:59 kahrl       nrzkt, Zeno`: pushing now
16:00 nrzkt       ok, thanks you, i will not crash 0.4.12 servers next week :D
16:00 hmmmm       I have like real work todo
16:00 hmmmm       so i'll talk more about this later
16:00 celeron55   gregorycu: i can respect that as long as you make it clear; IRC is terrible sometimes
16:02 gregorycu   I have updated the specific PR in question. I've already been miscategorised once about what was/wasn't said on IRC, so I'd just prefer to be clear.
16:02 Zeno`       There are logs
16:02 Zeno`       You can read them, gregorycu
16:04 Zeno`       #2219... yikes
16:04 ShadowBot   https://github.com/minetest/minetest/issues/2219 -- Singleplayer segfaults if server rejects client
16:05 Zeno`       game_has_run = true ... is that correct?
16:05 * Zeno`     looks at main.cpp
16:09 fz72        hey there, question: why is #2211 closed and not merged? There is fix merged but nogosang said that this fix doesn't work correctly
16:09 ShadowBot   https://github.com/minetest/minetest/issues/2211 -- Fix map_meta.txt error when loading a new world (last try) by ngosang
16:11 kahrl       ok, before I push, I loaded an old version (around ser ver 20) to make sure it can be loaded
16:12 kahrl       indeed it could, so I'm pushing for real now
16:12 Zeno`       :)
16:13 nrzkt       :p
16:32 nrzkt       go back home, see u later
16:45 fz72        I can confirm what nogosang said. I tested it. Who is kwolekr in IRC?
16:47 exio4       fz72, hmmmm
16:47 fz72        thanks exio4
16:48 ShadowNinja hmmmm: https://github.com/minetest/minetest/commit/eeea454bff0cfcda495c20029a0246f63f14393e#commitcomment-9462726
16:48 hmmmm       ShadowNinja:  That IS a fix.
16:49 ShadowNinja hmmmm: Create to worlds with different mapgens, load the first world, see second mapgen.
16:49 ShadowNinja two*
16:49 hmmmm       ShadowNinja:  I am aware of this.  It is absolutely not related in any manner to the bug solved by that commit.
16:50 ShadowNinja hmmmm: It is related because the map_meta.txt should always exist.
16:50 hmmmm       ShadowNinja:  Says who?
16:50 ShadowNinja hmmmm: Me?
16:50 hmmmm       map_meta.txt is a set of parameters.  If the file doesn't exist, it's equivalent to there being no parameters specified in an existing map_meta.txt.
16:50 ShadowNinja Otherwise you don't know what the mapgen's supposed to be.
16:51 hmmmm       It falls back onto the defaults.  This is the SAME exact behavior as every other configuration file.
16:51 ShadowNinja You just have to guess from the main settings.
16:52 hmmmm       If you don't know what the mapgen is supposed to be, it defaults to whatever is set in the current working global settings.
16:52 ShadowNinja hmmmm: I think it should be required becuase otherwise you'll make worlds with multiple mapgens whenever someone updates the defaults.
16:52 fz72        hmmm: https://github.com/minetest/minetest/pull/2184
16:53 hmmmm       ShadowNinja:  That's nonsense.  This does not affect anybody in that manner in reality.
16:53 hmmmm       ShadowNinja:  map_meta.txt gets created and populated with the mapgen parameters when it does start generating map.  The behavior you stated in your last line is expected and wanted.
16:53 ShadowNinja hmmmm: It's not a common bug, but it's a bug nontheless.
16:53 hmmmm       ShadowNinja:  And I recently decided that Minetest not having dishwashing functionality is a bug.
16:54 PilzAdam    hmmmm, why is the user prompted to select a mapgen at creation time then?
16:54 ShadowNinja hmmmm: Yes, it's populated then, but it has to be populated on creation.
16:54 PilzAdam    move the dialog to first startup then
16:54 hmmmm       PilzAdam:  Because somebody else did that who doesn't understand the mapgen system.
16:54 PilzAdam    move the dialog then
16:54 ShadowNinja hmmmm: Don't be unreasonable.
16:54 hmmmm       PilzAdam:  I did not advocate creating a mapgen selection dialog.
16:54 PilzAdam    remove it then
16:54 PilzAdam    but don't keep it this broken state
16:55 hmmmm       PilzAdam:  It's not broken.
16:55 hmmmm       If you don't like the interface provided, let's talk about changing the interface.
16:55 ShadowNinja It should be at creaton, like every other volel sandbox game.
16:55 hmmmm       But don't tell me that it's bugged when it isn't
16:55 PilzAdam    it does not work the way users expect it; so it's broken
16:55 hmmmm       That's the most ridiculous argument I've ever heard
16:56 hmmmm       the users expect some insane things at times.
16:56 Brains      Apparently these users here can't agree what the expectation is...  Users must be broken.
16:56 hmmmm       Not that this particular expectation is insane, but if you were to say in general that users are always right, then you're living in a fairyland
16:56 fz72        I think nobody wants to see this dialog. So whats wrong with nosang's PR? He fixed that problem!
16:56 hmmmm       I was not aware of this PR at all until you pasted it here.
16:56 ShadowNinja The customer is always right!  :-P
16:57 hmmmm       ShadowNinja:  Stores stopped respecting that phrase.
16:57 PilzAdam    when it comes to GUI then the user is always right
16:57 PilzAdam    since the GUI is created for the user, not the other way round
16:57 hmmmm       But the user is only one user, not the majority of users
16:57 hmmmm       One user can't speak for everybody
16:58 PilzAdam    every single person here thinks that it's wrong, except hmmmm
16:58 * Brains    will disagree just on principle.
16:58 hmmmm       Fine.  I'll populate a map_meta.txt on initializeWorld() using the current working settings.'
17:00 Brains      Hrm, forums and wikis are down...
17:01 PilzAdam    hmmmm, cool
17:02 hmmmm       But the point is that commit is not wrong at all in what it does
17:02 hmmmm       It simply does not solve something different you somehow mistakenly believe is related
17:04 hmmmm       PilzAdam:  What's up with backend = sqlite3 being hardcoded?
17:04 PilzAdam    hm?
17:04 fz72        hmmmm: That's true, but #2184 got closed for this reason
17:04 ShadowBot   https://github.com/minetest/minetest/issues/2184 -- Fix map_meta.txt error when loading a new world by ngosang
17:05 hmmmm       fz72:  I did not close that.
17:05 Zeno`       I closed it
17:05 Zeno`       because the issue seemed to be resolved in a sensible manner
17:06 hmmmm       I don't like the throwing of an exception if map_meta.txt is not found
17:06 hmmmm       changing it to infostream is sort of dumb
17:06 ShadowNinja hmmmm: SQLite3 is curreltly a hard dep, unlike the other DBs (it's also used for rollback).
17:07 hmmmm       when is a game's minetest.conf override loaded?
17:07 hmmmm       wondering if this is the wrong approach.  I want to capture the state of the configuration at the point just before mods get run
17:08 sfan5       :/
17:08 sfan5       ShadowNinja: why does your pull add static_cast's to database-redis.cpp
17:09 sfan5       it uses the libhiredis C interface
17:09 sfan5       static_cast will do exactly the same as now
17:09 sfan5       (feel free to correct me, my source for this is an answer on stackoverflow)
17:10 hmmmm       I agree C style casts should probably be used there
17:10 PilzAdam    sfan5, typing more, useless letters helps you memorize routines while coding and ultimately makes you a better programmer
17:10 hmmmm       it's the least-weird thing and it gets the correct intended behavior
17:12 hmmmm       C++ Standard, 5.4.5
17:13 exio4       PilzAdam, didn't know programming was more about memorizing and less about logic and data dependencies/flow
17:13 PilzAdam    exio4, the point of programming is obviously the typing
17:13 sfan5       PilzAdam: oh, i see, does this mean we should also use variable names like this_variable_stores_the_amount_of_items_the_player_has_in_the_current_slot?
17:14 PilzAdam    why would there be so much flamewars about tabs vs. spaces if it wasn't about the typing?
17:14 * hmmmm     blinks
17:14 hmmmm       PilzAdam wasn't being sarcastic?
17:14 hmmmm       I'm confused
17:15 exio4       now I don't know what is sarcasm and what isn't :)
17:16 hmmmm       Is it just me or does filesys.cpp absolutely suck?  Everything done there is done in an overcomplicated, ridiculous manner
17:45 hmmmm       just noticed an issue where you can't run minetest under valgrind on freebsd
17:45 hmmmm       how do the other OSes work where the correct intended executable path is retrieved?
17:46 hmmmm       can't rely on /proc/self/exe like linux since procfs is optional
18:12 kilbith     kahrl: you've forgot to close 2212 after merged
18:35 nrzkt       Network rework on packet writing is in a good way, writer is fully operational ! Now i can convert every Send to new NetworkPacket
18:49 kahrl       kilbith, thanks
19:44 nrzkt       #freeminer ?
19:44 nrzkt       rocket launch done :p
19:59 est31       !g rocket launch
19:59 ShadowBot   est31: https://www.kennedyspacecenter.com/events.aspx?type=rocket-launches | Find out what rocket launches are taking place at Kennedy Space Center Visitor Complex. Launch date, time and viewing opportunities are subject to change.
20:30 PilzAdam    it seems like everyone just forgot / ignored that the 0.4.11 build for ubuntu 12.04 failed: https://launchpadlibrarian.net/193537189/buildlog_ubuntu-precise-i386.minetest_0.4.11-0ppa1~ubuntu12.04.1_FAILEDTOBUILD.txt.gz
20:30 PilzAdam    sfan5, it seems to be related to the redis backend; could you look at it?
20:31 sfan5       how did you come to that conclusion
20:31 sfan5       anyway
20:31 sfan5       that build fail is leveldb related
20:31 sfan5       libsnappy/libleveldb/both are/is broken in ubuntu 12.04
20:32 sfan5       this is also the reason why the travis build download a custom leveldb build
20:32 PilzAdam    oh, yes, mixed up leveldb and redis
20:33 PilzAdam    does anyone know how to configure the launchpad build to not compile with leveldb, so we can get a working build?
20:34 sfan5       PilzAdam: isn't there some sort of build script you can change?
20:34 PilzAdam    celeron55, who was the guy that recently changed the launchpad build scripts to include more optional stuff?
20:36 PilzAdam    seems like it could be changed here: http://bazaar.launchpad.net/~minetestdevs/minetest-c55/packaging/view/head:/rules
20:37 sfan5       huh
20:37 sfan5       hm
20:39 celeron55   PilzAdam: i don't know
20:39 PilzAdam    do you know how to edit the rules file?
20:40 PilzAdam    another way would be to build with revno:22, but it would lack freetype and gettext
20:40 celeron55   you have to use the package manager
20:40 celeron55   called bazaar, that launchpad wants to use
20:40 celeron55   lol i mean version control program
20:40 celeron55   not package manager
20:41 celeron55   because... i guess the world needs more version control systems
20:41 sofar       git is soooo bad ;^)
20:42 celeron55   bazaar is the ultimate NIH of VCSes just like Mir is the ultimate NIH of windowing systems
20:42 celeron55   both pretty much by ubuntu
20:42 celeron55   it's stupid
20:42 sofar       out of curiosity, what OS's do most core MT devs use to develop?
20:42 sfan5       Linux
20:42 celeron55   various gnu/linux distributions
20:43 sofar       I know why they did MIR - purely to enable android drivers
20:44 * sofar     works on linux distributions for Intel... hence my question
20:50 celeron55   do you want a more specific answer? you'll need to set up a poll for that
20:51 celeron55   for us it's simply a matter of allowing anyone to use any distro they like
21:06 sofar       oh no, I figured most would be on Linux, so, I got my answer
22:52 est31       If anyone wants to discuss #2220 in irc not on github, I'm around :)
22:52 ShadowBot   https://github.com/minetest/minetest/issues/2220 -- add wait_clicked and texture_waiting to formspec button or image_button