| 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:01 |
|
Fixer joined #minetest-dev |
| 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:03 |
|
Taoki joined #minetest-dev |
| 02:17 |
MTDiscord |
<exe_virus> 712*92 total possible entities? |
| 02:17 |
MTDiscord |
<exe_virus> That's easily going to be overrun in the future. |
| 02:18 |
MTDiscord |
<exe_virus> Why not just go uint64? |
| 02:33 |
|
lhofhansl joined #minetest-dev |
| 02:42 |
|
Genshin joined #minetest-dev |
| 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 |
| 03:18 |
|
Ritchie joined #minetest-dev |
| 03:18 |
|
nathanfranke[m] joined #minetest-dev |
| 03:18 |
|
kb1000 joined #minetest-dev |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 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:26 |
|
fluxflux joined #minetest-dev |
| 06:41 |
MTDiscord |
<exe_virus> 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 |
| 08:00 |
|
ShadowNinja joined #minetest-dev |
| 08:55 |
|
calcul0n joined #minetest-dev |
| 09:54 |
|
m42uko joined #minetest-dev |
| 11:26 |
|
absurb joined #minetest-dev |
| 11:39 |
|
T4im joined #minetest-dev |
| 11:46 |
|
MTDiscord joined #minetest-dev |
| 11:55 |
|
calcul0n joined #minetest-dev |
| 12:06 |
|
m42uko joined #minetest-dev |
| 12:19 |
|
proller joined #minetest-dev |
| 12:28 |
|
calcul0n joined #minetest-dev |
| 12:58 |
|
Icedream joined #minetest-dev |
| 14:11 |
|
YuGiOhJCJ joined #minetest-dev |
| 14:21 |
|
Fixer joined #minetest-dev |
| 18:07 |
|
lhofhansl joined #minetest-dev |
| 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:18 |
|
Fractalis joined #minetest-dev |
| 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:30 |
|
lisac joined #minetest-dev |
| 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 |
| 18:32 |
|
homthack joined #minetest-dev |
| 18:37 |
|
hecks joined #minetest-dev |
| 18:46 |
|
fluxflux joined #minetest-dev |
| 18:56 |
|
Fractalis joined #minetest-dev |
| 19:04 |
|
Fractalis joined #minetest-dev |
| 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:02 |
|
hecks joined #minetest-dev |
| 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:19 |
|
hecks joined #minetest-dev |
| 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 :) |
| 20:59 |
|
_Zaizen_[m] joined #minetest-dev |
| 20:59 |
|
Kimapr[m] joined #minetest-dev |
| 21:00 |
|
nathanfranke[m] joined #minetest-dev |
| 21:00 |
|
kb1000 joined #minetest-dev |
| 21:11 |
|
zughy[m] joined #minetest-dev |
| 21:12 |
|
celeron55[m] joined #minetest-dev |
| 21:13 |
|
Benrob0329[m] joined #minetest-dev |
| 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:19 |
|
LoneWolfHT joined #minetest-dev |
| 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 |
|
calcul0n joined #minetest-dev |
| 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 |
<appguru> Ah yes, the future |
| 21:43 |
MTDiscord |
<appguru> 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 |
