Time Nick Message 00:47 hecks I need a way to check the client protocol version in connectionthreads.cpp:580, any hints? 00:53 hecks oh wait, I think I found a better place to hook into 01:08 hecks okay, the channel funnel is done - now how do we futureproof this to avoid having to do this ever again? 01:10 hecks option 1: statically allocate like 32 or 64 channels on the premise that no more will ever be needed anyway 01:10 hecks option 2: use a vector and ask the server how many channels it needs 01:13 hecks but wait, does the client ever give a damn besides that assert? 01:15 hecks option 3: preallocate 256 channels because it's not like silent channels eat much cpu or ram 01:17 hecks screw this, I'll just give it 256 unless someone has a better idea 02:17 MTDiscord 712*92 total possible entities? 02:17 MTDiscord That's easily going to be overrun in the future. 02:18 MTDiscord Why not just go uint64? 02:45 hecks new problem, the funnel... isn't working? 02:52 hecks help me out here, I commented out the funnel and by all logic the 5.3 client should just die 02:52 hecks instead it's simply ignoring the packets 03:02 lhofhansl Hey all... Going to merge #10698 within a few. 03:02 ShadowBot https://github.com/minetest/minetest/issues/10698 -- Revert "Increase limit for simultaneous blocks sent per client and the meshgen cache." by sfan5 03:04 lhofhansl It ended up making not much a change in maploading after all. The important changes were the emerge queue limit increases and the "correct" parallel mapgen with multiple emerge threads. 03:04 lhofhansl (Seem everyone is happier with this one reverted :) ) 03:06 lhofhansl hecks: see of that improves your base problem. I still think we more channels! 03:06 lhofhansl merged 03:16 hecks lhofhansl: I already nerfed the block thing in my config, it helped a little but only delayed the queue death 05:21 celeron55 hecks: i think 256 channels (or less) is enough because it's not a problem to stuff a few extra things in one channel, you still get the same benefit 05:22 celeron55 the server just has to map things to the available channels 05:22 celeron55 so u8 is fine 05:22 hecks I was asking more if anyone has any problems with having as many as 256 05:23 hecks as opposed to something that scales, I guess quiet channels aren't very expensive 05:23 celeron55 well what's the overhead 05:23 celeron55 apparently simply that many Channel objects 05:24 celeron55 per connection 05:24 celeron55 that's basically nothing 05:24 hecks pretty much, there's a few places where it iterates over the channels but 256 iterations isn't much 05:24 celeron55 there's a mutex in each i guess 05:25 hecks now why could possibly my compat funnel not be working 05:27 hecks https://github.com/hecktest/minetest/blob/8d7d7022ac62e446851005352cd358452e9d65a1/src/clientiface.cpp#L716 05:27 hecks what am I doing wrong here? 05:28 lhofhansl I'll take a look tomorrow. 05:28 hecks :[ 05:28 lhofhansl celeron55: we reverted the 10627 :) 05:28 lhofhansl night night folks. 05:29 celeron55 legacyChannelMap should probably be a lookup table for optimization 05:30 celeron55 it's just a 256->something mapping 05:30 celeron55 i mean u8->something 05:30 hecks the legacy map is something I did 05:30 celeron55 i know 05:30 hecks it's just new channels -> 3 old channels 05:30 hecks and yeah 05:30 hecks might as well be a u8[256] 05:31 celeron55 i don't know why it doesn't work though, does that line of code run? 05:32 hecks damned if I know, let me verbosestream it and try it on the server box 05:32 celeron55 umm 05:32 celeron55 well probably not, because you have protocol version set to 40? 05:32 celeron55 i mean, is your client the appropriate version 05:34 celeron55 eh whatever, should be easy to debug with a breakpoint or some stupid log prints 05:41 hecks https://a.uguu.se/VJC87CuLjDxB_verbose.jpg 05:42 hecks well this is a compat feature so I'm connecting with 5.3 05:42 hecks it's not a problem at all on a matching client 05:46 celeron55 did you add some logging to ClientInterface::patchLegacyChannel()? 05:47 celeron55 clearly there's no new log lines in that screenshot 05:49 hecks yeah, now I added more and 05:49 hecks by the looks of it, patching is never called 05:50 hecks which would mean getClientNoEx(peer_id) is not working 05:50 hecks because the negotiated version is definitely 39 and not 40 05:53 celeron55 well there are other places where Connection::Send() is called 05:54 celeron55 ClientInterface::sendToAllCompat 05:54 celeron55 i think you forgot that one 05:54 hecks that isn't called anywhere 05:54 hecks it's dead code 05:55 celeron55 altough... yeah, i wonder what's up with that 05:55 hecks it's actually failing to find the RemoteClient https://a.uguu.se/6zKsJaaYdNBl_notfound.jpg 05:55 hecks this is init, those are nodedefs and media announces 05:56 hecks getClientNoEx just fails 05:57 celeron55 umm 05:57 hecks does the code just not have a remote client at this stage? 05:57 celeron55 what happens if you swap MTSCMC_INIT and MTSCMC_HUD 05:57 celeron55 wait 05:57 celeron55 there's 4, 6 and whatever also 05:58 hecks inventory 05:58 hecks media 05:58 hecks position, and time of day I guess 05:59 hecks in any case, remoteclient or not, I need to obtain the protocol version in this function somehow 05:59 celeron55 ah 05:59 celeron55 ClientInterface::getClientNoEx checks the client's initialization state 06:00 celeron55 and the default for that argument is ClientState state_min = CS_Active 06:00 hecks uhhhh the log occasionally says a packet has been patched on the HUD channel so that's working 06:00 celeron55 pass CS_Invalid to it and it'll work 06:00 celeron55 RemoteClient *rcl = getClientNoEx(peer_id, CS_Invalid); 06:00 hecks oh? and uhhhh with that arg it'll always work? 06:01 celeron55 by default it only gives out initialized clients and filters out the uninitialized ones 06:01 hecks oh, it's like that, okay 06:02 hecks whoaaaa 06:02 hecks it worked 06:03 celeron55 i kind of hate default arguments 06:04 celeron55 nice idea but in the end they just confuse everyone 06:04 celeron55 becaue nobody reads documentation or even headers 06:04 celeron55 because* 06:12 hecks it's more that the function is a little unusual, it's a getter but also a filter 06:41 MTDiscord Default args are great, in my opinion, why wouldn't someone check a header file? 06:56 hecks wow, here's a bizarre development 06:56 hecks I connected with a 5.3 client and it's even faster 06:57 hecks makes me wonder if anything else changed along the way that made things slower in 5.4 07:04 hecks alright, that may have been a fluke 07:53 hecks #10699 slam 07:53 ShadowBot https://github.com/minetest/minetest/issues/10699 -- Break up the message queue by category by hecktest 18:07 lhofhansl Please chime in on #10597 18:07 ShadowBot https://github.com/minetest/minetest/issues/10597 -- Increase defaults for viewing_range, active_object_range and related settings by lhofhansl 18:27 lhofhansl I'm starting to get the feeling that we're not interested much in making MT better by default for reasonably decent machines. 18:30 sfan5 the question is what about machines that aren't reasonably decent 18:30 sfan5 perhaps there needs to be a choice between several preset settings 18:30 sfan5 or even some kind of automatic detection 18:31 lhofhansl That's what I am thinking. And more automatically derived settings as I suggest at the of #10683 18:31 ShadowBot https://github.com/minetest/minetest/issues/10683 -- Frame stutter seems worse since 2-3 months ago 18:32 lhofhansl server and client need to be configured together 19:46 lhofhansl Have a look at #10700. Simple change, only does mesh translation for visible blocks. 19:46 ShadowBot https://github.com/minetest/minetest/issues/10700 -- Only translate meshes for visible blocks - reduces client stutter. by lhofhansl 19:47 lhofhansl hecks: Has a better solution that we should work on afterwards. 19:48 sfan5 I don't think that works 19:48 lhofhansl Why not? 19:49 sfan5 updateCameraOffset will move the meshes by how much the camera offset changed, if this is skipped the positions will be wrong once the blocks are visible again 19:49 lhofhansl We never update all the blocks anyway in that loop. Only the sectors in range, so this does not rely on all blocks' meshes to be translated. 19:49 sfan5 hmm 19:50 sfan5 I guess each block stores its own offset then 19:50 sfan5 because that'd work 19:50 lhofhansl See the part in the loop above it. 19:51 lhofhansl the camera offset also is a fixed value, no? I.e. it is known and final at any time when we render the map. 19:51 sfan5 yes 19:53 lhofhansl In the end hecks is right and we should just apply the offset to the model matrix. That would need some changes in the shaders (which are covered in #10688) 19:53 ShadowBot https://github.com/minetest/minetest/issues/10688 -- Remove hardcoded matrices from OpenGL shaders. by lhofhansl 19:54 sfan5 that doesn't work without shaders, does it? 19:55 sfan5 at some point it will not feasible to support the fixed GL pipeline, if more rendering improvements are going to come in that point might be soon 20:05 lhofhansl The standard pipeline would do the same. There's always a model, view, and project matrix. 20:05 lhofhansl (i.e. Irrlicht would do the same) 20:11 sfan5 oh right 20:14 lhofhansl I'd still do 10700 for now, so we can reduce that jitter, then do it the right(tm) way 20:15 sfan5 sure 20:18 lhofhansl if you agree could add your approval to the PR? 20:18 sfan5 once i've tested it I will 20:22 lhofhansl Cool! Perhaps we can get paramat to test his scenario and see whether it improves. 20:22 hecks what's up, are we talking about that stupid camera offset? 20:25 sfan5 https://irc.minetest.net/minetest-dev/2020-12-05#i_5758366 20:27 lhofhansl Yes :) 21:14 hecks well the solution to that is basically done in my batching branch 21:14 hecks I can extract that if you want 21:26 sfan5 yes please 21:26 hecks on it 21:36 pgimeno hecks: re #10699, please keep a channel number (or a few ones) reserved for possible future extensibility 21:36 ShadowBot https://github.com/minetest/minetest/issues/10699 -- Break up the message queue by category by hecktest 21:36 hecks pgimeno: the entire u8 of channels is up for grabs now 21:37 hecks the channel array is just [256] 21:37 pgimeno what if more are needed in future? you can't use the trick of mapping 255 to a longer integer 21:37 hecks the channel ID is a u8 21:38 hecks and I doubt that more channels will be needed, instead I'd rather allocate subchannels for specific uses like per-entity queues 21:38 hecks for the broadest categories of packets, 256 is more than enough 21:38 hecks while things like modchannels and entities would benefit from a fully dynamic system layered on top of this 21:39 hecks since entity packets already carry an entity ID right after the header, always 21:39 pgimeno I'm just suggesting to reserve a code for possible future use 21:40 hecks I still don't get it 21:40 hecks we use 17 out of 256 available channels 21:40 hecks most are still free for taking 21:40 hecks and you don't need that many general categories 21:40 hecks I'd feel safe even with [32] or [64], only 256 basically costs just as little so why not 21:42 pgimeno or even limit it to 128 so in future a bit can be used to flag something 21:43 pgimeno you just don't know what you will need, so using everything available knowing that you won't use it doesn't sound wise. 21:43 MTDiscord Ah yes, the future 21:43 MTDiscord TBH the future can break compat 21:46 hecks then chop the protocol when you do that 21:46 hecks it's only [256] to avoid ever having to expand it 21:47 hecks because each expansion requires a funnel 21:48 sfan5 you can theoretically switch the channel no to whatever type in the future, it just requires more annoying compat code 21:50 hecks I suggested before that if we ever run out of 256 channels, you can use 255 as an escape for a longer number 21:52 pgimeno re camera offset, please PLEASE get rid of that abomination, but do it carefully. I doubt that moving the matrix to GL is the solution. 21:53 sfan5 I thought the camera offset was implemented for a reason 21:53 sfan5 the implementation is awful, the concept is not the issue with it as far as I understand 21:54 pgimeno the concept is an abomination 21:54 pgimeno at render time, the camera should be at 0,0,0 and the geometry to render, around it 21:55 pgimeno I implemented that for OpenMiner, and it works fine at a million km from the origin 21:56 hecks https://a.uguu.se/WBvX3cTKA1GU_Clipboard01.jpg it's done and it works 21:56 hecks no cracks, no vertex boil, no jitter 21:56 hecks because you see, what you're supposed to do is collapse the offset and blockpos back to offset space before you BS it and feed it to the matrix 21:57 hecks right as the block is being rendered - the GPU gets offset space stuff and everyone is happy 21:58 pgimeno hecks: is there a PR for that? 21:58 hecks in a minute, I just coded it 21:59 pgimeno my point is that you shouldn't need to account for camera offset everywhere you need to do some rendering 21:59 pgimeno that's what I'm strongly against 22:01 pgimeno I need to go now, will check chat later 22:01 sfan5 hm 22:03 hecks #10702 22:03 ShadowBot https://github.com/minetest/minetest/issues/10702 -- Implement mapblock camera offset correctly by hecktest 22:04 hecks from branch to PR in less time than it took to play the whole doom soundtrack =] 22:18 lhofhansl Nice 22:22 sfan5 haha ok my changes made it not build 22:25 hecks sfan5: force push the original? 22:25 sfan5 i'll fix it 22:25 hecks ok 22:58 lhofhansl sfan5: you wanna merge? Or want me to? 22:59 sfan5 you can 23:00 lhofhansl ok... In a few. One source of jitter down. 23:02 lhofhansl (and I'll squash this time) 23:03 lhofhansl done