Time Nick Message 00:15 MisterE[m] "MisterE so the workaround is..." <- That, yes 00:18 erle MisterE[m] miiiiight make sense to put your workaround function in the issue for others then 00:18 erle MisterE[m] does minegames have an equivalent to mcl_engine_workarounds? 00:18 erle maybe we should make some polyfill library 00:19 MisterE[m] ... it doesn't. But the workaround has to be implemented in arena_lib, which I don't maintain directly 00:19 erle so that players could largely code stuff for latest minetest engine and then just use that mod 00:20 erle oh well, i'm always the party pooper that tells people to put stuff in separate mods so it can be shared ig ;) 00:20 erle but if you only need it at one of two places i can understand 00:21 erle is there any new stuff that is massively important to people like vector? 00:21 MisterE[m] I mean, I'm not sure how having this simple workaround in a separate mod helps. 00:21 MTDiscord typically backwards compat things get made by subject, see fs51 00:22 MisterE[m] Minetest.after is a common thing to do whith bugs like this, but it is jank and I don't think it always works 00:26 erle it will break if you manage to shut down the server in between 00:26 MisterE[m] ... so? 00:27 erle i guess you want all of these things to be atomic actions 00:27 erle detaching and teleporting 00:27 MisterE[m] Well I guess if you are saving imporant info thats a consideration 00:27 MisterE[m] Oh... position is kinda imporatnt 00:27 erle yeah 00:27 MisterE[m] It is not on my serer 00:27 erle wanna know my extremely cursed workaround? 00:28 MisterE[m] Bot usually is 00:28 MisterE[m] erle: For what? 00:28 erle to make it survive a server restart 00:28 MisterE[m] Oh... not really, its not applicable to me 00:29 erle well, i am talking from the position of a person who has lagged minetest to the point that they can do shit to not-yet-fully generated terrain in vanilla. so i take it kinda seriously ;) 00:30 erle though i have not yet found out how the “dig any node if you can lag the game hard enough” bug works 00:30 erle otherwise there would be a bug report, not because it is bad, but because it encourages people to lag 00:30 erle if anyone of you knows, i am *very* interested 00:37 MTDiscord i think i have an educated guess on the digging any node with lag bug 00:38 MTDiscord probably due to a race condition between the DB manager and lua mod protection 00:38 MTDiscord minetestserver (and the DB) by default doesn't give two shits about whether the node has protectiion 00:39 MTDiscord the lua thread, however, does seem to give a shit, and if the lua thread were to lock up for a while, then the server removes a node from the queue 00:39 MTDiscord it obviously removes it from the DB because it assumes the node has no protection (nor waits for the blocked lua thread) 00:42 MTDiscord the order of events roughly looks like this client node dig event -> server node dig event -> Lua process any on_dig or other dig related code and remove from DB 00:46 erle oha thanks 00:47 erle so hanging for long enough just tosses the on_dig? 00:47 erle and anything else that might happen 01:11 MTDiscord basically 01:11 MTDiscord the entire network stack is: does it talk the way i expect it to 01:22 erle well, maybe protection should work on a deeper level then 01:23 MTDiscord but isn't that an unnecessary feature 01:41 erle the problem is not if protection works or not 01:41 erle the problem is such a thing motivates assclowns to lag servers 01:41 erle lag should never have a beneficial effect on game-theoretical grounds at least 02:21 MTDiscord bold of you to assume that lag isn't a natural extension of the server's age (and level of user builds) 02:24 MTDiscord and if server operators want to guarantee that things are properly protected, they want to invest in better performing vHosts than paying peanuts for monkeys 12:52 Oblomov fwiw, https://github.com/minetest/minetest/pull/12210 is up for the review again with the requested changes, if anyone wants to have a go again 8-) 12:58 sfan5 I suggest renaming it to "Fix some textures not being sent correctly to older clients by 5.5 servers" 13:01 Oblomov sfan5: done, though I stopped at “older clients” to keep the title not too long 13:02 sfan5 s/older/old/ it'll be exactly as long 13:02 Oblomov ah damnit 13:02 Oblomov please don't make me change that again 8-) 13:02 Oblomov (but I will if you think it's important) 13:04 Oblomov sfan5: if you're in the mood there's https://github.com/minetest/minetest/pull/12213 too, which touches things I know nothing about (my analysis of the issue was purely from looking at how the code changed and how revering the change restored the behavior) 13:06 MTDiscord @x2048 ^ 13:07 sfan5 I think I wondered about that change in the review of the PR but I don't remember whether I added a comment to ask for clarification 14:39 rubenwardy Reminder to add things to the meeting agenda. https://dev.minetest.net/Meetings#2022-04-24 15:37 Oblomov sfan5: thanks for the comments on #12210, I pushed an update that hopefully addresses both the actual bug you found and the comment style remarks 15:37 ShadowBot https://github.com/minetest/minetest/issues/12210 -- Fix some textures not being sent correctly to older clients by Oblomov 15:38 sfan5 "except for the ones that are supported on older clients" is that actually all of them? 15:39 Oblomov the ones that can be used as base 15:39 sfan5 okay 15:39 MTDiscord see my comment: What about '(([inventorycube{a.png{b.png{c.png))'? 15:39 MTDiscord I believe the shotgun parser comes back to bite us now 15:40 MTDiscord because the server has no AST to work with 15:40 Oblomov well, that would pass unchanged and it's fine 15:40 Oblomov the real issue would be a (([png:base64])) 15:41 Oblomov but that would have crashed the client even before my fix 15:41 Oblomov so my argument would be that this at least restores functionality without introducing NEW issues, and a more sophisticated solution for this issue can be worked on 15:42 Oblomov I believe a separate issue should be opened to track that 15:42 MTDiscord wait, so to bybass krocks bug, one could just wrap the texture modifier in (()) ? 15:43 MTDiscord wsor: yes 15:43 MTDiscord just () should suffice too 15:44 MTDiscord nice, thanks. guess im going to have to add that to every affected mod now 15:44 Krock and suddenly it's my bug 15:44 MTDiscord :) 15:44 sfan5 how many is "every affected mod" 15:44 sfan5 one? 15:45 MTDiscord if i understood correctly, any mod that used [modifier is affected? 15:45 MTDiscord to start with 15:45 Oblomov only [png so far 15:45 Oblomov the newly introduced [png is the only real issue 15:45 Krock any that does not exist in 5.4.0 or older clients 15:45 MTDiscord wsor: [combine and [inventorycube were affected 15:46 Oblomov well they were affected by the safety introduced for [png 15:46 Oblomov and only when connecting older clients to newer servers 15:46 MTDiscord yes, but [combine and [inventorycube will be broken on non 5.5 clients connecting to a 5.5 server 15:46 Oblomov jonathon: that's what #12210 fixes 8-) 15:46 ShadowBot https://github.com/minetest/minetest/issues/12210 -- Fix some textures not being sent correctly to older clients by Oblomov 15:47 MTDiscord yes, so every single mod that uses those, has to wrap in () now 15:47 MTDiscord for 5.6 15:47 Oblomov ah yes 15:47 Oblomov I mean, this could be backported into a quickfix for 5.5.1 15:47 sfan5 this is for mods who use node tiles that begin with "[combine" or "[inventorycube" 15:47 sfan5 nothing more 15:47 MTDiscord which means things like signs lib, game agnostic mods that fallback to [combine^[noalpha^[colorize:color 15:48 Oblomov well, I found it through nodecore 15:48 MTDiscord or multiple images being combined, etc 15:48 Oblomov only if the _base_ is a [combine 15:48 MTDiscord yeah 15:48 Oblomov if the images are combined starting from an actual png, no issue 15:49 MTDiscord even if 5.5.1 comes out, still with have to work around it, due to people not upgrading 15:49 Oblomov ¯\_(ツ)_/¯ 15:50 Krock to protect against ((([png it would be possible to scan for the first '[' character which is not after '^' although that's quickly escalating into hacky territory 15:50 MTDiscord wait, thats wrong, 5.5 servers would need to update 15:50 MTDiscord clients would be fine 15:50 Oblomov yeah, only the server needs the fix 15:50 Oblomov in fact this is a server-side fix 15:51 MTDiscord but singleplayer would still mean patches needed 15:51 Oblomov singleplayer usually have the same version on client and server 15:51 sfan5 this issue cannot affect singleplayer 15:51 Oblomov again no issue 15:51 MTDiscord oh right, sorry 15:51 Oblomov this only affects pre-5.5 clients connecting to unpatched 5.5+ servers 15:52 Oblomov Krock: it's a possibility, but again I would argue that that's a separate issue 15:52 Oblomov @luatic: open an issue for the () bug please 8-) 15:52 MTDiscord Oblomov: give me a minute to type 8-D 15:53 MTDiscord anyways #12214 15:53 ShadowBot https://github.com/minetest/minetest/issues/12214 -- Complex texture modifiers using [png may still crash older clients 15:53 Oblomov FASTER! FASTER! 15:54 MTDiscord @wsor: don't patch this in sources! 15:55 MTDiscord ...in sources? 15:55 MTDiscord as in, don't start editing sources, replacing [combine with ([combine:...) 15:55 MTDiscord ....and why not? 15:56 Oblomov only wrap [png: in (), so you can crash old clients ;-) 15:56 MTDiscord because considering that this is apparently neatly scoped, you could just write a trivial patch mod looping over registered nodes and their tiles 15:57 MTDiscord well yeah, that was my idea. however getting people servers to actually use it would be the issue, rather than patching the mod directly 16:00 Oblomov well, let's start by getting the fix in, and possibly backport it for 5.5.1 if there is such a thing 16:00 Oblomov is there a way to mark a PR to ask that it may be consindered for the fix branch? 16:01 sfan5 no 16:01 Oblomov so I would have to create a new PR against stable-5? 16:01 sfan5 no 16:01 Oblomov (after this gets merged) 16:02 sfan5 if it's decided that there should be a 5.5.1 then some dev will go through commits and backport eligible ones 16:02 Oblomov ok 16:16 MTDiscord anyways @wsor https://gist.github.com/appgurueu/dea1d1d9d8494e9c00114d36d58c5932 should work 16:25 Krock > fixing a bug 16:25 Krock > new issue opened 16:25 Krock yet another normal day in MInetest 16:28 Oblomov Krock: at least if the fix got merged the balance would be even, but the fix hasn't even been merged yet 8-D 16:28 Krock this is why we can't have <1000 issues 16:29 Krock anyway, I agree that the comment wording in your PR could be shorter 16:29 Krock nobody wants to read a wall of text 16:29 Oblomov Krock: I have shortened 16:29 Oblomov is it still too long? 16:30 Krock no, it's my browser that did not update properly. alright. thanks! 16:31 Oblomov (fwiw, as a developer I don't mind wall-of-text comments that provide the right context information, and I would argue that the issue with my comment rather than legnth was that it failed to touch the proper points) 16:33 Oblomov so, anything else needed to get it merged? 16:35 Krock I don't think so 17:04 Oblomov (anyway, I fixed TWO bugs, including the other PR, so maybe we could even consider the whole thing a net positive this weekend) 17:12 jwmhjwmh I don't know if you've had the meeting yet, but if you haven't, perhaps you could discuss #12128. There are some unresolved questions in the Todo section of the PR. 17:12 ShadowBot https://github.com/minetest/minetest/issues/12128 -- Add bulk ABMs by TurkeyMcMac 17:52 sfan5 rubenwardy: do you want to merge your pull on the website repo? 17:59 rubenwardy Donation ? Was going to run it by celeron55 18:00 sfan5 ye 18:22 MTDiscord Oh wow, #12128 looks like almost exactly the same functionality as a patch I once implemented, tested, and then abandoned when I discovered that the benefit I had expected to get from it was basically nil and it had no impact... 18:22 ShadowBot https://github.com/minetest/minetest/issues/12128 -- Add bulk ABMs by TurkeyMcMac 18:23 MTDiscord The idea of using a VM is not bad but it does of course require rewriting a bunch of logic. I was looking more for a drop-in replacement of the old ABM's method of calling into Lua for each node. The idea was that it would take less work to pass a table once and unpack it lua-side than making all those C++/lua calls ... but that didn't work out. 18:25 sfan5 ==reminder==: meeting is in about half an hour+ 18:27 Krock ? 18:30 sfan5 I am positively surprised that the first page of visible issues goes back further than a month 19:03 sfan5 merging #12213, #12210, #12207, #12192 in 5 minutes 19:03 ShadowBot https://github.com/minetest/minetest/issues/12213 -- Fix worldaligned textures by Oblomov 19:03 ShadowBot https://github.com/minetest/minetest/issues/12210 -- Fix some textures not being sent correctly to older clients by Oblomov 19:03 ShadowBot https://github.com/minetest/minetest/issues/12207 -- Fix typo: `vector.check()` ought to be `vector.check(v)` by appgurueu 19:03 ShadowBot https://github.com/minetest/minetest/issues/12192 -- Use mod names/titles instead of technical names by GoodClover 19:05 erle #12210 should be told to debian, ig. 19:05 ShadowBot https://github.com/minetest/minetest/issues/12210 -- Fix some textures not being sent correctly to older clients by Oblomov 19:05 sfan5 debian has not even packages 5.5 19:05 sfan5 s/s /d / 19:06 erle i just told the debian packager 19:06 sfan5 and if you plan to individually report every backportable fix to the debian devs then I won't be surprised if they ignore you 19:06 rubenwardy meeting time. HuguesRoss, Krock, nore, nrz_, sfan5, ShadowNinja, sofar 19:07 Krock sfan5: feel free to add #12143 to the list if you don't mind 19:07 ShadowBot https://github.com/minetest/minetest/issues/12143 -- Builtin: Allow to revoke unknown privileges by SmallJoker 19:07 v-rob[m] Here and present 19:07 sfan5 Krock: sure 19:08 erle sfan5 could you add https://github.com/minetest/minetest/pull/12089 too? it's been 2 months and the devtest nodes are obviously showing a rendering bug (i.e. the textures may even look different on different systems). 19:09 Krock erle: still needs a second approval 19:09 erle Krock, that could come from sfan5 himself 19:09 sfan5 just like I disagreed with the previous devtest texture test PR I also disagree with this one 19:10 erle do you disagree with *all* devtest nodes that serve to check correct rendering? 19:10 Krock how likely is it that the rendering part that it's supposed to check breaks? 19:10 erle because if yes ig i don't have to ask you for any approval on that topic 19:10 sfan5 these nodes have nothing to do with rendering, they check correct decoding of textures, that's why 19:11 erle not entirely sure 19:11 erle these nodes should also allow you if you have set the gamme correct for your monitor if i am not mistaken 19:12 Krock well, isn't that a unittest for irrlicht instead? 19:12 sfan5 ok PRs merged 19:12 Krock great then let's begin. 19:12 sfan5 "PR discussion/reviews " first? 19:12 Krock #11854 19:12 rubenwardy sure 19:12 ShadowBot https://github.com/minetest/minetest/issues/11854 -- Shader improvements: Saturation Optimization by Pevernow 19:13 Krock I only had a brief look at it, and do not really see a need for overly saturated filters 19:13 Krock s/filters/shaders 19:14 erle in before “add a postprocessing stage already“ 19:14 sfan5 firstly that thing should be called "Saturation setting" 19:14 sfan5 and second haven't we discussed that before 19:14 rubenwardy we discussed it in the last meeting, but no conclusion was made 19:14 v-rob[m] I thought Tone Mapping already saturated stuff somewhat? 19:15 v-rob[m] At least, it looks that way to me 19:15 sfan5 needs changes which weren't/can't easily be done -> close 19:15 Krock as a side thought it would be an interesting toy for modders to change the environment's appearance, although not everyone has their shaders enabled 19:16 sfan5 shaders will become a requirement sooner than later anyway 19:18 sfan5 sooo, do we close that PR? 19:18 Krock if feasible, a post-processing step would be more favourable which hopefully handles all cases that currently lack support (entities, particles) 19:18 rubenwardy I'm not a fan of building low-priority features on unstable foundations, I'd like to see post-processing for tone and color mapping generally 19:18 sfan5 that sounds like the way forward for that PR yes 19:19 Krock good idea, but needs, as you said a better foundation. 19:20 Krock I'd prefer to not decide on the future of this PR. it's way off my knowledge 19:21 v-rob[m] Heh, same 19:22 sfan5 given that HuguesRoss has pointed out issues with it that don't look like they're getting fixes I don't see any option but to close 19:22 Krock okay 19:22 MTDiscord To sum it up: (1) doesn't deal with entities / particles / sky etc.; (2) is inefficient as it can't leverage a second rendering stage; (3) requires shaders (which we don't require yet) 19:23 sfan5 (3) is not a problem 19:23 rubenwardy (3) isn't a problem imo for optional graphics settings 19:23 MTDiscord (inefficient as in: this is done for every node fragment, even occluded fragments, instead of every screen "fragment" (pixel)) 19:23 rubenwardy but yeah, I don't like how it's less efficient and how it needs to be implemented over and over again 19:24 MTDiscord Also game devs can already achieve a somewhat similar effect simply by increasing the saturation of their node textures. 19:25 MTDiscord (public opinion seems to be +1-4 BTW, looking at the reactions on GH) 19:25 Krock @luatic although that's not user-specific to customize their needs 19:26 MTDiscord Krock: users can use auto-generated texture packs :P 19:26 erle Krock sfan5 the gamma thing is not a texture decoding bug btw, just to clear that up. the values are correctly decoded. it is a rendering pipeline issue and currently it breaks for every PNG file with a gAMA chunk (except for gamma 1.0 i guess?) 19:26 MTDiscord heck, I even wrote myself a mod that can generate texture packs 19:27 erle uh, did the saturation PR *change* meaningfully since the last meeting in which everyone said the same things? 19:27 rubenwardy no 19:27 sfan5 Minetest does not handle texture gamma so in what way are they "correctly decoded" 19:27 rubenwardy let's close 19:28 v-rob[m] Seems reasonable to me 19:28 Krock no more wasting time on this topic. two votes against. it can still be re-opened if someone is willing to implement it properly 19:29 Krock next up is #12181 19:29 ShadowBot https://github.com/minetest/minetest/issues/12181 -- Single functions to get/set every celestial vault element by Zughy 19:29 Krock concept judging 19:30 sfan5 dunno 19:30 Krock it's basically recycling the other API to combine them all 19:30 rubenwardy API design is hard 19:30 Krock solving cluttered API by adding more API 19:30 Krock works for me 19:30 sfan5 not totally necessary IMO 19:31 Krock there's not many mods that would actually use this API 19:31 Pexin "there are now N+1 competing standards" ? 19:32 Krock not quite. it's not competing but recycling 19:32 MTDiscord it would be useful to be able to set them all at once, but why couldnt this be done by a mod? 19:32 MTDiscord ^ 19:32 v-rob[m] In theory, the "correct" design could either be a single function or one function for each member of the tables. I dunno. 19:32 MTDiscord And even if it weren't done in a mod, why not in builtin Lua? That would be a lot less gross 19:32 Krock latter already happens 19:33 erle sfan5, if i am not mistaken for correct texture calculations (like blending) you need the raw pixel values, have to transform that into linear space which has to have a higher precision, do any work, then transform it back. you can *probably* try to hack it roughly by putting in some pow(pixels, gamma) and pow(pixels, 1/gamma) stuff in the texture decoding logic, but I think it does not belong there. 19:33 Krock @luatic Lua should not mess with C++ API in objects 19:33 rubenwardy erle: please don't go offtopic during a meeting 19:33 v-rob[m] Extending userdata from builtin Lua seems like the wrong way to do things to me 19:33 erle rubenwardy ok 19:33 Krock the PR will stay open and discussed later on 19:33 MTDiscord I'm not saying that Lua should mess with the C++ API 19:34 v-rob[m] Oh, a standalone function? 19:34 Krock (previous sentence refers to the gamma devtest PR) 19:34 MTDiscord v-rob: as long as we can't properly obtain the metatables: ys 19:34 MTDiscord yes* 19:34 sfan5 erle: at that point this becomes a feature request for Minetest to implement gamma-correctness which I will gladly NACK 19:35 sfan5 the engine can extend the API from Lua fine but mods shouldn't be encoraged to do this 19:35 sfan5 for example the http api is one or two C++ methods plus a Lua method contributed by builtin 19:35 MTDiscord I want more Lua and less C++ :P 19:36 Krock @luatic That would scatter the code even more. If the functions already exist on C++ side, why not use that right away there? Lua functions can still be used in mods, but should not be done with metatables to keep the code structured 19:36 v-rob[m] Same here 19:36 v-rob[m] I think it's generally easier to write correct and simpler Lua wrappers than to add a new C++ function dealing with the Lua API, but it doesn't play as well when working with userdata 19:37 Krock right 19:37 MTDiscord yes a mod could make a wrapper for this, but is there any reason not to add it? seems there are no negatives and could help reduce boilerplate for mods 19:38 v-rob[m] Code maintainability and reviewer time are the only negatives, I guess. 19:38 sfan5 I've seen worse boilerplate than making 3 function calls for "one" operation 19:38 Krock even though it does seem very low priority, the function does indeed improve the usability 19:39 sfan5 I guess the consensus is to allow it then? 19:40 Krock was about to write that, yes. 19:40 rubenwardy I'm not too worried about maintainability given that the code is shared/structured well enough 19:40 erle having more ways to do the same thing means it is inherently backwards-incompatible though 19:40 rubenwardy one thing I was thinking about is whether we want to redesign the API 19:40 rubenwardy but that doesn't necessarily depend on this, could add more keys to set_cellestial_vault for that 19:41 Krock also to allow versioning in it 19:41 erle i.e. mods written for 5.6 may not work in 5.5 or earlier (cue ubuntu LTS having 5.4 for 2 more years) 19:41 rubenwardy that's not a backwards compatibility problem, that's a forwards compatibility problem 19:41 erle right 19:41 rubenwardy we can't retroactively add features 19:42 erle well, luatic has a point in that something that reuses existing stuff should be lua. vector for example can be easily added to mods that want to have it before it was introduced. and if this thing does nothing new, i wonder why it is not lua. 19:42 Krock commented. this is fine by me even though it does add yet another API function 19:42 Krock objections? 19:42 rubenwardy fine by me 19:43 Krock #12118 19:43 ShadowBot https://github.com/minetest/minetest/issues/12118 -- Make `debug.g/setmetatable` respect the `"__metatable_debug"` metatable field by Desour 19:43 v-rob[m] I'm OK with it 19:43 Krock great, thanks. 19:43 MTDiscord rubenwardy: have you grepped for debug.[gs]etmetatable yet? 19:43 rubenwardy yes 19:43 erle results? 19:43 rubenwardy it's not meaningfully used at all 19:44 MTDiscord can I get the link 19:44 rubenwardy one sec, it expired 19:44 v-rob[m] What's the difference between it and the non-debug versions? Can you just set metatables of non-table/userdata stuff? 19:44 rubenwardy it's unrestricted 19:44 rubenwardy so can get/set any metatable 19:44 v-rob[m] (I mean, it sounds hacky for a mod to use it in the first place) 19:45 rubenwardy Lua allows you to restrict get/set on a metatable by using the __metatable key 19:45 sfan5 that PR is the most promising solution to #12216 19:45 ShadowBot https://github.com/minetest/minetest/issues/12216 -- Lua messing with userdata metatables trivially leads to segfaults 19:45 Krock > Returns the metatable of an object. This function ignores the metatable's __metatable field. 19:45 rubenwardy which can be used on eg: `""` to protect it 19:46 sfan5 so I say keep it open to collect feedback/thoughts and make progress 19:46 rubenwardy https://content.minetest.net/zipgrep/78988654-884c-4271-9acc-83126c7dd8d0/ 19:46 Krock sure 19:46 rubenwardy `local getmetatable = debug and debug.getmetatable or getmetatable` 19:46 rubenwardy is the only use 19:47 sfan5 I am pretty sure nodecore has ObjectRef wrappers which do my knowledge require the use of debug.getmetatable 19:47 sfan5 s/do/to/ 19:47 MTDiscord @Warr1024 ^ 19:48 MTDiscord ah right, somebody just sticked serpent in there 19:48 MTDiscord it seems nodecore uses s/getmetatable, but not debug.s/getmetatable 19:49 rubenwardy I don't see how this solves the issue you linked 19:50 Krock it fixes them by not silently manipulating the metatable 19:50 Krock changes could be detected on Lua/C++ side to properly invalidate and free the instance 19:50 sfan5 @Jonathon it appears to use getmetatable with objectrefs yes 19:51 v-rob[m] Exposing Lua debug functions just feels like the wrong thing to do in the first place. 19:51 sfan5 rubenwardy: it disallows allow access, changes or removal of special metamethods like __gc 19:51 sfan5 because those reside in the raw metatable which the PR would hide 19:51 sfan5 s/disallows allow/disallows/ 19:52 rubenwardy surely you'd have the same effect by removing debug.g/setmetatable and adding __metatable to userdata? 19:52 sfan5 it already has __metatable 19:52 sfan5 combined with the former, yes 19:53 rubenwardy allowing untrusted mods to change the "" metatable is a very bad idea 19:53 sfan5 mesecons would have to become a trusted mod 19:53 rubenwardy mesecons doesn't use debug.g/setmetatable 19:54 sfan5 it changes the string metatable regardless 19:54 sfan5 also practically speaking that is the same as modifying the global `string` table, not something we could protect either 19:55 v-rob[m] Actually, when I was first starting out coding for Minetest, I briefly had a line `debug.setmetatable(nil, {__index = nil})` in one of my unreleased mods to fix nil crashes. Bad idea, but I didn't realize the impact. So, it's definitely more impactful than just modifying the global `string` table or similar. 19:55 v-rob[m] (Actually, I don't think I really understood what it did, since I barely understood Lua) 19:56 sfan5 anyway we can keep the PR open for now even though it may not be needed after all 19:56 rubenwardy sure 19:56 Krock I'll note "Postponed for further inputs." in the wiki 19:56 Krock unless someone else is currently taking notes there 19:56 rubenwardy I'm not 19:56 sfan5 nope 19:56 Krock okay so #12080 would be the next to decide on 19:56 ShadowBot https://github.com/minetest/minetest/issues/12080 -- WIP: Do not stat /etc/localtime all the time by erlehmann 19:57 v-rob[m] I'll pretend I know what anything in that PR means 19:58 Krock calling /etc/localtime does seem to be efficient enough so that nobody noticed this yet 19:58 rubenwardy (remember we're still in roadmap approvals, you don't necessarily need to deep dive - although this is a fairly technical one) 19:59 sfan5 it mixes two changes, the first can/should be done regardless, the second perhaps 19:59 sfan5 the PR itself is a bit inactive 19:59 rubenwardy I don't really have any opinion either way, just that I don't feel like having to read the documentation on these functions 19:59 sfan5 I'd put it in "possible close" in favor of new ones (plural) being opened 20:00 rubenwardy what's the two things? 20:00 rubenwardy 1) localtime not being threadsafe 2) tzset /etc/localtime ? 20:00 sfan5 strftime thread safety + the tzset() call 20:00 sfan5 or was it localtime yes 20:00 rubenwardy cool 20:02 Krock #11946 20:02 ShadowBot https://github.com/minetest/minetest/issues/11946 -- Add mods list formspec display for /mods cmd. by Andrey2470T 20:02 rubenwardy implementation looks like a mess 20:02 Krock unlike the /help GUI, there is no helpful information in it 20:03 rubenwardy the engine already has this information 20:03 sfan5 the helpful information is which mod is in which modpack 20:04 sfan5 from myself: concept yes, implementation needs work 20:04 Krock when exactly would you need that? 20:04 rubenwardy I'm not a fan of the implementation, it should get the information from the engine which already has information on modpack/mod relationships 20:04 sfan5 I don't know, but it is additional information 20:04 rubenwardy I guess it's useful when you have things like mesecons 20:05 rubenwardy and you want to collapse it down to just mesecons modpack 20:06 Krock I'd assume barely any player looks at /mods and even cares about whether there's modpacks. in local games, the structure is already shown in the World Configuration dialog 20:06 MTDiscord suprisingly a lot of players look on servers 20:07 Krock +2 -1, although I wouldn't mind much. it's just not something where I see actual need 20:07 Krock concept approved 20:07 rubenwardy who's the plus 2? 20:08 sfan5 you and me? 20:08 Krock ^ 20:08 Krock yet only concept. the implementation obviously needs work 20:08 rubenwardy suppose. I'm borderline neutral, but that might be because the implementation scares me 20:08 rubenwardy but yeah go ahead 20:08 MTDiscord excuse me for butting in, but why don't you make the existing mod formspec code work with this so there's not two copies? 20:09 rubenwardy Yeah, you could use the existing code for mods -> tree 20:09 erle sorry, was distracted. yes, i had a 1.5 months or so break from developing minetest-related things. i can continue to work on https://github.com/minetest/minetest/pull/12080 20:09 MTDiscord Two cents if I may: I severely disliked the formspec menu for /help and I really wish there was a command switch to do text-mode (maybe there is and I missed it?), so if such a thing were to be added for /mods I would really like a text option. 20:09 sfan5 /help -t 20:09 rubenwardy but you wouldn't want the full GUI 20:09 MTDiscord Awesome, thank you 20:09 MTDiscord Please make sure such an option is included for /mods as well should it be merged 20:10 sfan5 the existing code is written in the context of the mainmenu, it can be generalized/ported to work in the game environment but it's questionable how much work that really saves 20:10 Krock /help /help or so should actually state the text-only feature 20:10 sfan5 @GreenXenith yes 20:10 rubenwardy <+sfan5> the existing code is written in the context of the mainmenu 20:10 rubenwardy I'd like to refactor the mod/content/pkgmgr code, that will make this easier 20:11 sfan5 >>next>> https://github.com/minetest/minetest/pull/12208 20:11 sfan5 #12208 20:11 ShadowBot https://github.com/minetest/minetest/issues/12208 -- Add support for translating content titles and descriptions by rubenwardy 20:12 Krock why can't the textdomain be constant? 20:12 rubenwardy games 20:12 Krock take the technical game name 20:12 Krock and if this is only in the main-menu, actual constants might be possible too if only one is loaded at the time 20:13 Krock although I must admit that I'm not too familiar with the translation domains 20:14 Krock it just seemed like unneeded additional effort 20:14 sfan5 why not have a game/minetest_game/game.XX.tr 20:14 Krock (for modders to specify a name) 20:14 MTDiscord v-rob: debug.setmetatable(nil, {__index = nil}) does next to nothing, you'd have to use __index = function() return nil end to silence nil index errors (for all mods); nevertheless, debug functions are useful for what their name implies: writing good debuggers - it's a shame MT modders don't leverage this and instead often (ab)use chat messages, printing or logging 20:14 rubenwardy the point of using .tr is to allow the translations to be in one file, which helps when distributing to translators 20:15 rubenwardy I was thinking about using game/minetest_game/locales/minetest_game.XX.tr instead, might require adding that as a media path 20:15 sfan5 it's only one file if you have exactly one mod in the game 20:15 sfan5 you don't need it in a media path if it's only for the content thing 20:15 rubenwardy you do if you're using other text from the file 20:16 sfan5 well yes 20:16 sfan5 I was thinking a game.XX.tr that is solely to locale stuff in game.conf 20:16 sfan5 (is that even where the description goes?) 20:16 rubenwardy yeah 20:17 rubenwardy well, adding`textdomain` was pretty simple - it's just another key in the game/mod.conf 20:17 rubenwardy the problem comes from finding the textdomain in a modpack or game 20:18 rubenwardy if you require it to be at a fixed path, at the top-level of the game/modpack, that becomes easier 20:19 rubenwardy well I mean it's not that much of a problem, it's like 5 lines 20:19 v-rob[m] "it's like 5 lines" -- famous last words 20:19 rubenwardy https://github.com/minetest/minetest/pull/12208/files#diff-634c1c6ad28b8e5a9f60adacec383a160704209ec7719b57781fcb8ca7d7a226R220-R232 20:19 rubenwardy 5 lines until bugs are found I guess 20:20 sfan5 I guess ruben can decide what do about it 20:20 sfan5 >>next>> #12185 20:20 ShadowBot https://github.com/minetest/minetest/issues/12185 -- Add login/register dialog to separate register and login by rubenwardy 20:20 rubenwardy haha 20:21 sfan5 conceptually, I dunno 20:21 sfan5 it adds an extra step to every login flow even if you already have an account 20:21 rubenwardy an alternative implementation would have the login box on the Join Game menu, with Register/Login below 20:21 rubenwardy same number of steps for both register and login in that case 20:21 Krock separate Register button seems more logical 20:21 rubenwardy just the confirm password dialog is replaced with a create account dialog 20:22 rubenwardy this is the design preferred by Zughy 20:22 sfan5 possible tradeoff: direct username password box for servers in the favorite list but not others? 20:22 sfan5 though that'd be a trap waiting to be stumbled into by a new user 20:22 Krock then why not a login box for all of them? 20:23 rubenwardy yeah, I think for all of them is simpler 20:23 rubenwardy just finding the mock up 20:23 sfan5 with "Login" and "Register" btns you mean? 20:23 Krock I generally like the two password inputs in a single formspec 20:23 sfan5 otherwise it doesn't differ from the current way at all 20:23 rubenwardy so: 20:23 rubenwardy login: enter username/password, click login 20:23 rubenwardy register: click register, see form with username/password/password2 20:23 MTDiscord I don't like complicating Login flows as long as we don't have a password manager 20:24 rubenwardy if they enter username/password then click register, it should prefill on the register dialog 20:24 sfan5 now that sounds okay 20:24 sfan5 where do you move the button to delete favorites? 20:25 MTDiscord what about the current flow of logging in with a nonexistent user? 20:25 rubenwardy that's another question to ask - I'm thinking of replacing it with a star 20:25 rubenwardy x2048: I have WIP code that exits with an error 20:25 Krock hmm.. replace the buttons with icons to save space? 20:25 Krock at least the "delete favourite" one 20:25 rubenwardy I think a star is obviously favourite, because it matches the serverlist 20:25 v-rob[m] So, make the star a toggle button? That makes sense 20:25 rubenwardy yea 20:27 sfan5 really cool would be drag-drop lists 20:27 MTDiscord that would break the today's easy registration flow for existing users trying new servers 20:27 sfan5 but that is too advanced fro minetest 20:27 Krock so I did submit the few wiki notes and will take my leave now. Thanks for participating in the meeting. 20:28 sfan5 @x2048 it neither adds nor removes a step, I don't get it 20:28 rubenwardy maybe if you forget whether you have an account or not 20:28 rubenwardy that's the same with any website though 20:29 MTDiscord sfan5, rubenwardy suggested that logging in with a nonexistent user fails with an error. 20:29 sfan5 well you'd click the register button instead 20:31 MTDiscord sfan5 that requires me to remember which button to click 20:31 sfan5 ¯\_(ツ)_/¯ 20:31 rubenwardy It would be pretty easy to add a setting if this is a big complete. I already have a mode which allows any connect, for CLI --go 20:31 sfan5 maybe we can add a setting, dunno 20:32 rubenwardy no idea where complete came from 20:32 rubenwardy *problem 20:33 rubenwardy well, it sounds like the idea is supported - I'll make the changes and then a setting can be discussed 20:33 rubenwardy As it's been almost an hour, might be worth wrapping up 20:33 rubenwardy well, I'm not limited on time 20:33 sfan5 I'd rather we do it through the end than stop now 20:33 rubenwardy sure ok 20:33 sfan5 next is apparently #12040 20:34 ShadowBot https://github.com/minetest/minetest/issues/12040 -- THST library; helper PR for WIP spatial indexing improvements by JosiahWI 20:34 sfan5 adds a library but nothing else, I can't tell if it's WIP 20:34 sfan5 should be closed 20:34 sfan5 also question is do we want to vendor this but that should be answered when the connected PR (11973) is ready 20:34 MTDiscord https://github.com/minetest/minetest/pull/11973#issuecomment-1027340650 was suggested to be separate pr 20:35 rubenwardy I think consensus is needed on whether we should require a spatial lib for entity lookups 20:36 sfan5 @Jonathon having a separate PR that rips out spatialindex and replaces it with THST (as a first step) would be potentially useful 20:36 sfan5 a PR that just adds a folder is useless on its own 20:37 sfan5 of course that should be in a separate commit in the end, but that doesn't mean a separate PR 20:38 sfan5 more accurately s/useless/pointless/ 20:39 rubenwardy yeah makes sense to me 20:39 MTDiscord lhofhansl suggested separating it from 11973 20:39 rubenwardy It's worth confirming the lib before asking for more work though 20:39 rubenwardy I haven't looked into libraries 20:40 MTDiscord rubenwardy, what do you mean by confirming? 20:40 rubenwardy do we want to switch to THST? 20:40 sfan5 looking into alternatives, deciding whether we want to use it 20:41 rubenwardy yeah 20:41 MTDiscord Re: Rubenwardy's translation thing, it was pretty easy to add support for NodeCore since I can just use the existing translation infrastructure. If I have to support multiple files then it becomes a lot messier. As a game maker I prefer it the way Ruben did it already. 20:43 MTDiscord I mean, the fact that I already added support and am just waiting for my translators is testament to it being pretty workable for game devs. 20:43 sfan5 >> next >> #12128 20:43 ShadowBot https://github.com/minetest/minetest/issues/12128 -- Add bulk ABMs by TurkeyMcMac 20:43 rubenwardy Warr1024: thanks 20:43 sfan5 not sure why it's tagged "Help needed", but it's WIP in any caser 20:44 sfan5 -r 20:44 sfan5 concept ok for me 20:44 MTDiscord rubenwardy, sfan5, 11793 has a justification for THST 20:44 rubenwardy I don't have big opinions on this one. Concept ok, would like to see benchmarks if it's for performance reasons 20:45 MTDiscord Bulk ABMs would offer some interesting opportunities for performance but it would require adapting logic specifically to them. Just naively porting existing ABMs to use the bulk API wouldn't net any gain. They need to specifically do vmanip type things. Hence it makes sense to keep it an explicitly separate API. 20:46 MTDiscord Personally I'm still struggling with ABM stuff a lot and have considered and rejected or shelved a ton of ideas for optimizing them, and seen a number of experiments end in no improvement beyond learning what doesn't work... 20:47 sfan5 it's already concept approved so no idea why we're discussing this 20:47 rubenwardy it was requested 20:47 rubenwardy I don't know if you've had the meeting yet, but if you haven't, perhaps you could discuss #12128. There are some unresolved questions in the Todo section of the PR. 20:47 ShadowBot https://github.com/minetest/minetest/issues/12128 -- Add bulk ABMs by TurkeyMcMac 20:47 rubenwardy sorry, should have added that to the notes 20:47 sfan5 questions 20:47 sfan5 i dont see questions 20:48 sfan5 implementation questions maybe but nothing we can discuss in meeting well 20:48 rubenwardy they're not even here 20:48 rubenwardy let's continue 20:48 sfan5 >> next >> https://github.com/minetest/minetest/issues/8601#issuecomment-1107857690 20:48 sfan5 (proposal) 20:49 sfan5 sounds good 20:49 rubenwardy cool 20:50 MTDiscord what would be the behavior in 5.6.0? 20:50 rubenwardy a deprecation warning 20:51 MTDiscord ah, yes 20:51 rubenwardy I'm not 100% the deprecation period is needed, but we might want a full release cycle to improve the deps resolver 20:52 rubenwardy eh idk 20:53 MTDiscord I don't think the proposal solves Vanessa's problem 20:53 rubenwardy It does 20:53 rubenwardy The problem will be solved by Minetest failing to load, and the user having to disable the unfilled mod 20:54 rubenwardy also, I've already written the PR for the hard error: #8059 20:54 ShadowBot https://github.com/minetest/minetest/issues/8059 -- Add fatal error on missing dependency by rubenwardy 20:54 MTDiscord The expectation is ABCD loaded, not a crash. 20:55 MTDiscord But a good crash (telling what to fix) might be a better solution 20:56 rubenwardy there's no crash currently, it disabled all the mods 20:56 rubenwardy and my proposal adds a graceful error 20:56 MTDiscord I'm thinking a scenario when E gains hard dependency on F on an upgrade. 20:57 MTDiscord Then a graceful error telling to add F is better than E vanishing 20:57 rubenwardy yeah, the proposal does that 20:58 rubenwardy I had the same issue on CTF in 2019, it broke moderation/security tools 20:58 rubenwardy by implicitly disabling them 20:58 MTDiscord Yes, it's good. Better that the described expectation. 20:58 MTDiscord *than 20:59 sfan5 >> next >> #12092 20:59 ShadowBot https://github.com/minetest/minetest/issues/12092 -- Calling set_detach() + set_properties() + set_pos() on the same step fails to teleport the player half of the time, especially online 20:59 sfan5 needs someone with time to look at it 20:59 sfan5 nothing to discuss here 20:59 rubenwardy fair I guess 21:00 MTDiscord network protocol race? 21:00 sfan5 probably 21:01 sfan5 >> next >> "Should we start assessing old PRs under the roadmap / assigning people?" 21:02 sfan5 someone already started doing this (including some of the PRs we discussed in this meeting) so I guess everyone agrees with it 21:02 MTDiscord Random off-topic note, dunno if anyone opened an issue or fixed this yet: there is a package download state for queue when more than max downloads are occurring and its texture is missing 21:03 rubenwardy I don't recall that being reported, open an issue? 21:03 MTDiscord That needs a github issue with a screenshot 21:03 MTDiscord Will get to it when I can 21:04 MTDiscord sfan5 assessing old PRs - is there a list? 21:04 MTDiscord click on the last page and work backwards? 21:04 rubenwardy https://github.com/minetest/minetest/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc probably 21:04 rubenwardy ninjad 21:04 sfan5 https://github.com/minetest/minetest/pulls?q=is%3Aopen+is%3Apr+-label%3ARoadmap+-label%3A%22Supported+by+core+dev%22+sort%3Acreated-asc 21:05 rubenwardy also -label:Bugfix 21:05 sfan5 yeah 21:05 rubenwardy https://github.com/minetest/minetest/pulls?q=is%3Aopen+is%3Apr+-label%3ARoadmap+-label%3ABugfix+-label%3A%22Supported+by+core+dev%22+sort%3Acreated-asc+ 21:06 rubenwardy feels like something that we can start outside of meetings, and finish up in the next one 21:06 sfan5 also -label:Maintenance 21:06 rubenwardy oh yeah 21:06 sfan5 yes 21:06 rubenwardy well, some maintenance PRs can be feature-y 21:06 rubenwardy but generally can be exempt 21:07 rubenwardy ok, happy with that one. Is that it now? 21:07 sfan5 >> next >> Release in June? 21:08 rubenwardy rough target really 21:09 sfan5 january plus six would be July 21:09 sfan5 if we want to do that we should use the next meeting to prioritise some PRs goals 21:09 Zughy[m] you can at least add the "Bug" label to #12092, there are 3 people confirming it 21:09 ShadowBot https://github.com/minetest/minetest/issues/12092 -- Calling set_detach() + set_properties() + set_pos() on the same step fails to teleport the player half of the time, especially online 21:09 rubenwardy good point 21:09 rubenwardy to both 21:10 sfan5 s|PRs goals|PRs and goals| 21:10 rubenwardy yeah, having that in the next meeting makes sense 21:10 sfan5 plus there are existing ones in here https://github.com/minetest/minetest/milestone/19 21:10 sfan5 >> next >> (I made this up on the spot) how are the opinions on adding a "Regression" label for issues? 21:11 rubenwardy that sounds like a good idea 21:11 rubenwardy regression labels should also be added by default to the milestone 21:11 rubenwardy although we do that already 21:11 sfan5 yes 21:12 sfan5 I usually add to the milestone if something is a regression 21:12 sfan5 but this should really be a label 21:12 rubenwardy yeah makes sense 21:12 MTDiscord did not know we did nit have it :) 21:12 MTDiscord *not 21:13 MTDiscord Do we just add issues/prs to the milestone for the next meeting? 21:13 sfan5 things to discuss next meeting are on the wiki, if you meant that 21:14 MTDiscord Plus sfan5 you made a issue to collect input for 5.5.0, would you do it again? 21:15 sfan5 that was intended as an one time thing for 5.5 but we can make it regular 21:15 sfan5 sure 21:16 MTDiscord I'm thinking about the list of PRs to discuss for 5.6.0, do add them to the milestone first, make our private lists or use wiki? 21:17 sfan5 if it's for a meeting I'd make my own list, for prolonged discussion an issue like you said would be better 21:17 MTDiscord ok, thanks 21:19 rubenwardy ok, so next meeting in two weeks? 2022-05-08 21:19 sfan5 sounds okay 21:19 rubenwardy I'll create an org post, can always move it 21:19 MTDiscord How about my PR #11632 ? I made changes which @appgurueu requested for already 4 months ago. 21:19 sfan5 lastly everyone please (as time permits) look at this list https://github.com/minetest/minetest/pulls?q=is%3Aopen+is%3Apr+label%3A%22One+approval%22 21:19 ShadowBot https://github.com/minetest/minetest/issues/11632 -- lua_api.txt: more detailed documentation of 'set_attach' method. by Andrey2470T 21:19 MTDiscord So what do you think about that PR? 21:22 rubenwardy reviewing now, but outside the meeting 21:22 rubenwardy concept seems ok 21:28 MTDiscord Fixed the package queue issue myself #12218 21:28 ShadowBot https://github.com/minetest/minetest/issues/12218 -- Fix invalid queued package element and path by GreenXenith 21:28 MTDiscord The PR is trivial, just a bit more detailed info about each of parameters passed in 'set_attach' and also the bug is documented when it is necessary to multiply by 10 the child relative position 21:31 rubenwardy argh, I need slower internet to test that, GreenXenith 21:31 rubenwardy or I suppose I could lower the limit 21:31 rubenwardy that would be the clever thing to do 21:31 MTDiscord I just changed the else tree 21:31 rubenwardy hard when my UI is french 21:31 MTDiscord if false then if true 21:31 rubenwardy cool, works 21:35 rubenwardy sacré bleu! Je ne sais pas où je suis 21:36 rubenwardy I was testing translations the other day for a side project, using Chrome in Chinese was very painful. Ended up using Google Translate's camera feature 21:37 rubenwardy Merging #11870 and #12218 in 10 21:37 ShadowBot https://github.com/minetest/minetest/issues/11870 -- DevTest: Add more test weapons and armorball modes by Wuzzy2 21:37 ShadowBot https://github.com/minetest/minetest/issues/12218 -- Fix invalid queued package element and path by GreenXenith 21:59 rubenwardy pushing trivial bug fix in 10: https://github.com/rubenwardy/minetest/commit/7198c111ea0619ea235aff6a7cf8471e0cbb2021 22:00 pgimeno I've just read about this new image format, dropping the link here in case it's of interest to the project: https://qoiformat.org/ 22:03 pgimeno the "similar size of PNG" claim is probably disputable, but someone in the love2d forums claims that his Lua + FFI implementation beats love2d's built-in, C or C++ PNG decoding speed 22:03 pgimeno https://love2d.org/forums/viewtopic.php?f=5&t=92841 for reference 22:59 MTDiscord Decoding speed is not an issue with minetest