| Time |
Nick |
Message |
| 00:24 |
|
ShadowBot joined #minetest-dev |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 06:01 |
|
ivanbu joined #minetest-dev |
| 06:08 |
|
proller joined #minetest-dev |
| 06:31 |
|
calcul0n_ joined #minetest-dev |
| 08:58 |
MTDiscord |
<paradust> Trying 5.8.0 for the first time on linux, and it crashed with an xcb assertion (part of libX11). Is this known issue, or should I try to debug? |
| 09:22 |
|
Warr1024 joined #minetest-dev |
| 09:30 |
|
YuGiOhJCJ joined #minetest-dev |
| 09:46 |
|
Warr1024 joined #minetest-dev |
| 12:28 |
MTDiscord |
<paradust> Update: It turns out libX11 is buggy and may not be internally thread-safe, despite offering the option to not initialize thread safety. |
| 12:28 |
MTDiscord |
<paradust> They gave up and just enabled thread safety by default: https://gitlab.freedesktop.org/xorg/lib/libx11/-/commit/afcdb6fb0045c6186aa83d9298f327a7ec1b2cb9 |
| 12:29 |
MTDiscord |
<paradust> I noticed the latest libX11 wasn't crashing, so I did a bisect and traced it to that |
| 12:30 |
MTDiscord |
<paradust> if I take the latest libX11 and disable XInitThreads, it still crashes |
| 12:30 |
MTDiscord |
<paradust> An easy fix would be for minetest to just call XInitThreads itself, so that older broken libX11 don't crash |
| 12:32 |
MTDiscord |
<paradust> I didn't see any evidence that minetest is accessing X11 from multiple threads, so it is most likely something else (could be Mesa) that is interfacing with xcb or x11 from multiple threads and triggering a race |
| 12:51 |
[MTMatrix] |
<localhost> time to switch xlib :) |
| 12:51 |
[MTMatrix] |
<localhost> time to switch xlib ? :) |
| 12:56 |
MTDiscord |
<paradust> is there an alternative? |
| 12:58 |
MTDiscord |
<rollerozxa> curious to know, does the crash also occur with the SDL2 device? (I assume you're using the X11 device which is still the default) |
| 13:01 |
MTDiscord |
<paradust> It appears SDL2 always calls XInitThreads, so I would assume not |
| 13:01 |
MTDiscord |
<paradust> https://github.com/libsdl-org/SDL/blob/main/src/video/x11/SDL_x11video.c#L113C24-L113C24 |
| 13:18 |
sfan5 |
feel free to send a PR to irrlichtmt |
| 13:19 |
sfan5 |
but it's best not to spend too much time on this since the x11 driver is going away anyway |
| 13:52 |
|
proller joined #minetest-dev |
| 14:20 |
|
imi joined #minetest-dev |
| 14:22 |
|
cx384 joined #minetest-dev |
| 15:25 |
|
Desour joined #minetest-dev |
| 16:47 |
MTDiscord |
<warr1024> Can I get a second reviewer for #14054? Seems like a really simple change, should be easy to get merged, right? |
| 16:47 |
ShadowBot |
https://github.com/minetest/minetest/issues/14054 -- Manually configurable minimum protocol version by Warr1024 |
| 17:21 |
pgimeno |
OpenGL in general is not thread-safe, if that matters - not just mesa |
| 17:41 |
|
calcul0n_ joined #minetest-dev |
| 17:55 |
Krock |
will merge #14010 #14083 and #13093 in 15 minutes |
| 17:55 |
ShadowBot |
https://github.com/minetest/minetest/issues/14010 -- GUIFormspecMenu: Fix race condition between quit event and cleanup in Game by SmallJoker |
| 17:55 |
ShadowBot |
https://github.com/minetest/minetest/issues/14083 -- Add sound volume when unfocused setting by srifqi |
| 17:55 |
ShadowBot |
https://github.com/minetest/minetest/issues/13093 -- Avoid movement jitter while attached by lhofhansl |
| 18:09 |
Krock |
merging |
| 18:12 |
Krock |
done |
| 18:51 |
sfan5 |
#13092 is now ready for reviews |
| 18:51 |
ShadowBot |
https://github.com/minetest/minetest/issues/13092 -- Lua on each mapgen thread by sfan5 |
| 18:58 |
Krock |
cool. I did not think it would become this large of a PR |
| 19:05 |
pgimeno |
awesome! |
| 19:05 |
pgimeno |
is there any means of communication between threads? |
| 19:05 |
sfan5 |
one-way only |
| 19:05 |
sfan5 |
(emerge -> main) |
| 19:10 |
MTDiscord |
<warr1024> Krock: if you're around ... would Settings be an acceptable place to refactor the protocol version limits into? I've got a test branch I'm working on at https://github.com/minetest/minetest/compare/master...Warr1024:minetest:protomin2?expand=1 |
| 19:11 |
Krock |
@warr1024: Settings is used by client and server. This here is server-specific, thus I'd rather find a place for it in the ServerEnvironment or Server class |
| 19:11 |
Krock |
after all, this can be a static function |
| 19:12 |
MTDiscord |
<warr1024> Oh geez, it's times like this that make me wish I knew C++... |
| 19:12 |
Krock |
oh |
| 19:13 |
MTDiscord |
<warr1024> If it's got a g_settings then I can probably move it there. |
| 19:13 |
Krock |
it's easy. you just put the `static` keyword in front of the function (header file) and it'll be accessible without the instance itself |
| 19:14 |
Krock |
I see how Settings also kind of makes sense since you're only using functions from that class.... it's a question of concept. perhaps it's best to ask for a second opinion |
| 19:14 |
MTDiscord |
<warr1024> If I make it static then that would mean I'd need to pass in the settings then, since settings live in instance-land ... would that be okay, or would that be uglier than if I just didn't use static? |
| 19:14 |
Krock |
depends on where you put the function. the server uses `g_settings` for everything anyway |
| 19:15 |
MTDiscord |
<warr1024> Personally, settings didn't feel quite "right" to me either, since looking at settings, almost everything in there is very generic and not application-specific like this. |
| 19:15 |
Krock |
since that's a globally accessible variable it does not really matter where you put the function |
| 19:15 |
MTDiscord |
<warr1024> Ohhh I see, g_settings is like a super global thing, not an instancey kind of thing, so yeah, that'd work. |
| 19:16 |
Krock |
FYI: you don't need to update minetest.conf.example. It'll be done automatically |
| 19:16 |
Krock |
you still can do that if you want, though. |
| 19:16 |
MTDiscord |
<warr1024> So all I need is an opinion about whether Server of ServerEnvironment makes more sense then. |
| 19:17 |
MTDiscord |
<warr1024> I'm guessing Server since the word "protocol" seems to appear there but not in serverenv. |
| 19:20 |
sfan5 |
`Settings` feels like the wrong class to me because in most ways it's just totally generic |
| 19:20 |
sfan5 |
maybe one of the protocol related headers would be good, or server.h |
| 19:22 |
MTDiscord |
<warr1024> It feels weird to be elbow-deep in C++ code and opening up a search on "how to call a static method" right in the middle of it 😏 |
| 19:22 |
sfan5 |
you can also make it a top level static inline u16 getMinProtocolVersion() { ... } |
| 19:23 |
MTDiscord |
<warr1024> Hmm, I forgot about inline ... do those really make a significant difference, or do compilers just make their own decisions about inlining anyway? |
| 19:24 |
Krock |
the compilers tend to put it more in-line but at the end for this case it does not matter. inline only increases the binary size. |
| 19:27 |
sfan5 |
the 'static' here means something different than 'static' for a class function |
| 19:28 |
Krock |
(that is: no exported function name) |
| 19:28 |
MTDiscord |
<warr1024> Haha, I'm vaguely familiar with C and C-descendant languages having multiple weird meanings for "static" in different contexts, but I'm sure I don't know most of them. |
| 19:30 |
Desour |
inline nowadays has a different meaning than "please inline". see <https://en.cppreference.com/w/cpp/language/inline>. TL;DR: you can define the function in multiple translation units |
| 19:36 |
MTDiscord |
<warr1024> error: cannot declare member function ‘static irr::u16 Server::getProtocolVersionMin()’ to have static linkage |
| 19:36 |
MTDiscord |
<warr1024> apparently I used the wrong kind of "static" here? |
| 19:36 |
Krock |
no static in the C++ file |
| 19:37 |
MTDiscord |
<warr1024> That's nice and confusing 😏 |
| 19:37 |
Krock |
for example: these functions here are all static: https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_env.cpp#L293 |
| 19:39 |
MTDiscord |
<warr1024> https://github.com/minetest/minetest/compare/master...Warr1024:minetest:protomin2?expand=1 has been updated |
| 19:40 |
MTDiscord |
<warr1024> Should I replace my original PR with a squashed version of this then? |
| 19:47 |
Krock |
yes, looks good. By the way. the maximal protocol version is always SERVER_PROTOCOL_VERSION_MAX. That and LATEST_PROTOCOL_VERSION have the same value. I don't know why both exist. |
| 19:47 |
Krock |
"strict_protocol_version_checking" makes no difference there, however it could still be desirable to force a version later on? I don't know. |
| 19:48 |
[MTMatrix] |
<localhost> oh noes sounds like deprecation |
| 20:02 |
MTDiscord |
<warr1024> I was wondering why proto max and proto latest were separate, myself. I assumed it was for some arcane reason that I just didn't know but would make perfect sense if I ever heard it 😆 |
| 20:07 |
[MTMatrix] |
<localhost> some stupid question (maybe): where i can find expain of version rules protocol |
| 20:10 |
Krock |
the what? |
| 20:12 |
[MTMatrix] |
<localhost> the server protocol version |
| 20:12 |
Krock |
https://github.com/minetest/minetest/blob/master/src/network/networkprotocol.h#L218 |
| 20:15 |
MTDiscord |
<warr1024> #14054 rebased |
| 20:15 |
ShadowBot |
https://github.com/minetest/minetest/issues/14054 -- Manually configurable minimum protocol version by Warr1024 |
| 22:25 |
|
YuGiOhJCJ joined #minetest-dev |
| 23:32 |
|
panwolfram joined #minetest-dev |
