Time |
Nick |
Message |
00:01 |
BlockMen |
always when i finished a build something is comitted here... |
00:02 |
PilzAdam |
BlockMen, thats a feature |
00:04 |
BlockMen |
i bet so |
00:05 |
Exio4 |
how many seconds/minutes does it take to compile? |
00:05 |
PilzAdam |
NaN, he uses MSVC |
00:05 |
Exio4 |
exactly, with msvc |
00:06 |
BlockMen |
:P ~10min for me |
00:06 |
Exio4 |
what hardware? |
00:06 |
BlockMen |
i5 2,6ghz |
00:06 |
Exio4 |
k |
00:07 |
Exio4 |
mobile or not? |
00:07 |
BlockMen |
*m |
00:24 |
|
BlockMen left #minetest-dev |
01:17 |
|
aoper joined #minetest-dev |
01:47 |
|
NakedFury joined #minetest-dev |
02:18 |
|
kaeza joined #minetest-dev |
02:44 |
hmmmm |
so weather seems to be always enabled, all the time |
02:50 |
hmmmm |
can somebody please explain to me why it needs to be sent to the client in the first place? |
03:19 |
hmmmm |
https://github.com/kwolekr/minetest/commit/f12078263dd56cfbfa59a140d37f7f2d14fa2dde |
03:22 |
hmmmm |
https://forum.minetest.net/viewtopic.php?pid=103885#p103885 |
03:24 |
hmmmm |
i don't know, now i'm having second thoughts about this |
03:25 |
hmmmm |
when someone uses a raw schematic specifier and they want to simply place things, right now they just have an array of {name="nodehere"}, probably. now they'd have to do {name="nodehere", param1=255} for all of them |
03:25 |
hmmmm |
i don't know how inconvenient that is |
03:39 |
|
smoke_fumus joined #minetest-dev |
03:45 |
kahrl |
hmmmm: perhaps use a different readnode that reads a missing param1 as 255? |
03:50 |
hmmmm |
wouldn't it be weird if a node is different for this one particular API, though? |
03:51 |
kahrl |
a bit, yes |
03:56 |
hmmmm |
i'm not sure what i should do here |
03:57 |
hmmmm |
should i scrap this whole thing and leave it be? or is this change good? |
03:57 |
hmmmm |
i think i will use a different readnode though, as long as this exception to the rule is documented it's fine |
04:05 |
kahrl |
I'm not a modder, but I prefer the new 0-255 range |
04:06 |
kahrl |
huh, is MutexedQueue::pop_back used anywhere? |
04:06 |
hmmmm |
the thing that the old emergethread used? i don't think so |
04:07 |
kahrl |
I'm getting an error when I try to use it, in util/container.h:303 |
04:07 |
kahrl |
because back() does not return an iterator |
04:07 |
hmmmm |
that could've been caused by thexyz's container migration |
04:09 |
kahrl |
looks like it |
04:16 |
kahrl |
it compiles! https://gist.github.com/kahrl/6153470 |
04:16 |
kahrl |
(basically an API for async curl requests) |
04:16 |
hmmmm |
oh boy, DSTACK() |
04:16 |
hmmmm |
haven't seen that used for a while |
04:19 |
kahrl |
I felt like I might use a number of asserts in this code |
04:19 |
kahrl |
.. didn't end up with as many as I thought |
04:19 |
hmmmm |
well DSTACK is much more heavyweight than a couple of asserts, and from what i recall it's not very useful anyway |
04:21 |
kahrl |
it's used for the stacktrace when an assert fails, given the lack of asserts I guess I can remove it |
04:22 |
kahrl |
(none of these functions should be performance relevant though) |
04:22 |
hmmmm |
ah.. didn't realize that |
04:23 |
hmmmm |
we should have a cross between assert and dstack like ASSERTWARN(foobar != NULL, "Something isn't right"); |
04:24 |
hmmmm |
this way you understand that something went wrong, where it went wrong, but without all the extra stuff with the stacktrace which might not be useful |
04:26 |
kahrl |
well you can always do if (...) errorstream<<... |
04:26 |
kahrl |
might not look as nice though |
04:27 |
hmmmm |
well it's an if, an errorstream print, an abort, and then a closing brace so that's 1 line vs. 4 |
04:27 |
hmmmm |
besides, ASSERTs aren't placed if it's not compiled for debug |
04:27 |
hmmmm |
or i guess not |
04:27 |
kahrl |
ah, I thought with the WARN that the program would continue to run |
04:28 |
kahrl |
oh and yes asserts are checked in release builds |
04:29 |
hmmmm |
how does DebugStacker even work? I don't see how it keeps a backtrace |
04:31 |
hmmmm |
oh, you need to have a DSTACK in every single function you'd like to keep track of |
04:32 |
kahrl |
yep |
04:32 |
hmmmm |
that's not very helpful though |
04:32 |
hmmmm |
has DSTACK ever actually helped you solve anything? |
04:33 |
kahrl |
maybe, I can't remember |
04:33 |
hmmmm |
i'm thinking, what if libexecinfo is used instead |
04:34 |
hmmmm |
that way you can get a complete stack traceback, and for release builds you'll still get some useful information (the largest functions will remain after optimization, obviously) |
04:34 |
kahrl |
are release builds built with symbol info? |
04:34 |
hmmmm |
maybe i'm underestimating how much the symbols add |
04:35 |
hmmmm |
currently no |
04:35 |
hmmmm |
i never actually checked the size of the executable to be honest |
04:35 |
hmmmm |
alright nevermind |
04:37 |
kahrl |
in any case you'd have to code this for ELF in all its variants, whatever the different BSDs use, MSVC, mingw |
04:37 |
hmmmm |
libexecinfo is portable |
04:37 |
hmmmm |
but nevermind that, seriously. i did not expect an executable with symbols to be 52mb |
04:38 |
kahrl |
whoa |
04:40 |
kahrl |
are the debug sections uncompressed? |
04:41 |
hmmmm |
looks like it |
04:41 |
hmmmm |
maybe that's just because of my old gcc |
04:56 |
|
neko259 joined #minetest-dev |
05:02 |
|
nalkri`` joined #minetest-dev |
05:16 |
|
mrtux joined #minetest-dev |
06:05 |
kahrl |
https://github.com/kahrl/minetest/compare/httpfetch |
06:06 |
kahrl |
I need to find a server with remote_media to test ^ |
06:11 |
|
darkrose joined #minetest-dev |
06:40 |
kaeza |
kahrl, does the server need the patch? |
06:41 |
kahrl |
kaeza, nope |
06:41 |
kaeza |
give me a second and I'll put up a temp server |
06:42 |
kahrl |
cool, thanks |
06:43 |
* VanessaE |
peeks in |
07:01 |
kaeza |
kahrl, 186.122.187.35:30000 |
07:10 |
kahrl |
I'm getting stuff like http://paste.dy.fi/Dn8 |
07:10 |
kahrl |
but only a few of the textures have these "ERROR" lines, most are fine |
07:12 |
kaeza |
hmm... |
07:12 |
kaeza |
I'm using HDX just for testing |
07:13 |
VanessaE |
ok, I have a remote media server set up as well, allegedly. |
07:14 |
kaeza |
could it be that HDX uses JPEG files instead of PNGs? |
07:14 |
VanessaE |
indeed it does, in places/ |
07:14 |
kaeza |
s/that/the fact that/ |
07:17 |
kahrl |
just got an error on http://digitalaudioconcepts.com/vanessa/hobbies/minetest/media_for_curl_server/cart.x |
07:17 |
VanessaE |
ah, I didn't think to copy the model files over |
07:17 |
VanessaE |
but at least that means the media server is working. |
07:18 |
kahrl |
ah |
07:19 |
kahrl |
it seems a bit slow, though faster than the builtin media fetching, and I never measured remote_media before |
07:21 |
kahrl |
I'll have to go to bed now, tomorrow I'll connect twice to VanessaE's server (with the old and the new remote_media fetcher) and compare the timings |
07:21 |
thexyz |
it's probably a good idea to have something like Django's collectstatic |
07:21 |
thexyz |
which'd copy all media files to some dir |
07:21 |
kahrl |
VanessaE: is that ok, bandwidth-wise? |
07:22 |
VanessaE |
sure |
07:22 |
VanessaE |
my site is unlimited anyway |
07:22 |
kaeza |
lol unlimited |
07:22 |
VanessaE |
"unlimited" * |
07:22 |
kaeza |
sorry, had to say it |
07:23 |
kaeza |
:P |
07:26 |
kahrl |
ah yeah, and it seems to be loading the missing files via the builtin method fine |
07:26 |
kahrl |
I wanted to test that too |
07:26 |
kahrl |
but ugh, so slow :P |
07:29 |
VanessaE |
there, the files at the media server should be complete now (I uploaded them in the wrong order, default stuff wasn't being overwritten by third-party mod stuff) |
07:29 |
VanessaE |
s/uploaded/copied/ |
07:30 |
VanessaE |
ogg, png, .x, what else? |
07:30 |
|
nore joined #minetest-dev |
07:30 |
nore |
hi |
07:31 |
VanessaE |
hi nore |
07:31 |
nore |
Did anyone ever notice my leaving message? |
07:31 |
VanessaE |
kahrl: what happens if the game server has a file that's different from what's on the media server? |
07:32 |
kahrl |
good question |
07:33 |
nore |
kahrl: were you able to fix the fact that some of the textures were .jpg? |
07:35 |
kahrl |
nore: what's that about? |
07:36 |
nore |
07:14 |
07:36 |
nore |
kaeza |
07:36 |
nore |
could it be that HDX uses JPEG files instead of PNGs? |
07:36 |
nore |
from the logs |
07:37 |
kahrl |
I haven't figured out why the remote_media on kaeza's server reported errors |
07:37 |
kahrl |
(everything looked fine in-game, though) |
07:39 |
nore |
From what you posted, it looks like it was that (because default_brick is a .jpg in HDX) |
07:40 |
nore |
Made API to access biomes, was in the TODO list http://pastebin.com/i9A7DT7w |
07:40 |
nore |
is it OK? |
07:40 |
kahrl |
so the file was missing from the remote_media server? |
07:41 |
nore |
kahrl: yes, it was default_brick.jpg and not default_brick.png |
07:41 |
kahrl |
well, he could have copied both :P |
07:41 |
nore |
you should ask him |
07:41 |
kahrl |
kaeza: ^? |
07:42 |
nore |
did you look at the biome access API? |
07:42 |
kahrl |
nore: that pastebin looks like what hmmmm had in mind |
07:42 |
kaeza |
no only .jpg |
07:43 |
nore |
Should I send a pull? |
07:43 |
VanessaE |
serving HDX via remote? insane :) |
07:43 |
kaeza |
it is running again with only PNG files |
07:43 |
kaeza |
VanessaE, 64px :P |
07:43 |
VanessaE |
still insane :P |
07:43 |
kahrl |
nore: I think you could just ping hmmmm when he's back |
07:44 |
nore |
kahrl: is there no doc about register_biome? |
07:44 |
kaeza |
anyway, the client only fetches what it needs |
07:44 |
nore |
I can't even test my code... |
07:44 |
kahrl |
looks like there isn't |
07:46 |
kahrl |
nore: have you tried these? http://pastebin.com/1kjU5tN7 |
07:47 |
nore |
the ID should in fact be the name, I will change that |
07:49 |
kahrl |
nore: tell hmmmm that if he's okay with your patch, then I'm okay with it too, since it seems simple enough |
07:49 |
kahrl |
anyway, off to bed |
07:51 |
nore |
done the change, here is the commit: https://github.com/Novatux/minetest/commit/c94f380e2d1a2af30baa653a01f3b04a2aa6a670 |
08:25 |
|
nore left #minetest-dev |
08:39 |
|
darkrose joined #minetest-dev |
08:39 |
|
darkrose joined #minetest-dev |
08:41 |
|
Calinou joined #minetest-dev |
08:43 |
|
RealBadAngel joined #minetest-dev |
09:05 |
|
jin_xi joined #minetest-dev |
09:50 |
|
Jordach joined #minetest-dev |
10:10 |
|
iqualfragile joined #minetest-dev |
10:53 |
|
kaeza joined #minetest-dev |
10:54 |
|
Calinou joined #minetest-dev |
11:00 |
|
proller joined #minetest-dev |
11:08 |
|
PilzAdam joined #minetest-dev |
11:17 |
kaeza |
not sure if this helps with anything, but got these errors while connecting to a remote_media-enabled server http://pastebin.com/BaUVrwE8 |
11:17 |
VanessaE |
the files are there, they have appropriate permissions, and they can be downloaded via a web browser |
11:20 |
VanessaE |
(the files are located at http://digitalaudioconcepts.com/vanessa/hobbies/minetest/media_for_curl_server ) |
11:28 |
|
BlockMen joined #minetest-dev |
12:04 |
|
iqualfragile_ joined #minetest-dev |
12:30 |
PilzAdam |
RealBadAngel, any news on that itemstack:get_metadata() thing? |
12:35 |
PilzAdam |
https://github.com/minetest/minetest/pull/851 seems good to me, any objections? |
12:42 |
PilzAdam |
updated lua-api.txt and squashed: https://github.com/PilzAdam/minetest/commit/8fe09af87ea9c902632e915d5c2f92295a24ad4a |
13:27 |
|
nore joined #minetest-dev |
13:27 |
nore |
What do you think of #860? |
13:27 |
|
Taoki[mobile] joined #minetest-dev |
13:53 |
|
iqualfragile joined #minetest-dev |
14:01 |
PilzAdam |
nore, I have tested it and it works |
14:01 |
nore |
karhl is ok to merge it |
14:01 |
PilzAdam |
but I wonder if any mods would like to know the biome ID |
14:01 |
nore |
see the logs |
14:01 |
nore |
the biome ID is its name |
14:01 |
PilzAdam |
kahrl is ok to merge it if hmmmm is |
14:02 |
PilzAdam |
nore, but what if I want to read it from a mod? |
14:02 |
nore |
yes, do you know when hmmm will be there? |
14:02 |
nore |
what do you mean, read it from a mod? |
14:03 |
PilzAdam |
a field "id" in the table |
14:03 |
nore |
it is field "name" in the biome definition |
14:03 |
nore |
minetest.registered_biomes[biome.name] = biome |
14:04 |
PilzAdam |
ah, I though the ID would be an int |
14:05 |
nore |
that what at made at the beginning, but then I thought it was good like that |
14:34 |
|
Calinou joined #minetest-dev |
14:43 |
celeron55 |
DSTACK() could be removed; it hasn't been in actual use in a very long time |
14:44 |
celeron55 |
it's a bit of an experiment that didn't prove to be useful |
14:44 |
celeron55 |
maybe i'll do that |
14:54 |
celeron55 |
328 deletions :---) i |
14:54 |
celeron55 |
+'ll test if this builds |
14:56 |
|
NakedFury joined #minetest-dev |
15:01 |
jin_xi |
soo, lets hope hmmmm reads the backlog |
15:01 |
celeron55 |
no it didn't build; there's even more to delete 8) |
15:02 |
jin_xi |
i have ported my turtle system to lua, using VoxelManip |
15:02 |
jin_xi |
some remarks: would be nice if points could be added to lua VoxelAreas, as for the c++ ones |
15:03 |
jin_xi |
then, if you fuck up your data in the slightest, the VoxelManip places fire |
15:03 |
jin_xi |
and lots of it... |
15:12 |
|
proller joined #minetest-dev |
15:26 |
celeron55 |
http://paste.dy.fi/DOr |
15:35 |
celeron55 |
^ that gets rid of the DebugStack stuff; 360 lines worth of it |
15:41 |
|
hmmmm joined #minetest-dev |
16:11 |
nore |
hmmmm, what do you think of #860? |
16:31 |
|
iqualfragile joined #minetest-dev |
16:51 |
nore |
anything about #861 too? |
17:03 |
hmmmm |
ah |
17:03 |
hmmmm |
860 is good |
17:03 |
hmmmm |
that's precisely what i was looking for somebody to do |
17:03 |
hmmmm |
i'm surprised people are using mapgen v7 though |
17:04 |
Calinou |
the same reason a few people use wayland or mir :P |
17:04 |
Exio4 |
systemd or upstart* |
17:05 |
Calinou |
no, these are production ready |
17:06 |
hmmmm |
but v7 is a bunch of crap |
17:06 |
hmmmm |
it might be better than v6 and it might be better than the older version, but it's still not exactly what i was looking for |
17:10 |
sokomine |
hi hmmmm. i'm still having some issues with mapgen (flat) and not generated areas while using place_schematic. sometimes, sand and dirt look a bit strange and appear on top of the houses, while two nodes at the floor are missing |
17:12 |
hmmmm |
perhaps try using mapgen v7 with the flat attribute |
17:12 |
sokomine |
later on, i want to spawn these villages on a "normal" map. for that to work, i need to get the area concerned flat. what's the best way to do it? voxelmanip? what i need is a flat area of 100-400m in an otherwise non-flat world |
17:12 |
hmmmm |
i suppose i could also add a v6 attribute to not flow mud |
17:13 |
sokomine |
ah, so it's flowing mud that's causing the problem? |
17:13 |
hmmmm |
obviously voxelmanip is the best way to do that |
17:14 |
jin_xi |
im really impressed with VoxelManip, its fast and cool. Apart from when you do something wrong and it places a huge chunk of fire |
17:14 |
sokomine |
sounds reasonable. need to change the area in some way anyway.....there have to be fields with corn around the city, and some more paths, and fences...a lot of work left :-/ |
17:14 |
sokomine |
easy solution: remove the fire-mod :) |
17:14 |
hmmmm |
jin_xi, i guess that's incentive for you to do it right |
17:15 |
Calinou |
we should count the number of times jin_xi complains about it :P |
17:15 |
|
kaeza joined #minetest-dev |
17:15 |
hmmmm |
well, it's not a bug or anything |
17:15 |
hmmmm |
it's just a sort of "oh, that's interesting" side effect when doing something wrong |
17:16 |
jin_xi |
well, it took me some tries to get it right, as i could not find a non mapgen example of how to use it right |
17:16 |
sokomine |
using voxelmanip on the area might get rid of the flowing mud automaticly i guess? so the only problem left then would be to blend the borders somehow to the rest of the terrain. but that's for the future... |
17:16 |
hmmmm |
? |
17:17 |
hmmmm |
the TNT mod is a good non-mapgen example |
17:17 |
jin_xi |
well, now i know |
17:20 |
jin_xi |
anyways, ported the turtle thing. but doing all the matrix math in lua is not so fast |
17:20 |
hmmmm |
that sucks, luajit is supposed to be great for computational kinds of things |
17:21 |
hmmmm |
we were lied to :/ |
17:21 |
jin_xi |
oh, not using it |
17:21 |
jin_xi |
should give that a try |
17:21 |
jin_xi |
also its a very naive implementation, but i cant do clever :( |
17:22 |
hmmmm |
yup, definitely use luajit, your minetest experience will improve a lot |
17:31 |
nore |
hmmmm, did you look at #861? |
17:37 |
ShadowNinja |
How about #862. |
17:40 |
nore |
good idea, but I don't know the code, so I can't tell you whether the coding is good or not |
17:49 |
nore |
sokomine: to blend the borders, the best would be to use the get_surface function, because it will be difficult without it |
17:51 |
sokomine |
sounds good. so there is a get_surface function now? |
17:55 |
nore |
yes, it is a pull request by sapier |
17:55 |
nore |
I don't think it has been merged now though |
17:56 |
sokomine |
then i hope that it gets merged |
17:59 |
|
sapier1 joined #minetest-dev |
18:01 |
nore |
sokomine: I will try something, using a custom mapgen... (single-layer, to test blending) |
18:01 |
nore |
you will have to blend the terrain then using that method |
18:05 |
|
neko259 joined #minetest-dev |
18:35 |
|
mrtux joined #minetest-dev |
18:52 |
|
neko259 joined #minetest-dev |
18:57 |
|
nalkri joined #minetest-dev |
19:04 |
|
PilzAdam joined #minetest-dev |
19:17 |
|
Yepoleb joined #minetest-dev |
19:28 |
VanessaE |
I've filed an issue for the *.x thing kaeza was having: https://github.com/minetest/minetest/issues/863 |
19:30 |
VanessaE |
bbiab |
19:50 |
ShadowNinja |
Needs updating: https://forum.minetest.net/viewtopic.php?id=1378 |
19:55 |
|
Miner_48er joined #minetest-dev |
20:06 |
|
BrandonReese joined #minetest-dev |
20:10 |
|
BrandonReese joined #minetest-dev |
20:12 |
|
BrandonReese joined #minetest-dev |
20:34 |
|
Miner_48er joined #minetest-dev |
20:47 |
|
loggingbot_ joined #minetest-dev |
20:47 |
|
Topic for #minetest-dev is now Minetest core development and maintenance. Chit-chat goes to #minetest. Consider this instead of /msg celeron55. http://irc.minetest.ru/minetest-dev/ http://dev.minetest.net/ |
21:06 |
|
PilzAdam joined #minetest-dev |
21:14 |
celeron55 |
http://paste.dy.fi/DOr <- DebugStack removal; any comments? |
21:14 |
|
proller joined #minetest-dev |
21:18 |
PilzAdam |
celeron55, that patch does not apply correctly |
21:21 |
celeron55 |
oh... apparently i made it on top of version bc5db9b02 |
21:22 |
celeron55 |
well whatever; no hurry for that |
21:22 |
kahrl |
I think it should still at least show which threads are running when an assert fails |
21:22 |
kahrl |
e.g. useful for stuff like the MeshUpdateThread assertion failure |
21:22 |
celeron55 |
feel free to iterate on that 8) |
21:26 |
kahrl |
whoa, is there no mutex for log_threadnames? |
21:29 |
PilzAdam |
celeron55, here is it rebased: https://github.com/PilzAdam/minetest/commit/5e6f614349488c3c1dc0b98b08055a52e8d75780 |
21:29 |
celeron55 |
kahrl: apparently no :-) |
21:30 |
celeron55 |
it's probably a considerable problem nowadays as there are more threads |
21:30 |
celeron55 |
(started and stopped all the time) |
21:31 |
kahrl |
although just adding a mutex to it might cause enough slow down to be a bad idea |
21:31 |
kahrl |
maybe use TLS? |
21:31 |
sapier1 |
tls? |
21:31 |
kahrl |
thread local storage |
21:32 |
sapier1 |
oh |
21:32 |
kahrl |
so that the logger can get the current thread name without having to lock a mutex |
21:32 |
celeron55 |
do we have a portable implementation for that |
21:32 |
celeron55 |
if so, go ahead |
21:32 |
kahrl |
I have one that works with win32 and pthreads, it's in my jthread-1.3.1 patch |
21:32 |
sapier1 |
hmm at least within linux each thread has its own threadname and can access it's name but not sure if this is true for windows too |
21:33 |
kahrl |
(well, I only tested it with pthreads) |
21:33 |
sapier1 |
yea I meant phtreads features |
21:34 |
celeron55 |
sapier1: i think it's better not use such names directly for logging so that the stuff that goes to log can be logging-optimized |
21:34 |
sapier1 |
I see |
21:34 |
celeron55 |
that is, user readable stuff and not some ids or long crap |
21:35 |
sapier1 |
doing a syscall for each log entry obviously is a bad idea |
21:35 |
sapier1 |
does jthread use pthreads threadname feature? |
21:36 |
kahrl |
sapier1: I don't think so |
21:37 |
sapier1 |
I missed decisio about forking/integrating jthread what has been done? |
21:37 |
kahrl |
sapier1: nothing ;) |
21:37 |
sapier1 |
ok :-) |
21:40 |
kahrl |
https://github.com/kahrl/minetest/commit/681d6df31b672a92cd3e1618668279ad93a6cddc#L6R276 <-- very simplistic ThreadLocal class |
21:40 |
sapier1 |
what about my fixes 774 640 and 418? ... haven't asked for days now ;-) |
21:42 |
sapier1 |
another question why don't we just use threadid for logging? |
21:43 |
kahrl |
because nobody knows what thread has what id? |
21:43 |
sapier1 |
isn't tls a little bit overkill for storing "some thread type 1" to "some thread type n"? |
21:43 |
sapier1 |
e.g. "<Threadtyp>:tid" |
21:44 |
sapier1 |
threadtype beeing mapgen thread1 |
21:44 |
sapier1 |
-1 |
21:44 |
kahrl |
and how/where would you store type and tid? |
21:44 |
sapier1 |
ok ok tid would have to be read each time again ... not best idea ... threadtyp is a fixed value that can be compiled in |
21:45 |
sapier1 |
a mapgen thread will always be of mapgen type |
21:45 |
kahrl |
erm |
21:45 |
kahrl |
it's not fixed inside log.cpp |
21:45 |
kahrl |
unless you mean to add <<"[EmergeThread"<<tid<<"]" to each *stream write |
21:46 |
sapier1 |
functions usually get parameters ;-) e.g. log(facility,level,text,parameters) |
21:46 |
sapier1 |
ok ok c++ style |
21:46 |
kahrl |
that would be really verbose and you'd have to rely on the programmer to get the thread name right |
21:46 |
sapier1 |
btw how is this stream locked against concurrency? |
21:47 |
kahrl |
e.g. what if you want to log an error in some utility function, how'd you know the thread name? |
21:47 |
sapier1 |
usually I use file defines to get this done |
21:48 |
kahrl |
file defines? |
21:48 |
sapier1 |
e.g. in a cpp file first add a #define LOG_FILENAME "some filename" |
21:48 |
kahrl |
what does that have to do with the thread name? |
21:49 |
sapier1 |
nothing if you don't have a single cpp file for a single component |
21:49 |
sapier1 |
but if you have component specific files you can use this do specify origin of a log message |
21:51 |
kahrl |
I see no practical way to do this for e.g. the logging in tile.cpp:512 |
21:52 |
sapier1 |
I don't see any use for threadname/id in this logging message either? |
21:53 |
sapier1 |
what do you expect thread information might be usefull in cases like that? |
21:54 |
kahrl |
try fixing issue #831 |
21:55 |
sapier1 |
first compile -O0 |
21:55 |
sapier1 |
that stack is useless |
21:56 |
kahrl |
no but I mean, why do you propose removing information from the log |
21:56 |
kahrl |
when we absolutely don't have to |
21:56 |
sapier1 |
I didn't know it's there ... but I don't know many cases where thread name is more usefull than thread id |
21:57 |
sapier1 |
actually a threadid is even better as it's REAL and not what someone tried to save |
22:00 |
sapier1 |
btw 831 seems to be a missing lock around result_queue |
22:01 |
kahrl |
huh, why do you think it needs to be locked |
22:01 |
kahrl |
it's local |
22:01 |
sapier1 |
you push a stack variable to some other thread to do something with it |
22:02 |
sapier1 |
am I wrong? |
22:03 |
kahrl |
yup and ResultQueue is a MutexedQueue so that should be safe |
22:04 |
sapier1 |
mutex queue? |
22:05 |
sapier1 |
using a mutex where a semaphore would be native type ... grrrr |
22:07 |
* VanessaE |
peeks in |
22:08 |
sapier1 |
I hope noone ever uses pop_back and pop_front same time ... this is not threadsafe |
22:09 |
sapier1 |
who wrote this thing? it's safe for some usecases but no documentation about other things that never may happen to it? |
22:11 |
kahrl |
how is it not threadsafe |
22:11 |
sapier1 |
call pop_front and pop_back same time |
22:11 |
kahrl |
and then what? |
22:11 |
sapier1 |
from different threads |
22:12 |
sapier1 |
wait_time_ms calculation is not protected by mutex |
22:12 |
kahrl |
wat |
22:12 |
sapier1 |
getList is available public so list is passed outside where it's not protected by mutex |
22:13 |
sapier1 |
wait_time_ms += 10; is not within autolock scope so it's calculation can be done while the second thread does it's comparison |
22:13 |
kahrl |
wat |
22:14 |
sapier1 |
container.h line 284/310 wait_time_ms +10; |
22:14 |
kahrl |
u32 wait_time_ms = 0; |
22:14 |
sapier1 |
too same issue |
22:15 |
sapier1 |
line 321 passes reference to list to anyone |
22:15 |
kahrl |
or written out: auto u32 wait_time_ms = 0; |
22:15 |
kahrl |
_auto_ |
22:15 |
sapier1 |
ohh .. local variable sorry |
22:16 |
sapier1 |
but 321 is still critical |
22:17 |
kahrl |
getList and getMutex are only there so that RequestQueue etc. can do their thing |
22:18 |
sapier1 |
yes but obviously they're opening a critical internal element for abuse |
22:18 |
kahrl |
it's not abused anywhere |
22:19 |
sapier1 |
"it's not abused right now" is not an excuse for unsafe design ;-) |
22:20 |
kahrl |
this is not locked down enterprise java, this is C++ |
22:20 |
sapier1 |
believe me if you don't prevent bad things from happening they will happen |
22:21 |
sapier1 |
not now, not tomorrow ... but some day later for sure ... if there is a way to change this in a safe way it should be done |
22:24 |
kahrl |
well to me it's not a problem, and I tend to look at code that goes upstream that does thread related things, if there was a getList or getMutex in there that'd raise some flags |
22:24 |
sapier1 |
you'll miss something sometime for sure ;-) |
22:24 |
kahrl |
if you can convince some other devs to change it so that these are private and RequestQueue is a friend class, fine by me |
22:26 |
sapier1 |
what happens if a request is updated while it's beeing processed? |
22:31 |
kahrl |
I don't think a request can be updated in that state |
22:31 |
kahrl |
as it's just a local variable in processQueue() |
22:31 |
kahrl |
however, what might happen is that a request is replaced before processQueue finds it |
22:32 |
sapier1 |
or exactly while pop is copying it's data? |
22:32 |
kahrl |
but there would have to be at least two non-main threads trying to call getTextureId concurrently, and afaik there's only the mesh update thread that calls it |
22:33 |
sapier1 |
ok that can't happen due to result queue taking requests mutex |
22:34 |
sapier1 |
I suggest trying to find a way to reproduce and debugging live with an -O0 build |
22:35 |
kahrl |
that would mean I can reproduce the bug first, but I can't |
22:35 |
kahrl |
(I haven't really tried) |
22:43 |
sapier1 |
I suggest replacing the assert by printing the content of result.key first |
22:44 |
sapier1 |
maybe something is put to wrong result queue ... but I don't see how this should happen rightnow |
22:46 |
sapier1 |
and of course enable the infomessage at 784 |
22:46 |
|
sapier1 left #minetest-dev |
22:58 |
|
pitriss joined #minetest-dev |
23:00 |
pitriss |
Please can any MT dev look here https://forum.minetest.net/viewtopic.php?id=6197&p=12 ? lot of landrush players can't log in with strange message.. Can it be solved by clients? or it fix for serverside is needed? |
23:01 |
kaeza |
same happened to me with VanessaE's server earlier today |
23:01 |
PilzAdam |
^ celeron55, might be related to your DoS commit |
23:03 |
VanessaE |
kaeza: https://github.com/minetest/minetest/issues/863 |
23:20 |
|
AllegedlyDead joined #minetest-dev |
23:36 |
|
jin_xi joined #minetest-dev |