Time Nick Message 09:54 sfan5 rubenwardy: do you think it would be useful for lua objects to have these infos? https://github.com/minetest/minetest/blob/master/src/collision.h#L44-L54 09:54 sfan5 I'm looking into giving Lua objects the collision results 09:54 sfan5 and i'd definitely expose the struct collisionMoveResult, not sure how useful the rest is however 09:55 rubenwardy makes sense 09:55 rubenwardy is that server side? 09:55 sfan5 yes 09:55 rubenwardy oh nice 10:50 sfan5 ok so I wrote the code to push it to lua and it ... just doesn't work 10:50 sfan5 the last argument to the function call always ends up being nil 10:51 sfan5 if I swap the second and third argument, then the collision stuff gets through fine (as 2nd arg) but the last argument is still nil 10:58 sfan5 yeah nope I don't get it 11:00 sfan5 that you can't pass more than 2 arguments to on_step seems to be a fundamental law of the universe 11:05 sfan5 minetest_game is breaking this 11:05 sfan5 for fucks sake 12:11 nerzhul hello, just published #9665 bugfix. I found that in my hardened setup with a pg backend :) 12:11 ShadowBot https://github.com/minetest/minetest/issues/9665 -- Verify database connection on interval by nerzhul 15:55 nerzhul sfan5 do you have some time for #9665 please ? :) 15:55 ShadowBot https://github.com/minetest/minetest/issues/9665 -- Verify database connection on interval by nerzhul 15:55 sfan5 yes I planned to look at it next 16:39 nerzhul perfect, thanks 17:08 sfan5 merging game#2647 17:08 ShadowBot https://github.com/minetest/minetest_game/issues/2647 -- Make default:snow collisionbox half of nodebox height by paramat 18:12 nerzhul sfan5: approved #9669 18:12 ShadowBot https://github.com/minetest/minetest/issues/9669 -- scriptapi: Some small optimizations to value pushing by sfan5 18:13 nerzhul sfan5: i fixed #9665 regarding your comments, and waiting for more inputs or approval on #9647 please :) 18:13 ShadowBot https://github.com/minetest/minetest/issues/9665 -- Verify database connection on interval by nerzhul 18:13 ShadowBot https://github.com/minetest/minetest/issues/9647 -- Add WorldSettings object & env var support by nerzhul 18:21 nerzhul i'm looking for modkit api and some API are mob high level and some seems to be lack on our API, builtin included i think 18:21 nerzhul i saw many mods reimplementing some helpers in the same way it's not good 18:26 Krock - dst.push_back(pg_to_v3s16(results, 0, 0)); 18:26 Krock + dst.push_back(pg_to_v3s16(results, row, 0)); 18:27 Krock will push that fix in 15 minutes. dunno whether the migration works afterwards, but it's surely a step into the right direction 18:27 Krock unless somebody here has a pgsql build setup plus world to test it 18:31 nerzhul i have it 18:32 nerzhul woot 18:32 nerzhul this is a very old code no ? 18:33 Krock 2017-04-23 14:35:08 +0200 303) for (int row = 0; row < numrows; ++row) 18:33 Krock yes 18:33 nerzhul go ahead 18:34 nerzhul you are right haha 18:34 nerzhul i'm looking for the impact 18:34 nerzhul it seems it's a problem on database migration & clearObject commands only 18:34 nerzhul impact is hopefully minor it seems 18:35 nerzhul you saw it looking my PR ? :D 18:35 Krock after all the original database is still kept after the migration 18:35 Krock no, I shamelessly stole it from https://github.com/minetest/minetest/issues/9670#issuecomment-613563738 18:35 nerzhul yep and it's only for safer migration, to save the loaded blocks 18:35 nerzhul woot :( 18:36 nerzhul it matches my code analysis, perfect haha 18:36 nerzhul Krock can you take 5 min to look at my post here please: https://github.com/orgs/minetest/teams/engine/discussions/27 ? 18:39 sfan5 merging #9669 in 5m 18:39 ShadowBot https://github.com/minetest/minetest/issues/9669 -- scriptapi: Some small optimizations to value pushing by sfan5 18:41 Krock til Lua has an API for pre-allocating tables 18:41 nerzhul merging #9600 18:41 ShadowBot https://github.com/minetest/minetest/issues/9600 -- Refactor texture overrides and add new features by Df458 18:42 nerzhul sfan5: approved #9621 too 18:42 ShadowBot https://github.com/minetest/minetest/issues/9621 -- Update wireshark dissector by sfan5 18:42 sfan5 thanks 18:42 Krock pushing that prostgres fix (ok according to nerzhul)... 18:42 nerzhul yep, it was a very nice mistake 4 years ago and nobody migrated since or ? :D 18:43 sfan5 postgres is pretty good so why would you want to migrate back 18:43 nerzhul haha :D 18:43 Krock well, they apparently want to make a torrent out of it (or whatever) 18:44 nerzhul i hope the WorldSettings PR can be improved, i wait for your last comments to amend sfan5 on it, after that we will have proper for-core interface 18:46 sfan5 nerzhul: next time you merge it would be nice to clean up the commit message https://github.com/minetest/minetest/commit/5cf6318117edcae6bf30d829d9e9dd9dbf1d4bf7 18:46 nerzhul woops, right i forgot that point, sorry 18:47 nerzhul i think i will code a helper for a such thing, it's pretty common to do a search research, i think luaEntitySAO can do the helper itself, maybe with some useful filters for things with mobs => https://pastebin.com/tyArFwuQ 18:48 nerzhul `local pos = self.object:get_pos()` we have that call just before, having an optimized version can be nice 18:49 sfan5 I think most of the time is currently wasted on an inefficient get_objects_inside_radius 18:49 nerzhul yep 18:49 nerzhul it's why i think luaEntitySAO should do the search 18:50 nerzhul it's on the mob loop, with a timer, i understand why but it's consuming 18:51 Krock nerzhul: how about MinGW for GH Actions? 18:52 nerzhul i didn't do it in my poc, but if we use the travis way it should work, it's on a regular ubuntu 18:53 rubenwardy I disagree. The objects should store in spatial partition before any offer option 18:54 Krock nerzhul: getting the build log will be a click further away, and the build logs are no longer colored. however, it works, thus no objections from my side 18:54 rubenwardy Putting a cache into SAOs won't help particles or the lua APIs 18:55 nerzhul rubenwardy, we should prevent using cache where it's not needed, and i didn't talked about a cache, i talked about high level lua call on entitySAO 18:55 nerzhul to prevent heap moves/allocations between core & lua part 18:56 Krock binary tree position access 18:56 nerzhul here if i offer luaEntitySAO a getObjectAround call it permits to prevent 3 lua call (object:get_pos()), get_ojbects_inside_radius + various comparison 18:56 rubenwardy I don't understand why that would help anything 18:56 nerzhul you reduce the server load and you prevent having many call stacks between core and code, and it stays sufficiently generic 18:57 rubenwardy Optimisations are better when they don't involve any involvement from modders, and they reduce a bottle neck 18:57 nerzhul binary tree is useless in my case, i have 30 entities loaded on the map currently and that lag, it's not normal 18:57 sfan5 4 Lua -> C calls won't matter when get_objects_inside_radius takes a whole second 18:57 nerzhul i'm trying to figure which calls we can add to reduce the lua load 18:57 rubenwardy Calling a few different methods isn't a problem, get_objects_inside_radius is the problem 18:58 sfan5 rubenwardy: you don't plan to rely on libspatialindex for this do you? 18:58 rubenwardy No 18:58 sfan5 not just because I'd have to build it for windows but also because many people build without it 18:58 nerzhul i can look at both, but 2 years ago on the epixel fork my entities in pure c++ are using it without optimization and it lagged only after 1 thousands loaded 18:58 rubenwardy The simplest spatial partition is a chunk based kne 18:58 rubenwardy And should be used for most uses 18:59 sfan5 how does that look in C++? 18:59 sfan5 hm maybe I should read the wikipedia page on that 18:59 nerzhul wtf is that 18:59 nerzhul why we have a call to send mapblock from a sao position, it's wtf 18:59 nerzhul sao object sorry 19:00 rubenwardy sfan5: by chunk based, I meant like how mapblocks store modes 19:00 rubenwardy You have fixed size partitions based on space 19:00 sfan5 std::map> ? 19:01 sfan5 that would be terrible for range searches though 19:02 sfan5 https://en.wikipedia.org/wiki/Spatial_database#Spatial_index oh hmm 19:02 sfan5 I remember some issue suggesting octrees in 2013 19:02 Krock aren't objects currently assigned to a mapblock? if so, then the mapblock and its 3x3x3 * radius^3 neighbours must only be checked 19:02 nerzhul they are saved with mapblocks, but you cannot rely on that for the map for me 19:02 sfan5 they are not, mapblocks only store the static data of objects 19:02 rubenwardy No, objects are in a list in the environment currently 19:02 nerzhul mapblocks are pretty stateless 19:03 sfan5 ..which also means get_objects_inside_radius is an O(n) operation 19:03 rubenwardy There was a pr by numberzero that did what I said - moved to using a fixed size indes 19:03 rubenwardy It was merged and then reverted 19:03 Krock somehow already expected that. would also be an approach, although not the best one 19:03 Krock well yes, it was buggy 19:04 Krock large objects didn't collide with others, or small ones didn't at all (I believe) 19:04 nerzhul another point you missed on this call, on the lua side 19:04 nerzhul we do a double lookup 19:05 nerzhul we gather all ids and the we lookup the ObjectPointer based on ids 19:05 nerzhul i think one of the most costly operation is there 19:05 nerzhul we should return AO pointers directly when possible 19:05 sfan5 isn't id to object a std::map? 19:05 nerzhul std::unordered_map yep but we lookup it to 19:05 nerzhul and it's the same lookup 19:06 nerzhul ActibeObjectMgr m_active_objects map 19:06 nerzhul let me propose an optimisation on the lua call 19:14 sfan5 merging #9621 in 5m 19:14 ShadowBot https://github.com/minetest/minetest/issues/9621 -- Update wireshark dissector by sfan5 19:15 sfan5 and #9603 19:15 ShadowBot https://github.com/minetest/minetest/issues/9603 -- Add all src folders to doxygen by Desour 19:19 nerzhul okay i have a good design and the lambda use for filtering is quite nice too 19:19 nerzhul it should reduce massively the calls to the std::map by calling it once only 19:20 sfan5 but std::map is efficient, the problem is the linear search 19:20 sfan5 also why do client/activeobjectmgr.cpp and server/activeobjectmgr.cpp contain essentially the same code? 19:21 nerzhul yep but calling it less is also fine, if you have massive object list to search :D 19:21 nerzhul hmmm i don't know, maybe a non refactored part, if i remember both where refactored and extract from their respective env, but certainly not factorized in the past (i did that if i remember some years ago) 19:22 nerzhul in my case i have also item drop, i think mobs & item drops (which are certainly the two major usecase mods in survival) are very heavy because they must rely on that 19:26 nerzhul #9671 19:26 ShadowBot https://github.com/minetest/minetest/issues/9671 -- Optimize get_object_inside_radius call by nerzhul 19:26 nerzhul optimized version 19:26 nerzhul it's far more better except if the list is empty, it's the same complexity :) 19:26 nerzhul and also sexier c++11 :D 19:27 nerzhul and also notice that it fixes table size which is too big if we filter entities gone just after :D 19:28 nerzhul oh a second, i think we can remove the old call too 19:29 nerzhul the only call to the ids is to lookup another time after that 19:29 nerzhul i will refactor a little bit 19:30 nerzhul that may help the collision code too :D 19:30 nerzhul collisionMoveSimple call do the same thing 19:42 nerzhul sfan5, and i agree that we should after my PR refactor, i can take the point 19:44 nerzhul okay #9671 is complete and unittests have been updated accordingly to the new possible scenario 19:44 ShadowBot https://github.com/minetest/minetest/issues/9671 -- Optimize get_object_inside_radius call by nerzhul 19:48 Krock nerzhul: IMO it would be more intuitive to use the filter callback as "include" rather than "exclude" from the output list 19:49 nerzhul exact, nice catch, i kept the original behaviour, i agree 19:52 nerzhul let ma second to compile & test and then i push force 19:53 Krock depending on how expensive it is to append table values, you could use the filter callback to push the values directly, without looping over the results for a second time.. although that's not a nice solution 19:53 nerzhul on the collision code you mean ? 19:54 Krock ModApiEnvMod::l_get_objects_inside_radius and collisionMoveSimple 19:54 Krock you basically only need a callback for matches there, and could use the results directly within the callback to push values or append them to the vector 19:54 nerzhul this can be a hack on my feature yep 19:55 nerzhul we will create a vector for nothing but why not 19:55 nerzhul we prevent looping over another 19:55 Krock question is whether the vector return value is needed in most use-cases 19:56 Krock after all it would also be possible to make it an optional pointer, and to ignore push_back for nullptr 19:57 Krock that or the callback could skip push_back anyway in the current code 19:57 nerzhul i return false in the cb and no push back in the result :D 19:58 Krock exactly yes 19:58 nerzhul for the l_get_objects_inside_radius if i push in lua direct i don't know the lua table size at allocation 19:59 nerzhul i think it will be heavier to allocate the memory on each insert than just looping on the result htere 19:59 nerzhul there* 20:00 nerzhul optimized version pushed :) 20:01 nerzhul hmm that seems broken with the latest change 20:01 nerzhul oh i inverted the condition... 20:02 nerzhul it's fixed :D i forgot to inverge the push_back condition path on the manager 20:03 nerzhul mobs seems a little bit smoother on my PC, fine 20:03 nerzhul thanks for your return Krock, waiting for your analysis/approval :D 20:07 Krock looks about right. will test tomorrow 20:07 nerzhul perfect 20:08 nerzhul i'm looking for the item drop mods, i think there is some possible optimisation paths too, but don't know if i can generate some lua api calls for that 20:08 Krock although "includeObjCallback" is a strange was to name a variable 20:08 nerzhul feel free to propose better :D 20:08 Krock I thought Minetest enforces snake case for those 20:09 nerzhul item_drop call minetest.get_player_privs every 0.2 sec omg 20:09 Krock include_callback ("Obj" is self-explanatory due to the scope) 20:09 nerzhul it's why i like mods, you find a single call to core and then boom :D 20:10 sfan5 I doubt this is much faster but it's a good cleanup either way 20:11 Krock it reduces complexity from O(n²) to O(n) from what I can see 20:11 Krock unless unordered_map.find() is O(1) 20:12 sfan5 two for loops are O(n), only O(n²) if nested 20:13 Krock ikr, but getActiveObject is O(n) in worst case 20:13 sfan5 https://en.cppreference.com/w/cpp/container/unordered_map/find is linear on average, O(n) worst-case 20:13 sfan5 btw 20:13 sfan5 constant on average* 20:14 Krock after all there won't be much of a speed difference 20:14 Krock you're right 20:15 rubenwardy the worstcase would be if all the elements were in a single hash 20:33 sfan5 reminder #9617 20:33 ShadowBot https://github.com/minetest/minetest/issues/9617 -- Improve protocol-level receiving code by sfan5