| Time |
Nick |
Message |
| 00:09 |
|
kilbith joined #minetest-dev |
| 04:00 |
|
MTDiscord joined #minetest-dev |
| 04:54 |
|
lurker_8474 joined #minetest-dev |
| 04:56 |
|
lurker_8473 joined #minetest-dev |
| 05:01 |
|
calcul0n_ joined #minetest-dev |
| 06:49 |
|
cranezhou joined #minetest-dev |
| 07:16 |
|
lurker_8473 joined #minetest-dev |
| 08:22 |
|
Warr10240 joined #minetest-dev |
| 09:03 |
|
Warr1024 joined #minetest-dev |
| 09:20 |
|
Fixer joined #minetest-dev |
| 09:24 |
nrz |
sfan5, thanks for the review first. I fixed all points pointed by everyone, it make things clearly better, and you catched nice points 🙂 #12885 is now ready for a merge if you can take a last look 🙂 |
| 09:24 |
ShadowBot |
https://github.com/minetest/minetest/issues/12885 -- [NOSQUASH] Reduce exposure of various internals by nerzhul |
| 10:57 |
|
calcul0n joined #minetest-dev |
| 12:44 |
|
kilbith joined #minetest-dev |
| 12:44 |
|
kilbith joined #minetest-dev |
| 13:08 |
|
Taoki joined #minetest-dev |
| 13:15 |
|
kilbith joined #minetest-dev |
| 13:49 |
|
Desour joined #minetest-dev |
| 14:07 |
|
Eze14Mu joined #minetest-dev |
| 14:24 |
|
fluxionary joined #minetest-dev |
| 14:39 |
|
jwmhjwmh joined #minetest-dev |
| 15:21 |
|
proller joined #minetest-dev |
| 16:27 |
|
jwmhjwmh joined #minetest-dev |
| 17:05 |
|
linksword[m] joined #minetest-dev |
| 17:35 |
Krock |
meeting in 25 minutes |
| 17:47 |
|
olliy joined #minetest-dev |
| 17:49 |
|
jwmhjwmh joined #minetest-dev |
| 18:00 |
Krock |
nrz jwmhjwmh  rather quick meeting today |
| 18:01 |
Krock |
> #12772 |
| 18:01 |
ShadowBot |
https://github.com/minetest/minetest/issues/12772 -- Add `vector()` constructor by TurkeyMcMac |
| 18:02 |
Krock |
unfortunately Zughy does not seem to be available for this meeting |
| 18:03 |
sfan5 |
I'm here too |
| 18:03 |
Krock |
great |
| 18:04 |
Krock |
so apparently the topic is to decide whether or not we want to add this constructor? |
| 18:04 |
jwmhjwmh |
Yes. |
| 18:04 |
|
proller joined #minetest-dev |
| 18:04 |
Krock |
I have my concerns, because vectors are loosely defined; many times they are a simple table without a metatable |
| 18:06 |
Krock |
so what's been used are helper functions, without any class association |
| 18:06 |
Krock |
what's the goal in the future? to replace the old vector notation, or is it just about adding another shorthand command? |
| 18:07 |
jwmhjwmh |
I think it's good to have a declarative way to create vectors (vector.new has an imperative vibe IMO.) |
| 18:08 |
jwmhjwmh |
It's more consistent with stuff like ItemStack, though the constructor isn't overloaded. |
| 18:08 |
jwmhjwmh |
Like an item stack, a vector is conceptually a class of object. |
| 18:08 |
sfan5 |
adding that sounds fine to me |
| 18:08 |
Krock |
then why not make it equivalent to vector.new(), accepting other parameters too? |
| 18:09 |
jwmhjwmh |
Overloading it could be a good idea, although that functionality of vector.new is deprecated. |
| 18:09 |
Krock |
the three parameters x/y/z seems to be an artificial restriction where it could be as well made general-purpose like ItemStack where you can either pass strings or another itemstack ot copy |
| 18:09 |
jwmhjwmh |
That sounds good to me. |
| 18:11 |
sfan5 |
the constructors were split on purpose |
| 18:11 |
Krock |
also a version without arguments. that would then replace all of the other functions |
| 18:11 |
Krock |
what was the intend of splitting it? |
| 18:12 |
sfan5 |
performance, I think |
| 18:12 |
sfan5 |
Desour will more and/or check the relevant PR(s) |
| 18:12 |
sfan5 |
know more* |
| 18:13 |
Desour |
performance, and more importantly readability. vector.new() is less readable than vector.zero() |
| 18:13 |
Desour |
iirc |
| 18:13 |
Krock |
at least for my part, I'd prefer slightly slower multi-purpose functions over cluttered API |
| 18:13 |
Desour |
performance isn't the main goal of the vector helpers anyway |
| 18:13 |
jwmhjwmh |
In LuaJIT traces overloading the function is basically zero cost, too. |
| 18:14 |
jwmhjwmh |
IMO vector() with no arguments would be a confusing constructor. |
| 18:14 |
Krock |
is ItemStack() Â with no arguments confusing? |
| 18:15 |
Desour |
Krock: is ItemStack() valid? if yes, what does it do??? |
| 18:15 |
Krock |
empty stack |
| 18:15 |
Desour |
ah, those exist |
| 18:15 |
Krock |
well I guess for vectors it's less obvious |
| 18:15 |
jwmhjwmh |
I guess it could be supported for consistency with C++. |
| 18:16 |
jwmhjwmh |
And 0 is "the default". |
| 18:16 |
Desour |
modders don't need consistency with c++ |
| 18:16 |
Krock |
right |
| 18:17 |
pgimeno |
I'm quite annoyed with vectors being a hash rather than an array, but that's not changeable now |
| 18:17 |
Desour |
does ItemStack(string) work btw? |
| 18:17 |
Krock |
pgimeno: which hash? |
| 18:17 |
Krock |
Desour: sure |
| 18:18 |
Krock |
that's 99% of the purposes for ItemStack, in first place. |
| 18:18 |
Desour |
and if yes, would we also want vector(string) ? (imo not nice api) |
| 18:18 |
pgimeno |
Krock: {x = 1, y = 2, z = 3} rather than {1, 2, 3} |
| 18:18 |
jwmhjwmh |
Perhaps vector() should be supported for consistency with ItemStack(). |
| 18:18 |
jwmhjwmh |
I think vector(string) is too much overloading. |
| 18:18 |
Krock |
pgimeno: with the new metatable format, both should be accessible, to my knowlege |
| 18:19 |
Desour |
v[num] is slow though |
| 18:19 |
Krock |
it also improves readability when you index by x/y/z in your code, rather than messing with indices |
| 18:19 |
pgimeno |
it seems I've missed a lot in the last few months then |
| 18:20 |
Krock |
jwmhjwmh: I'd suggest to alias vector() to vector.new(), after all it's a constructor, and new() is basically the constructor |
| 18:20 |
Krock |
so these two doing the same would make sense to me |
| 18:20 |
Desour |
jwmhjwmh: >In LuaJIT traces overloading the function is basically zero cost, too. Â Â do you have a source for that claim? I'd like to know more details |
| 18:21 |
jwmhjwmh |
Desour: traces specialize to types, and nil/false/true are all separate types. Checking if nil is truthy is a no-op in a trace. |
| 18:21 |
jwmhjwmh |
Krock: That would work, although vector.new(v) is deprecated I believe. |
| 18:22 |
jwmhjwmh |
I'll try to find a source on LuaJIT traces. |
| 18:22 |
Desour |
ah, I see thx |
| 18:23 |
jwmhjwmh |
Sort of covers it: https://en.wikipedia.org/wiki/LuaJIT#Tracing |
| 18:27 |
jwmhjwmh |
So, final thoughts on adding vector(v) and vector() constructors? |
| 18:27 |
jwmhjwmh |
And on the vector(x, y, z) constructor. |
| 18:27 |
sfan5 |
don't mind it either ¯\_(ツ)_/¯ |
| 18:28 |
Krock |
I don't mind the zero argument constructor, but would prefer to keep consistency with vector.new |
| 18:28 |
Krock |
not like that it would matter too much |
| 18:28 |
jwmhjwmh |
Should the overloaded functionality of vector.new be un-deprecated? |
| 18:28 |
Krock |
the API is already huge so there's not much impact |
| 18:29 |
Krock |
I'd be in favour of un-deprecating, but idk about others |
| 18:30 |
|
Desour joined #minetest-dev |
| 18:33 |
jwmhjwmh |
Anything for "One Approval" or "Roadmap: Needs Approval" PRs? |
| 18:35 |
Krock |
hmm |
| 18:39 |
Krock |
why does #12757 have the roadmap approval label? |
| 18:39 |
ShadowBot |
https://github.com/minetest/minetest/issues/12757 -- Add dynamic media without loading from disk by GoodClover |
| 18:39 |
Krock |
sfan5: what do you think about this PR? |
| 18:40 |
sfan5 |
rasonable |
| 18:40 |
sfan5 |
+e |
| 18:41 |
Krock |
yes, I think so too. thanks. I'll then remove the label |
| 18:42 |
Krock |
although I think it's rather a niche-case |
| 18:45 |
Krock |
and #12749 is being reviewed even though it's still pending for approval |
| 18:45 |
ShadowBot |
https://github.com/minetest/minetest/issues/12749 -- Move code style guidelines into repo by Desour |
| 18:45 |
Krock |
so I assume the label was just forgotten? |
| 18:47 |
Desour |
could I get roadmap approval for this great feature: #12720 |
| 18:47 |
ShadowBot |
https://github.com/minetest/minetest/issues/12720 -- [nosquash] Add a worlds_here.txt file in the worlds folder and update .gitignore by Desour |
| 18:49 |
Krock |
hmm I see. the worlds/ directory was not generated |
| 18:50 |
Krock |
I don't like the wording but it seems useful, thus removing the label. |
| 18:51 |
Desour |
thx |
| 18:56 |
Krock |
so if there's nothing more, we can end this meeting earlier then usual. thanks for attending. |
| 18:58 |
sfan5 |
sure |
| 18:59 |
sfan5 |
one thing: it'd be good if someone could review #12844 so we can get it in, it's quite big |
| 18:59 |
ShadowBot |
https://github.com/minetest/minetest/issues/12844 -- DevTest: Remove `experimental`, major refactor by Wuzzy2 |
| 19:01 |
nrz |
sorry team i was with children, this schedule is not so compatible with me 😒 |
| 19:01 |
nrz |
i have nothing special to add, all sounds said 🙂 |
| 19:02 |
nrz |
i just want to continue working on the refactor to help the code to  be more maintenable and possibily performant, i have no special feature for now to propose |
| 19:02 |
nrz |
(maybe some SAO new lua calls at a point, but not ready yet) |
| 19:04 |
Krock |
sfan5: I had it opened a few times and the size is indeed scary. seems like a short playtest should be fine for this. it's devtest after all... |
| 19:08 |
sfan5 |
yes |
| 19:09 |
Krock |
is this to quash or not? |
| 19:10 |
Krock |
the commits are very clean. would be a shame to squash |
| 19:32 |
|
jwmhjwmh joined #minetest-dev |
| 19:48 |
sfan5 |
merging #12844, #12889, #12891 in 10m |
| 19:48 |
ShadowBot |
https://github.com/minetest/minetest/issues/12844 -- DevTest: Remove `experimental`, major refactor by Wuzzy2 |
| 19:48 |
ShadowBot |
https://github.com/minetest/minetest/issues/12889 -- Check `sizeof(int)` and `sizeof(size_t)` by TurkeyMcMac |
| 19:48 |
ShadowBot |
https://github.com/minetest/minetest/issues/12891 -- Fix some outdated stuff about falling node docs by Wuzzy2 |
| 19:59 |
|
proller joined #minetest-dev |
| 19:59 |
proller |
and this plz  https://github.com/minetest/minetest/pull/12142 |
| 20:21 |
sfan5 |
https://i.imgur.com/Vykuorz.png haha no |
| 20:27 |
Krock |
is there currently someone paying for the pipelines? |
| 20:27 |
sfan5 |
I don't think so |
| 20:29 |
sfan5 |
looking at usage quotas there's a huge spike of what gitalb appears to be consider "CI minutes": 90 in Sept, 1400 in Oct so far despite the "Shared runner duration" remaining mostly constant |
| 20:31 |
sfan5 |
if I had to guess I'd say gitlab changes how they count CI use |
| 20:31 |
sfan5 |
changed |
| 20:31 |
sfan5 |
* |
| 20:39 |
|
proller joined #minetest-dev |
| 21:13 |
|
celeron55 joined #minetest-dev |
| 22:35 |
|
panwolfram joined #minetest-dev |
| 23:04 |
|
HuguesRoss joined #minetest-dev |
| 23:34 |
|
AliasAlreadyTake joined #minetest-dev |
