Time |
Nick |
Message |
00:15 |
MisterE[m] |
<erle> "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 |
<Jonathon> 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 |
<Jordach> i think i have an educated guess on the digging any node with lag bug |
00:38 |
MTDiscord |
<Jordach> probably due to a race condition between the DB manager and lua mod protection |
00:38 |
MTDiscord |
<Jordach> minetestserver (and the DB) by default doesn't give two shits about whether the node has protectiion |
00:39 |
MTDiscord |
<Jordach> 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 |
<Jordach> 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 |
<Jordach> 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 |
<Jordach> basically |
01:11 |
MTDiscord |
<Jordach> 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 |
<Jordach> 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 |
<Jordach> 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 |
<Jordach> 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 |
03:10 |
|
Wuzzy joined #minetest-dev |
04:00 |
|
MTDiscord joined #minetest-dev |
05:54 |
|
calcul0n joined #minetest-dev |
07:27 |
|
YuGiOhJCJ joined #minetest-dev |
08:14 |
|
Fixer joined #minetest-dev |
08:15 |
|
Fixer_ joined #minetest-dev |
08:27 |
|
Fixer joined #minetest-dev |
10:25 |
|
MTDiscord1 joined #minetest-dev |
12:10 |
|
Oblomov joined #minetest-dev |
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 |
<Jonathon> @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:16 |
|
Alias joined #minetest-dev |
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 |
<luatic> see my comment: What about '(([inventorycube{a.png{b.png{c.png))'? |
15:39 |
MTDiscord |
<luatic> I believe the shotgun parser comes back to bite us now |
15:40 |
MTDiscord |
<luatic> 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 |
<Jonathon> wait, so to bybass krocks bug, one could just wrap the texture modifier in (()) ? |
15:43 |
MTDiscord |
<luatic> wsor: yes |
15:43 |
MTDiscord |
<luatic> just () should suffice too |
15:44 |
MTDiscord |
<Jonathon> 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 |
<luatic> :) |
15:44 |
sfan5 |
how many is "every affected mod" |
15:44 |
sfan5 |
one? |
15:45 |
MTDiscord |
<Jonathon> if i understood correctly, any mod that used [modifier is affected? |
15:45 |
MTDiscord |
<Jonathon> 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 |
<luatic> 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 |
<Jonathon> 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 |
<Jonathon> yes, so every single mod that uses those, has to wrap in () now |
15:47 |
MTDiscord |
<luatic> 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 |
<Jonathon> 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 |
<Jonathon> or multiple images being combined, etc |
15:48 |
Oblomov |
only if the _base_ is a [combine |
15:48 |
MTDiscord |
<Jonathon> yeah |
15:48 |
Oblomov |
if the images are combined starting from an actual png, no issue |
15:49 |
MTDiscord |
<Jonathon> 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 |
<Jonathon> wait, thats wrong, 5.5 servers would need to update |
15:50 |
MTDiscord |
<Jonathon> 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 |
<Jonathon> 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 |
<Jonathon> 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 |
<luatic> Oblomov: give me a minute to type 8-D |
15:53 |
MTDiscord |
<luatic> 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 |
<luatic> @wsor: don't patch this in sources! |
15:55 |
MTDiscord |
<Jonathon> ...in sources? |
15:55 |
MTDiscord |
<luatic> as in, don't start editing sources, replacing [combine with ([combine:...) |
15:55 |
MTDiscord |
<Jonathon> ....and why not? |
15:56 |
Oblomov |
only wrap [png: in (), so you can crash old clients ;-) |
15:56 |
MTDiscord |
<luatic> 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 |
<Jonathon> well yeah, that was my idea. however getting people servers to actually use it would be the issue, rather than patching the mod directly |
15:57 |
|
fluxionary joined #minetest-dev |
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 |
<luatic> 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:09 |
|
jwmhjwmh joined #minetest-dev |
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 |
<Warr1024> 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 |
<Warr1024> 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 |
18:39 |
|
HuguesRoss joined #minetest-dev |
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 |
<luatic> 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 |
<luatic> (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 |
|
Oblomov left #minetest-dev |
19:24 |
MTDiscord |
<luatic> Also game devs can already achieve a somewhat similar effect simply by increasing the saturation of their node textures. |
19:25 |
MTDiscord |
<luatic> (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 |
<luatic> 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 |
<luatic> 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 |
<Jonathon> 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 |
<luatic> ^ |
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 |
<luatic> 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 |
<luatic> 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 |
<luatic> v-rob: as long as we can't properly obtain the metatables: ys |
19:34 |
MTDiscord |
<luatic> 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 |
<luatic> 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 |
<Jonathon> 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 |
<luatic> 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 |
<luatic> 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 |
<Jonathon> @Warr1024 ^ |
19:48 |
MTDiscord |
<luatic> ah right, somebody just sticked serpent in there |
19:48 |
MTDiscord |
<Jonathon> 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 |
<Jonathon> 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 |
<GoodClover> 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 |
<GreenXenith> 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 |
<GreenXenith> Awesome, thank you |
20:09 |
MTDiscord |
<GreenXenith> 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 |
<luatic> 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 |
<luatic> 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 |
|
Taoki_1 joined #minetest-dev |
20:24 |
sfan5 |
now that sounds okay |
20:24 |
sfan5 |
where do you move the button to delete favorites? |
20:25 |
MTDiscord |
<x2048> 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 |
<x2048> 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 |
<x2048> 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 |
<x2048> 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 |
|
Fixer_ joined #minetest-dev |
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 |
<Jonathon> 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 |
<x2048> 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 |
<x2048> 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 |
<Warr1024> 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 |
<Warr1024> 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 |
<x2048> 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 |
<Warr1024> 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 |
<Warr1024> 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 |
<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. |
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 |
|
Taoki joined #minetest-dev |
20:48 |
sfan5 |
(proposal) |
20:49 |
sfan5 |
sounds good |
20:49 |
rubenwardy |
cool |
20:50 |
MTDiscord |
<x2048> what would be the behavior in 5.6.0? |
20:50 |
rubenwardy |
a deprecation warning |
20:51 |
MTDiscord |
<x2048> 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 |
<x2048> 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 |
<x2048> The expectation is ABCD loaded, not a crash. |
20:55 |
MTDiscord |
<x2048> 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 |
<x2048> I'm thinking a scenario when E gains hard dependency on F on an upgrade. |
20:57 |
MTDiscord |
<x2048> 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 |
<x2048> Yes, it's good. Better that the described expectation. |
20:58 |
MTDiscord |
<x2048> *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 |
<x2048> 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 |
<GreenXenith> 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 |
<x2048> That needs a github issue with a screenshot |
21:03 |
MTDiscord |
<GreenXenith> Will get to it when I can |
21:04 |
MTDiscord |
<x2048> sfan5 assessing old PRs - is there a list? |
21:04 |
MTDiscord |
<Jonathon> 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 |
<x2048> did not know we did nit have it :) |
21:12 |
MTDiscord |
<x2048> *not |
21:13 |
MTDiscord |
<x2048> 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 |
<x2048> 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 |
<x2048> 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 |
<x2048> 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 |
<Andrey01> 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 |
<Andrey01> So what do you think about that PR? |
21:22 |
rubenwardy |
reviewing now, but outside the meeting |
21:22 |
rubenwardy |
concept seems ok |
21:26 |
|
AliasAlreadyTake joined #minetest-dev |
21:28 |
MTDiscord |
<GreenXenith> 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 |
<Andrey01> 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 |
<GreenXenith> I just changed the else tree |
21:31 |
rubenwardy |
hard when my UI is french |
21:31 |
MTDiscord |
<GreenXenith> 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 joined #minetest-dev |
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:32 |
|
panwolfram joined #minetest-dev |
22:59 |
MTDiscord |
<exe_virus> Decoding speed is not an issue with minetest |
23:15 |
|
fluxionary joined #minetest-dev |
23:23 |
|
AliasStillTaken joined #minetest-dev |