Time Nick Message 06:42 paradust Is there a way to expose a core function to lua so that it can be used by builtin/, but is not visible to mods / games ? 06:43 paradust oh, guess I could just save it to a local and nil the global 07:16 sfan5 thats the way yes 09:16 MTDiscord That's one way. A cleaner way would probably passing a table with functions private to builtin around when loading files. 09:16 MTDiscord probably be* 14:23 appguru My serialization redo PR removed a check that would silence incorrect types being passed to minetest.deserialize, which is now triggering bugs in poorly written mod code: https://github.com/runsy/petz/issues/105#issuecomment-1153975021. Note that the handling of non-string types was not documented (and I consider throwing clean here). Should I make a PR to preserve backwards compat at all costs? 14:24 appguru What about a PR that would only preserve backwards compat for `nil` strs? Passing userdata, tables, numbers, functions or bools to minetest.deserialize should surely throw? 14:26 appguru https://github.com/runsy/petz/issues/105#issuecomment-1153991391 alright, seeing as runs is passing a table, this is clearly a bug in his code which the redo just surfaced 14:26 appguru I thus maintain that this increase in strictness (rather than possibly unexpected behavior) was for the better. 14:27 erle appguru did this show a warning before? 14:27 appguru erle: no 14:27 appguru I would add a warning and a note in the docs if I were to restore backwards compat 14:27 erle if runs code worked without a warning, randomly breaking it is a bit of a dick move, even if it is arguably incorrect. 14:28 appguru erle: the code was broken 14:28 appguru he is deserializing twice 14:28 appguru my PR surfaced the bug 14:28 appguru so that's a good thing 14:28 erle i'd add a warning nevertheless, then make it strict some time later. 14:28 appguru because his persistence never worked and he probably didn't even know it 14:28 erle but you have to get the warning entirely correct (i.e. if it does not warn, the code must be correct) 14:29 erle if it works as well as the warning about transparency in textures you'll have no problems making it super strict a year later 14:30 erle given that people accidentally rely on all kinds of bullshit without realizing it, giving them a head start seems like good API stewardship 14:30 erle unless there is a security issue involved, then make it as tight as possible obv 14:32 appguru erle: runs has fixed the bug now 14:32 appguru I wonder whether he would've bothered fixing a warning spam? 14:38 erle no idea. but the only warning i have seen fail is the one about dynamic media – and the reason was that it was phrased incorrectly (no mention of blocking vs non-blocking) and a wrong fix made the warning disappear (an empty callback function). 14:38 erle everything else leads to people sooner or later fixing it. more later than sooner. 14:38 erle appguru any cases besides runs case? 14:38 appguru erle: no, not yet 14:39 erle is there a unit test covering that code path? 14:40 erle i.e. calling the function with nil? 14:40 MTDiscord erle: no, and I certainly won't add one 14:40 erle why not? almost all tests in real-world software concern the unhappy path 14:41 erle also that way you could make sure it does not get enabled again in a future refactoring 14:43 MTDiscord anyways, here's an (untested) patch 14:43 MTDiscord https://cdn.discordapp.com/attachments/747163566800633906/985917438153592852/Restore_deserialize_backwards_compat.patch 15:50 Pexin note about petz: last week(?) I was trying to repro a thing (and failing) but t 15:50 Pexin pony's death func crashed to menu 15:51 Pexin on master 15:51 Pexin had to go back to 5.5.0 16:11 appguru Pexin: That's exactly the bug we were talking about. Runs has since fixed it. 16:12 Pexin ah 16:24 sfan5 merging #12422, #12428 in 5m 16:24 ShadowBot https://github.com/minetest/minetest/issues/12422 -- Fix permissions on workflow-generated macOS builds by sjml 16:24 ShadowBot https://github.com/minetest/minetest/issues/12428 -- Improve lua_api.txt a bunch by sfan5 16:28 appguru sfan5: while you're at it you could merge #12423 16:28 ShadowBot https://github.com/minetest/minetest/issues/12423 -- Docs: add missing backticks by Zughy 16:29 appguru it's trivial too and the two other missing backticks were fixed as well now :) 16:29 sfan5 I will if it doesn't become conflicted after I merge mine 16:31 appguru good 16:31 appguru chances that it conflicts are pretty low if you ask me 16:32 sfan5 oh right I thought I added an identical missing backtick, but I didn't 17:15 MTDiscord Appguru should write up a lua linter for us to run on the lua_api.txt 17:16 MTDiscord Seems like the kind of person that might enjoy it 17:17 MTDiscord nah, MD linting is a pesky topic :P this backtick thing was easy to approximate decently tho 17:20 MTDiscord Right, just do the low hanging fruit, better than nothing 17:20 MTDiscord Could be in any language really 17:20 MTDiscord Just thinking aloud 17:58 sfan5 what was the policy on feature checks again 17:58 sfan5 it's impossible to detect the presence of set_lighting without getting ahold of a player object 18:02 Pexin are conflicts automatically flagged in existing PRs, or only when commits are pushed to them? 18:02 sfan5 flagged = ? 18:03 Pexin like, the red X appears 18:03 sfan5 yes 18:03 Pexin ok 18:03 sfan5 the 'Rebase needed' label is manual labor 18:04 sfan5 @Luatic: http://sprunge.us/A2LpEc?diff look good? 18:24 MTDiscord Hmm yes, I guess so. I'd probably shorten this to assert(ItemStack"".add_wear_by_uses, "errmsg") but it's fine either way. 18:26 sfan5 pushing then 19:39 sfan5 are set_properties calls cached yet 19:40 sfan5 hmm no