Time Nick Message 14:52 MTDiscord Is there a good reason that client active objects have to remain at the same memory address? 14:52 MTDiscord Why isn't the active object manager allowed to copy them? 15:58 Desour josiah_wi: idk about CAOs, but we store SAOs ptrs in objrefs. and consistency is easier 15:59 Desour a deleted move or copy constructor doesn't necessarily mean that the operation is forbidden by logic, but it's often just not implemented because copying or moving is not necessary 16:02 MTDiscord We have a unit test that ensures that a registered object doesn't get its memory location changed. 16:02 MTDiscord To me, that implies that something very bad would happen if it did change, which implies a big design issue. 16:02 Desour can you point me to the file+line? 16:03 MTDiscord Oh boy, I have it all changed in my source tree now. Let me check the file on GitHub. 16:04 MTDiscord src/unittests/test_clientactiveobjectmgr.cpp line 95 16:04 Desour thx 16:05 MTDiscord I'm rewriting that file to use Catch2, and in the process I want to make the tests more useful to us and more robust. 16:05 MTDiscord I thought it might be good to remove that check if it isn't really a useful requirement. 16:06 Desour ah, that looks like it's just ensuring that the getActiveObject function works properly 16:06 Desour I'd still keep pointer stability though 16:06 Desour just to be sure 16:07 sfan5 there are no advantages to not having pointer stability 16:07 sfan5 all it would do is cause issues 16:08 Desour also, copying CAOs probably doesn't make sense, btw., as there would suddenly be 2 CAOs with the same id 16:08 MTDiscord Alright, thanks, I'll put the pointer stability requirement back in. 16:34 MTDiscord Running valgrind in CI was a great idea. Thanks whoever set that up. 16:38 MTDiscord This is concerning; the invalid memory access that it just caught should have happened with the old version of the unit tests as well. 16:40 MTDiscord That CI even builds in debug mode. Interesting. 16:50 MTDiscord I wonder whether DS's changes fixed this . 16:54 MTDiscord Ah, nope, leak was my fault. 16:54 MTDiscord invalid read* 17:09 MTDiscord Desour, would you be willing to review https://github.com/minetest/minetest/pull/13609? 17:10 MTDiscord Sorry, DS. I saw you prefer to be called that. 17:15 Desour don't worry about the name :). I don't have any experience with using catch2. do we have an issue about migrating unittests to catch2? 17:17 schwarzwald[m] We do not, would you like me to create one? 17:18 Desour yes, would be good for getting concept approval 17:41 schwarzwald[m] https://github.com/minetest/minetest/issues/13610 17:59 Krock don't we already have Catch2 enabled? 17:59 Krock ah. it's for benchmarks. 18:01 schwarzwald[m] Yes, we use it for its benchmarking capabilities. We do have our own profiler that we time the unit tests with as well, interestingly. 19:18 Neo[m]123 someone here with an example for play menu formspec ? like to remove all the pages excet init page and bind only few settings (like audio toggle or shadows) to that page? like no mods tab, no choosing of map at start etc... for making stand-alone game out of minetest... 19:19 ROllerozxa I have one for multiplayer 20:36 rubenwardy I support moving to a standard test framework, Catch2 makes sense as we already use it 20:36 rubenwardy plus CLion has support for Catch2 but now our custom test framework 20:36 rubenwardy *not 20:36 rubenwardy other IDEs probably too 20:37 rubenwardy shouldn't be _too_ hard to review just be comparing before and after and the tests pass 20:37 rubenwardy plus you can split into multiple PRs 21:01 rubenwardy Looks like clion expects MT to accept arguments of this form: minetest -r xml -d yes --order lex "test client active object manager" 21:08 pgimeno not having it block the main thread is already the biggest advantage <-- do they share the same Lua environment as each other, except the main thread? or do they use a separate Lua environment each? 21:09 pgimeno @josiah_wi please use the # notation for issues and PRs, which lets us see the title together with the link 21:09 pgimeno e.g. #13609 21:09 ShadowBot https://github.com/minetest/minetest/issues/13609 -- Upgrade client active object mgr tests to Catch2 by JosiahWI 21:10 pgimeno because Discord users can see ShadowBot's lines, right? 21:13 MTDiscord yes, we see shadowbots lines, it makes no difference on our end if its shadowbot or a link, since we see the embed with the og information either way 21:16 schwarzwald[m] Sorry sfan, I was inconsiderate of the IRC users. I will remember to use the # notation in the future. Thank you for bringing it to my attention. 21:44 MTDiscord I would assume each will have their own lua environment, but they share a common startup environment. I.e. the same lua files, settings and functions are preloaded into each. Actually extremely excited for this PR, it's one I had on my personal to do list. Can't wait to make a singular tree mapgen 21:48 MTDiscord It would be nice to have a spot or way to register what .lua files are read into that mapgen environment(s). Something like minetest.register_mapgen_lua(filename) or perhaps a folder dedicated to mapgen lua files (less good) 23:54 lhofhansl Going to merge #13314 in a few. Will be good to have these metrics for future optimizations. 23:55 ShadowBot https://github.com/minetest/minetest/issues/13314 -- Instrument touchMapBlocks and block loading/deserialization. by lhofhansl 23:59 lhofhansl And done.