Time Nick Message 13:26 grorp merging #14597, #14657 in 10 min 13:26 ShadowBot https://github.com/minetest/minetest/issues/14597 -- Display background & moving progress bar on shutdown screen by chmodsayshello 13:26 ShadowBot https://github.com/minetest/minetest/issues/14657 -- Fix inventory: Quickly picking up item and placing it again no longer works by sfence 13:28 MTDiscord pushing https://gist.github.com/appgurueu/65bdced10db37e4b9a07ede57ec7a989 in 15m 13:33 grorp luatic: looks good 13:36 MTDiscord how do we feel about C-style number casts 13:36 MTDiscord C++ best practice is static_cast(...) i think 13:41 grorp Lars: I don't care, but I'm not a C/C++ veteran, so maybe someone else should answer 13:42 grorp do you still aprove #14660? 13:42 ShadowBot https://github.com/minetest/minetest/issues/14660 -- Flip "opaque_water" setting to "translucent_liquids" by Xeno333 13:42 MTDiscord yes 13:50 sfan5 @luatic I don't think register_item is supposed to be allowed during runtime 13:51 sfan5 but I guess if it works right now it's not bad to have is accidentally unit tested too 13:51 sfan5 it* 13:52 sfan5 your test in fact does fail 13:52 grorp merging #14332 in 10 min (it has had two approvals for two weeks...) 13:52 ShadowBot https://github.com/minetest/minetest/issues/14332 -- Fix glitch through celiing with Sneak Glitch by sfence 13:55 MTDiscord sfan5: okay, where do you suggest i move this? 13:55 MTDiscord off the top of my head i don't see an appropriate place for "load-time unit tests" 13:56 MTDiscord should i start such a file? 13:56 sfan5 well I don't think it *needs* to be moved as long as it works in this state 13:57 sfan5 to answer the question: 1) add a file to devtest that unconditionally runs this test at startup 13:57 sfan5 or 2) create a C++ unittest that spins up ServerScripting and can thus run it in a self-contained manner 13:57 MTDiscord it won't work if executed with /unittests 13:58 sfan5 it doesn't work in CI either: 13:58 sfan5 (server) 2024-05-21 13:51:07: ERROR[Server]: .../work/minetest/minetest/bin/../builtin/game/register.lua:66: attempt to concatenate a nil value 14:00 MTDiscord could improve that error message tbh but yeah it's because it's running after load time 14:00 sfan5 wonder if it would work after load time with the ":mod:itemname" syntax 14:06 MTDiscord sfan5: well, the registration doesn't fail 14:06 MTDiscord but i doubt that it'll be registered properly 14:10 MTDiscord sfan5: can i make unittests error if autostart is enabled and there is a test failure? 14:10 MTDiscord i'm thinking assert(ok, "There were test failures. Check the console for detailed output.") 14:11 MTDiscord also would you like something like a simple framework for the load-time tests or is just asserting enough? 14:11 sfan5 unittest autostart currently wait for the first server step, I'm relucatant to change this 14:12 MTDiscord yes 14:12 MTDiscord i don't want to change this 14:12 MTDiscord i just want it to brutally error if a test fails 14:12 sfan5 how does that help? 14:12 MTDiscord makes it basically impossible to overlook the test failure 14:13 sfan5 still don't understand the problem you want to solve 14:14 sfan5 you ran /unittests manually earlier and missed that it said there were text failures? 14:14 sfan5 test* 14:14 MTDiscord no, i ran it via autostart, and nothing happened so i wrongly assumed it was fine when it was not because i didn't look at the console 14:15 MTDiscord i only ran /unittests later to see whether that differed in its behavior from autostart 14:16 sfan5 hm 14:17 sfan5 how about adding chat color to the result and also in case of autostart having the code send the result to the first player who joins? 14:18 MTDiscord sure 14:19 MTDiscord anyways as for properly testing this at load time: https://gist.github.com/appgurueu/65bdced10db37e4b9a07ede57ec7a989 14:19 MTDiscord tell me if you want something like a simple framework for labeling these tests as well rather than just a bunch of assertions run at load time 14:20 sfan5 your diff is fine for me 14:23 MTDiscord sfan5: why send just to the first player tbh? this complicates matters a bit. not much, but it's still a bit cumbersome tbh, and i don't see a big benefit. 14:29 MTDiscord sfan5: okay, so here's what i would do to colorize the result & send it to joining players: https://gist.github.com/appgurueu/119d51fdbbe0cd00af019ebdd3e263e5 14:30 MTDiscord (as well as already joined players) 14:31 MTDiscord if you think these are fine, i would go ahead and merge them 14:34 sfan5 lgtm 15:35 MTDiscord oof, CI still failing, because it sets on_finished and we overwrite that. this time i'll let CI run through first. 15:52 [MTMatrix] merging now 16:09 MTDiscord pushing the CI fix now that it passed CI on my repo 17:02 MTDiscord sfan5: would you be fine with reviewing animated gltf support as well, in the same PR, or would you prefer reviewing it split over two PRs? 17:04 MTDiscord it would add about 800 loc to review, including sparse accessor support 17:04 sfan5 i thought you wanted to merge this and do animated later? 17:04 MTDiscord sfan5: i have working animation support (and some more features for the static thing) in my branch already, also tested to be working in-game 17:05 MTDiscord we wanted to make it easier to review by making the animation features a followup 17:06 sfan5 should be that way imo 17:07 MTDiscord okay i take it you would prefer to review static / animated separately? 17:08 sfan5 yes 18:53 MTDiscord sfan5: we don't support big endian machines, do we? 18:54 sfan5 yes we do 18:55 MTDiscord sfan5: okay, well it should be fine either way with the latest commit