| Time |
Nick |
Message |
| 00:01 |
|
luizrpgluiz joined #minetest-dev |
| 00:01 |
|
luizrpgluiz left #minetest-dev |
| 00:05 |
Zeno` |
I'm more worried about 1527 heh |
| 00:07 |
RealBadAngel |
i can see lighting code there |
| 00:08 |
Zeno` |
doesn't seem to be related though. I'll remove that line and re-test |
| 00:08 |
RealBadAngel |
please do |
| 00:08 |
VanessaE |
RealBadAngel: I surmised this is why nore had to insert that extra check in technic to limit how many VM's it calls into use (a good check anyways since it reduces redundancy) |
| 00:08 |
VanessaE |
it was also causing a massive memory leak |
| 00:08 |
RealBadAngel |
it often happens that bug resides not where you suppose to |
| 00:10 |
Zeno` |
well, the problem is still there: http://codepad.org/tbgNIQ83 |
| 00:19 |
RealBadAngel |
local vm, emin, emax = minetest.get_mapgen_object("voxelmanip") |
| 00:19 |
RealBadAngel |
how this line is supposed to work? |
| 00:19 |
RealBadAngel |
set 3 variables to one value? |
| 00:20 |
RealBadAngel |
or just the 3rd? |
| 00:21 |
Zeno` |
https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L116 |
| 00:21 |
Zeno` |
all three are pushed onto the Lua stack |
| 00:23 |
RealBadAngel |
i dont know who invented that, but for me that looks like not stack but pile of shit |
| 00:23 |
Zeno` |
lol |
| 00:24 |
Zeno` |
yeah well, that may be so |
| 00:24 |
RealBadAngel |
that is against any coding logic |
| 00:24 |
Zeno` |
it's convenient... apparently |
| 00:24 |
Zeno` |
I don't like it myself, but *shrug* |
| 00:24 |
RealBadAngel |
i do read that as def of three variables and 3rd is defined |
| 00:25 |
RealBadAngel |
i mean set |
| 00:25 |
Zeno` |
that's how I would (normally) read it as well (i.e. in a C-like way) |
| 00:25 |
RealBadAngel |
1st and 2nd are just set local |
| 00:25 |
Zeno` |
or, heck, even pascal or modula-2 or normal basic |
| 00:25 |
Zeno` |
but that's Lua for ya |
| 00:25 |
Zeno` |
I think python does something similar but I've never used python |
| 00:26 |
RealBadAngel |
can you do some tests for me while youre at it? |
| 00:26 |
Zeno` |
sure |
| 00:26 |
RealBadAngel |
add there before another variable |
| 00:26 |
RealBadAngel |
and dump them all on call |
| 00:27 |
Zeno` |
before vm? |
| 00:27 |
RealBadAngel |
yes |
| 00:27 |
Zeno` |
like: local test, vm, emin, emax = minetest.get_mapgen_object("voxelmanip") ?? |
| 00:27 |
RealBadAngel |
yup, and try later on move test in between |
| 00:29 |
Zeno` |
ok, it doesn't work when at the start: 10:28:48: ERROR[main]: ERROR: An unhandled exception occurred: ...est/worlds/ram-test/worldmods/voxelleaktest/init.lua:6: attempt to call method 'get_data' (a nil value) |
| 00:31 |
Zeno` |
causes an unhandled exception with a extra at the beginning or the end |
| 00:31 |
Zeno` |
actually, anywhere |
| 00:33 |
Zeno` |
they're being assigned correctly (the return values) |
| 00:34 |
RealBadAngel |
if so, then ok |
| 00:34 |
RealBadAngel |
but anyway i still call it ugly as hell |
| 00:35 |
Zeno` |
and I would agree :) But, alas, it's not the issue :( |
| 00:35 |
RealBadAngel |
who knows if its using stacks |
| 00:36 |
RealBadAngel |
one byte not taken out while pushed is a diseaster |
| 00:37 |
RealBadAngel |
well, its pretending to be a stack |
| 00:37 |
RealBadAngel |
so propably the issue is somwhere else |
| 00:41 |
Zeno` |
the issue, I suspect, is the Lua garbage collector and https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L105 |
| 00:41 |
Zeno` |
never being deleted properly |
| 00:42 |
RealBadAngel |
*(void **)(lua_newuserdata(L, sizeof(void *))) = o; |
| 00:42 |
RealBadAngel |
love such code :) |
| 00:43 |
RealBadAngel |
you have to be twisted to get it and double twisted to write it |
| 00:44 |
Zeno` |
although it *appears to be* here: https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_vmanip.cpp#L34 |
| 00:45 |
Zeno` |
if (!o->is_mapgen_vm) is dodgy as crap though (any random object has a 50/50 chance of that being true due to the cast) |
| 00:45 |
Zeno` |
if !? |
| 00:45 |
Zeno` |
if not? |
| 00:45 |
Zeno` |
err... does that seem right? |
| 00:45 |
* Zeno` |
is confused now |
| 00:46 |
Zeno` |
why would you delete it only if it's not NULL ? |
| 00:46 |
Zeno` |
not true, rather |
| 00:46 |
* VanessaE |
pokes hmmmm |
| 00:48 |
RealBadAngel |
if !thing |
| 00:48 |
RealBadAngel |
means if thing doesnt exists |
| 00:49 |
RealBadAngel |
ie is NULL |
| 00:49 |
RealBadAngel |
!NULL = true |
| 00:50 |
Zeno` |
yeah, but it's a bool |
| 00:50 |
Zeno` |
(I made a mistake when I said !NULL... !NULL would be correct( |
| 00:53 |
hmmmm |
Zeno`: indeed, if (!o->is_mapgen_vm) is a typo |
| 00:53 |
hmmmm |
that would definitely cause a memory leak. |
| 00:53 |
hmmmm |
the casts are valid though |
| 00:53 |
Zeno` |
changing it to if (o->is_mapgen_vm) {} causes segfault though heheh |
| 00:54 |
hmmmm |
what's the cause of the segfault then? |
| 00:54 |
|
nore_ joined #minetest-dev |
| 00:56 |
RealBadAngel |
its a bool or a pointer then? |
| 00:56 |
Zeno` |
double free (delete) I think... I'm still trying to get my head around what's going on |
| 00:56 |
hmmmm |
OH |
| 00:56 |
Zeno` |
it's a bool |
| 00:56 |
hmmmm |
derp |
| 00:56 |
RealBadAngel |
it negates o ? |
| 00:56 |
hmmmm |
no |
| 00:56 |
hmmmm |
shoot |
| 00:56 |
Zeno` |
https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_vmanip.h#L38 |
| 00:56 |
hmmmm |
I am confused now |
| 00:56 |
hmmmm |
nevermind, that's not a typo |
| 00:57 |
hmmmm |
if it's a mapgen vm, we don't want to delete that object because it's owned by the mapgens, NOT! lua |
| 00:57 |
hmmmm |
LuaVoxelManip::is_mapgen_vm is a bool |
| 00:57 |
Zeno` |
although valgrind points to https://github.com/minetest/minetest/blob/master/src/script/lua_api/l_mapgen.cpp#L105 |
| 00:57 |
Zeno` |
as being the source of the leaked mem |
| 00:57 |
hmmmm |
we only want to delete VoxelManipulator objects that were allocated, not grabbed as an existing mapgen object |
| 00:58 |
Zeno` |
Where is that memory deleted, is the question I guess |
| 00:58 |
hmmmm |
okay |
| 00:59 |
hmmmm |
that memory is deleted in Map::finishBlockMake() |
| 00:59 |
Zeno` |
hmm ok |
| 01:00 |
hmmmm |
ahhhh |
| 01:00 |
Zeno` |
must be something else then. sorry for the red herring |
| 01:00 |
hmmmm |
I see the problem |
| 01:00 |
hmmmm |
Map::finishBlockMake deletes the underlying VoxelManipulator object |
| 01:00 |
hmmmm |
regardless of creation method, the LuaVoxelManip must be deleted |
| 01:01 |
hmmmm |
so it should be actually like |
| 01:01 |
hmmmm |
if (!o->is_mapgen_vm) delete o->vm; |
| 01:01 |
hmmmm |
delete o; |
| 01:01 |
hmmmm |
sorry |
| 01:02 |
VanessaE |
hmmmm: would this be the same thing that caused that big memory leak we had to patch over in technic? |
| 01:02 |
VanessaE |
(using non-mapgen vm's to force-load blocks) |
| 01:02 |
hmmmm |
i really don't know anything about that |
| 01:03 |
VanessaE |
hmmmm: this, except line 83 didn't exist before the patch. https://github.com/minetest-technic/technic/blob/master/technic/machines/switching_station.lua#L82 |
| 01:03 |
hmmmm |
if it's a mapgen vm you're getting, then o would never be deleted. if it's your own vm you're creating, the LuaVoxelManip would be deleted but the VoxelManipulator would be leaked |
| 01:03 |
hmmmm |
so in either case something is leaking |
| 01:04 |
Zeno` |
yes, that seems to be the case, but I'm new to the code base so I'm battling to understand it :) |
| 01:05 |
hmmmm |
alright i'll give an overview of what's happening |
| 01:05 |
Zeno` |
you have the test script I am using? |
| 01:05 |
Zeno` |
test Lua* |
| 01:05 |
hmmmm |
no. |
| 01:05 |
Zeno` |
https://github.com/minetest/minetest/issues/1527 |
| 01:05 |
hmmmm |
you can either create a new LuaVoxelManip object in order to do your own loading and whatever |
| 01:06 |
Zeno` |
you can delete the set_lighting() and calc_lighting()... it's not related |
| 01:06 |
hmmmm |
or you can grab the existing mapgen's voxel manipulator in order to make any changes to the terrain that has been generated |
| 01:06 |
hmmmm |
the existing mapgen's voxelmanip is created in initBlockMake, and deleted in finishBlockMake |
| 01:06 |
hmmmm |
as a result, the lua GC can't delete that VoxelManipulator |
| 01:07 |
Zeno` |
yep |
| 01:07 |
hmmmm |
therefore any LuaVoxelManips created as a wrapper around existing VoxelManipulators must ignore deletion of that VM |
| 01:07 |
hmmmm |
but the GC must still delete the LuaVoxelManip wrapper object itself |
| 01:07 |
Zeno` |
Ok, I understand the use of the bool now |
| 01:08 |
hmmmm |
as for user-allocated voxel manipulators (e.g. used outside of on_generated), the VoxelManipulator is allocated within that same wrapper and thus must be destroyed within the GC |
| 01:08 |
hmmmm |
so, indeed, changing that line to "if (!o->is_mapgen_vm) delete o->vm; delete o;" would fix this issue |
| 01:08 |
Zeno` |
so line 35 should be delete o->vm and then outside of the condition delete o ? |
| 01:08 |
Zeno` |
cool |
| 01:09 |
Zeno` |
testing |
| 01:09 |
hmmmm |
but |
| 01:09 |
hmmmm |
there's more to it |
| 01:09 |
Zeno` |
oh oh |
| 01:09 |
hmmmm |
look at LuaVoxelManip::~LuaVoxelManip() |
| 01:09 |
Zeno` |
ugh |
| 01:09 |
hmmmm |
so you can see the destruction logic was poorly thought out at the time |
| 01:10 |
hmmmm |
the best way to fix this would be to change l_vmanip.cpp:322 to if (is_mapgen_vm) delete vm; |
| 01:10 |
hmmmm |
and then remove the conditional on line 34 entirely |
| 01:11 |
hmmmm |
to the best of my knowledge, without testing anything at all, this should solve the issue |
| 01:11 |
hmmmm |
i can't be arsed to open up my IDE |
| 01:20 |
Zeno` |
testing |
| 01:22 |
Zeno` |
so, LuaVoxelManip::gc_object() should simply delete o; ? |
| 01:22 |
Zeno` |
after the cast of course |
| 01:25 |
Zeno` |
ok, that resolves the issue |
| 01:27 |
Zeno` |
I've updated the issue with the changes: https://github.com/minetest/minetest/issues/1527 |
| 01:27 |
Zeno` |
since I can't push |
| 01:29 |
Zeno` |
thanks hmmmm |
| 01:29 |
Zeno` |
thanks also for the explanation... apart from the bug fix I understand those source files a bit better now :) |
| 01:37 |
|
kaeza joined #minetest-dev |
| 02:20 |
Zeno` |
hmmmm, https://github.com/minetest/minetest/pull/1529 |
| 02:22 |
|
SoniEx2 joined #minetest-dev |
| 02:37 |
hmmmm |
yep looks good zeno` |
| 02:37 |
hmmmm |
oh wait a minute, you don't have committer privileges |
| 02:38 |
Zeno` |
no, you have to do it :) |
| 03:06 |
|
smoke_fumus joined #minetest-dev |
| 03:16 |
|
cheapie joined #minetest-dev |
| 04:04 |
|
NakedFury joined #minetest-dev |
| 04:05 |
|
Eater4 joined #minetest-dev |
| 04:28 |
|
cheapie joined #minetest-dev |
| 05:41 |
|
cheapie joined #minetest-dev |
| 05:45 |
|
Hunterz joined #minetest-dev |
| 05:47 |
|
grrk-bzzt joined #minetest-dev |
| 05:54 |
|
Miner_48er joined #minetest-dev |
| 06:11 |
|
kaeza joined #minetest-dev |
| 06:44 |
|
Krock joined #minetest-dev |
| 06:51 |
|
Kalista joined #minetest-dev |
| 06:56 |
|
proller joined #minetest-dev |
| 08:26 |
|
Taoki[mobile] joined #minetest-dev |
| 08:43 |
|
Kalista joined #minetest-dev |
| 09:22 |
|
Calinou joined #minetest-dev |
| 09:41 |
|
harrison_ joined #minetest-dev |
| 09:45 |
|
SmugLeaf joined #minetest-dev |
| 09:45 |
|
SmugLeaf joined #minetest-dev |
| 09:45 |
|
blaise joined #minetest-dev |
| 09:46 |
|
restcoser joined #minetest-dev |
| 10:18 |
|
PenguinDad joined #minetest-dev |
| 10:28 |
|
smoke_fumus joined #minetest-dev |
| 10:37 |
|
proller joined #minetest-dev |
| 10:50 |
|
proller joined #minetest-dev |
| 11:03 |
|
kahrl joined #minetest-dev |
| 11:03 |
kahrl |
RealBadAngel: wtf is wrong with multiple return values? |
| 11:04 |
kahrl |
it's a completely documented feature of lua and a lot of other languages |
| 11:04 |
kahrl |
even string.find uses it |
| 11:04 |
RealBadAngel |
kahrl, well, nothing in fact. i just dont like how it looks |
| 11:04 |
RealBadAngel |
if it works, then ok |
| 11:04 |
kahrl |
ah, ok |
| 11:15 |
|
ImQ009 joined #minetest-dev |
| 11:22 |
|
CheapSeth joined #minetest-dev |
| 11:22 |
|
CheapSeth left #minetest-dev |
| 11:22 |
Zeno` |
for a language like Lua, multiple return values /is/ the neatest solution |
| 11:23 |
Zeno` |
the problem is that local a, b, c = func() *looks* like c is assigned and not a and b |
| 11:24 |
Zeno` |
the only other solution would be to pass a table to the func, kind of like passing a struct in C, but due to Lua's stack mechanism that wouldn't be very efficient |
| 11:25 |
Zeno` |
personally I'd like to see some kind of, I dunno, wrapping. Maybe (a, b, c) = func() |
| 11:25 |
Zeno` |
but *shrug* |
| 11:25 |
Zeno` |
it's not going to change heh |
| 11:25 |
|
OldCoder joined #minetest-dev |
| 11:25 |
Zefram_Fysh |
returning a table is a nice solution when performance isn't tight |
| 11:26 |
Zefram_Fysh |
parenthesised var list would have been a better syntax, but too late now. you've just got to rely on people knowing the language |
| 11:26 |
Zeno` |
Zefram_Fysh, yes, or returning a table (same as passing a table, the performance is the same... not like C where you can pass a pointer to a struct) |
| 11:26 |
Zeno` |
yep, that's what I mean by "it's not gonna change" |
| 11:26 |
Zeno` |
because it's too late :) |
| 11:29 |
Zefram_Fysh |
not too late if you're willing to implement your preferred language in lua. you can implement it to compile to lua code, which you evaluate with loadstring(). with LuaJIT, this will even have non-horrendous performance |
| 11:30 |
Zeno` |
people are used to it now though |
| 11:31 |
Zeno` |
and I don't really care enough to fork Lua heh |
| 11:31 |
Zeno` |
it's not so bad |
| 11:32 |
Zefram_Fysh |
if I were implementing a better language, it wouldn't be just a slightly-modified lua |
| 11:40 |
Zeno` |
lua is pretty nice actually |
| 11:40 |
Zeno` |
I hated it at first |
| 11:40 |
Zefram_Fysh |
it's inoffensive |
| 11:41 |
Zeno` |
I began to like it much more when I actually started writing C "extensions" heh |
| 11:41 |
Zeno` |
the design it not without flaws, but it's pretty darn good |
| 11:42 |
Zeno` |
I couldn't do better |
| 11:42 |
Zeno` |
well, maybe I could but only in hindsight |
| 11:47 |
Zefram_Fysh |
the 1-offset for numeric indices is obnoxious |
| 11:47 |
Zefram_Fysh |
so is the implicit semicolon |
| 11:48 |
Zefram_Fysh |
and the implicit creation of variables on typo |
| 11:48 |
Zefram_Fysh |
there are many details that are more or less annoying |
| 11:48 |
Zefram_Fysh |
but it gets the big things generally right |
| 11:49 |
Zeno` |
not related to the language but the common practice of always putting a , after table entries annoys me for some reason |
| 11:49 |
Zefram_Fysh |
it's nice to have lambda expressions just work. you wouldn't have got that in a corresponding language from 20 years ago |
| 11:49 |
Zeno` |
lots of languages create implicit vars from typos |
| 11:50 |
Zeno` |
well, most "basic like" ones |
| 11:50 |
Zefram_Fysh |
putting a comma after the last entry of a table means that adding another entry after it doesn't edit that line, which makes editing easier and the diff less cluttered. that the language permits it is a desirable feature |
| 11:51 |
Zeno` |
Yes, it seems to be desirable to most people. I don't like it though :) |
| 11:51 |
Zeno` |
just one of those things I guess |
| 11:51 |
Zefram_Fysh |
that many other languages make the same mistake doesn't make it less problematic, nor in this case is it an excuse. the problems with it are well known, as is the solution |
| 11:51 |
Zeno` |
I don't like introducing compound statements in C when they're not necessary either heh |
| 11:52 |
Zeno` |
which annoys many other people immensely |
| 11:53 |
Zeno` |
I do for () for () for () {} and people hate me :( |
| 12:31 |
|
Jordach joined #minetest-dev |
| 13:17 |
|
Calinou joined #minetest-dev |
| 13:29 |
|
jp__ joined #minetest-dev |
| 13:31 |
|
jp__ left #minetest-dev |
| 13:43 |
|
hmmmm joined #minetest-dev |
| 13:46 |
|
Eater4 joined #minetest-dev |
| 13:53 |
|
Megaf joined #minetest-dev |
| 13:55 |
|
AnotherBrick joined #minetest-dev |
| 14:08 |
|
NakedFury joined #minetest-dev |
| 14:08 |
|
Exio joined #minetest-dev |
| 14:29 |
|
harrison joined #minetest-dev |
| 14:30 |
|
sfan5 joined #minetest-dev |
| 14:57 |
|
swaaws joined #minetest-dev |
| 15:06 |
|
werwerwer joined #minetest-dev |
| 15:50 |
|
kahrl_ joined #minetest-dev |
| 15:57 |
|
Kalista joined #minetest-dev |
| 16:04 |
|
Hunterz joined #minetest-dev |
| 16:08 |
|
AnotherBrick joined #minetest-dev |
| 16:19 |
|
AnotherBrick joined #minetest-dev |
| 16:37 |
|
zat joined #minetest-dev |
| 16:47 |
|
Calinou joined #minetest-dev |
| 16:51 |
|
SoniEx2 joined #minetest-dev |
| 17:42 |
|
ImQ009 joined #minetest-dev |
| 18:02 |
|
khonkhortisan joined #minetest-dev |
| 18:07 |
|
casimir joined #minetest-dev |
| 18:15 |
|
EvergreenTree joined #minetest-dev |
| 18:29 |
|
cheapie joined #minetest-dev |
| 18:44 |
Krock |
hmmmm, I'm almost sure, it has been asked before, but I can't find anything about this. Why does minetest call the function register_on_generated sometimes twice? |
| 18:46 |
|
proller joined #minetest-dev |
| 18:54 |
|
SmugLeaf joined #minetest-dev |
| 18:54 |
|
SmugLeaf joined #minetest-dev |
| 19:05 |
|
sapier joined #minetest-dev |
| 19:28 |
|
grrk-bzzt joined #minetest-dev |
| 19:28 |
|
grrk-bzzt joined #minetest-dev |
| 19:35 |
hmmmm |
minetest doesn't call that function, various games do |
| 19:49 |
|
vifino joined #minetest-dev |
| 21:15 |
|
Megaf_ joined #minetest-dev |
| 21:29 |
|
casimir joined #minetest-dev |
| 21:41 |
|
Jordach joined #minetest-dev |
| 21:51 |
|
smoke_fumus joined #minetest-dev |
| 22:37 |
|
blaise joined #minetest-dev |
| 23:21 |
|
nore_ joined #minetest-dev |
| 23:34 |
|
^v joined #minetest-dev |
| 23:46 |
|
^v joined #minetest-dev |
| 23:51 |
|
kaeza joined #minetest-dev |
| 23:52 |
|
diemartin joined #minetest-dev |
| 23:59 |
|
Miner_48er joined #minetest-dev |
