Time Nick Message 07:21 hmmmm ugh 07:22 hmmmm does anybody have objections to placing object files into src/CMakeFiles/.dir/ 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