| Time |
Nick |
Message |
| 03:28 |
MTDiscord |
<srinivas> heh, how did that number ome up? |
| 04:02 |
|
Taoki joined #minetest-dev |
| 04:52 |
|
olliy joined #minetest-dev |
| 06:43 |
|
m42uko joined #minetest-dev |
| 07:27 |
|
olliy joined #minetest-dev |
| 07:27 |
|
Flitzpiepe joined #minetest-dev |
| 08:00 |
|
ShadowNinja joined #minetest-dev |
| 08:32 |
|
lisac joined #minetest-dev |
| 08:49 |
|
pyrollo joined #minetest-dev |
| 09:49 |
|
pyrollo joined #minetest-dev |
| 09:58 |
|
NetherEran joined #minetest-dev |
| 10:04 |
|
calcul0n joined #minetest-dev |
| 10:31 |
|
proller joined #minetest-dev |
| 10:43 |
|
pyrollo joined #minetest-dev |
| 11:19 |
|
bebebeko joined #minetest-dev |
| 11:26 |
bebebeko |
Hey folks! tldr for my project i need some features in program, and i'm ready to sponsored these through bountysource, for example, https://github.com/minetest/minetest/issues/10593 At this moment the description looks ogly in my opinion. Could anyone suggest a way how i can improve it, or it's fine? |
| 11:29 |
bebebeko |
this is my first experience with bountrysource, please don't judge me harshly ;-) |
| 11:32 |
pmp-p |
hi bebebeko, there's not much to judge, as description of what you need is terribly incomplete is it client/server side ,static or dynamic (moving entities) ? |
| 12:29 |
bebebeko |
pmp-p: thank! I updated the issue, could you please verify, maybe I miss smth? https://github.com/minetest/minetest/issues/10593 |
| 12:45 |
|
Fixer joined #minetest-dev |
| 13:43 |
|
pyrollo joined #minetest-dev |
| 14:15 |
|
pyrollo joined #minetest-dev |
| 15:06 |
|
Taoki joined #minetest-dev |
| 16:13 |
Krock |
> Then when the mods have loaded and the server is stuck, interrupt with Control+C and enter thread apply all bt. |
| 16:13 |
Krock |
cool trick. I haven't used that in a while :D |
| 16:20 |
Krock |
when is lua_isnil() not the same as lua_isnoneornil ? |
| 16:21 |
rubenwardy |
when it is none |
| 16:21 |
rubenwardy |
none means that the index doesn't exist on the stack |
| 16:21 |
rubenwardy |
nil means that the index exists and is nil |
| 16:21 |
rubenwardy |
in func(1, nil), index 2 is nil but index 3 is none |
| 16:22 |
rubenwardy |
lua_isnoneornil should pretty much always be used |
| 16:22 |
rubenwardy |
over isnone or isnil |
| 16:22 |
Krock |
lua_isnil ensures that the arguments are provided, even if nil |
| 16:22 |
Krock |
so in terms of solid API that's a safer approach |
| 16:23 |
Krock |
perhaps |
| 16:23 |
Krock |
after all it might not matter because mostly the code would attempt converting this non-nil value to a number, which return 0 |
| 16:24 |
rubenwardy |
it makes the C++ API act differently to a Lua one, and you'd want to throw rather than treating none as a non-nil |
| 16:24 |
Krock |
that's what luaL_checknumber is for, no? |
| 16:25 |
rubenwardy |
yes, which is better to use over lua_isnone or lua_isnoneornil |
| 16:25 |
Krock |
thanks for the explanation |
| 16:25 |
rubenwardy |
my point is if an argument is nilable, none should be treated like nil. And if it's not nilable, you can use check* |
| 16:26 |
Krock |
so it's a question whether the arguments may be optional or not |
| 16:26 |
Krock |
and whether they may be nilable |
| 16:54 |
celeron55 |
i think there's a semantic difference between an optional argument and there being an option to pass something as nil |
| 16:54 |
sfan5 |
rubenwardy: (re 10368) how about ensuring that either the formspec version is bumped or the protocol version? |
| 16:55 |
rubenwardy |
it's weird from a Lua point of view though |
| 16:55 |
sfan5 |
what I'm thinking of is that it's not necessary to bump the protocol version if the formspec version has been bumped for this version |
| 16:55 |
rubenwardy |
the only way to tell from Lua is to do function(...) print(#{...}) end |
| 16:55 |
celeron55 |
of course you can define a style where one or the other is not allowed |
| 16:55 |
celeron55 |
and that's your convention then |
| 16:56 |
celeron55 |
is it? |
| 16:57 |
celeron55 |
in that case maybe it's better to not distinguish between them in C++ either |
| 16:57 |
Krock |
set_string("test") is less obvious than set_string("test", nil) |
| 16:58 |
Krock |
sometimes you want to enforce an argument to be provided |
| 16:58 |
celeron55 |
i don't care what's used as long as it's used consistently |
| 17:23 |
nerzhul |
is this me or celeron55 is now active as a coredev ? :D |
| 17:32 |
|
fluxflux joined #minetest-dev |
| 17:33 |
rubenwardy |
I'll believe it when I see a PR for VAEs |
| 17:33 |
rubenwardy |
out of the blue |
| 17:33 |
rubenwardy |
tbh, that probably isn't the highest priority rn :D |
| 17:42 |
celeron55 |
it would be so hacky you wouldn't accept it, trust me |
| 17:43 |
rubenwardy |
:D |
| 17:50 |
zughy[m] |
so wait, should I address Krock's reviews or not? About #10564 |
| 17:50 |
ShadowBot |
https://github.com/minetest/minetest/issues/10564 -- Fix ObjectRef crashes due to lua_isnil() by Zughy |
| 18:02 |
Krock |
zughy[m]: perhaps note what was the idea of doing these modifications? perhaps I missed an important part that's not mentioned |
| 18:17 |
|
Seirdy joined #minetest-dev |
| 18:23 |
|
olliy_ joined #minetest-dev |
| 18:24 |
|
Seirdy joined #minetest-dev |
| 18:56 |
zughy[m] |
Krock: ruben suggesting to change everything with isnoneornil supported by Desour https://github.com/minetest/minetest/pull/10564#issuecomment-717116073 |
| 19:23 |
Krock |
apparently I'm blind. somehow I only read the first few posts. it's a design decision whether lack of arguments is counted as "nil" too |
| 19:24 |
Krock |
my opinion is towards lua_isnil, also because a large part of the code uses that check. if other developers decide to go with isnoneornil, then please adjust the documentation accordingly |
| 19:32 |
zughy[m] |
yes, but they prevented crashes declaring the variable and its value first (i.e. int x = 0) and then checking if the index in the stack was nil. If it wasn't, they override x |
| 19:33 |
zughy[m] |
imho, using is_noneornil makes instantly explicit to whoever wants to contribute that none and nil are not the same thing in lua |
| 19:36 |
zughy[m] |
also, an exception are tables: when getting the field, lua_isnil works perfectly to check whether it exists |
| 19:37 |
zughy[m] |
that's why I didn't changed the whole set_sky/moon/etc. checks |
| 19:37 |
zughy[m] |
*change |
| 19:40 |
sfan5 |
functions that have optional arguments should use lua_gettop or lua_isnoneornil |
| 19:41 |
sfan5 |
functions that don't have that shouldn't be using lua_isnil, yet it seems many do |
| 19:41 |
sfan5 |
since this is kind of a mess I suggest doing the changes needed to restore compatibility and leaving the rest alone |
| 19:45 |
sfan5 |
in fact it almost seems like a good idea to revert the cleanup since who knows how much time has been wasted discussing and doing things that worked before and didn't bother anyone |
| 19:45 |
sfan5 |
(don't get me wrong I'm not actually in favor of a revert but I am annoyed at all the time and discussion that is now spent on this) |
| 19:49 |
zughy[m] |
(sad thing: the bridge is slow, I'm looking at your replies from the IRC log) |
| 19:49 |
zughy[m] |
I don't agree with the "it works, don't touch it". That code was a mess, like, a huge one. You may understand it as you've been cruising through it for years, but as a newcomer it's just a headache |
| 19:49 |
zughy[m] |
also: according to the docs, set_animation_framespeed should be 15.0 by default, but from l_object.cpp that's now what it happens. It pushes the value and a true value when there is a number and a false one where is none. Is that... normal? |
| 19:50 |
sfan5 |
lots of methods have an undocument boolean return value that indicates success |
| 19:51 |
sfan5 |
and regarding the default: the docs are lying and there is no such default |
| 19:52 |
sfan5 |
..perhaps that's supposed to be the default if not set, but the method argument is definitely mandatory |
| 19:53 |
rubenwardy |
yeah, that's the default if you don't call `set_animation_framespeed` |
| 20:02 |
pgimeno |
<rubenwardy> the only way to tell from Lua is to do function(...) print(#{...}) end <-- nooo! # is not guaranteed to work with embedded nil values. The way to do with lua is using select('#', ...) |
| 20:04 |
|
Darcidride joined #minetest-dev |
| 20:09 |
zughy[m] |
ok, I'll fix it later |
| 20:12 |
pgimeno |
I think the right thing to do in general, is to use isnoneornil when there's no default value for the parameter but nil is a valid value, and isnil when there's a default and it is something other than nil. |
| 20:14 |
rubenwardy |
the right thing to do is not care about nil vs none in the C++ api |
| 20:15 |
rubenwardy |
because Lua itself rarely cares about nil vs none |
| 20:17 |
pgimeno |
sorry, my experience is with the Löve API, e.g. https://love2d.org/wiki/love.graphics.draw - values in parentheses are defaults |
| 20:22 |
pgimeno |
it is rare that nil values are accepted as distinct parameters, actually |
| 20:23 |
pgimeno |
here's a somewhat delicate case: VoxelManip:write_to_map(bool), defaults to true - what should nil do? |
| 20:25 |
pgimeno |
IMO VoxelManip:write_to_map(nil) should be treated as false, distinctly from VoxelManip:write_to_map() |
| 20:47 |
zughy[m] |
PR is ready if you want Krock |
| 20:48 |
red-001 |
btw if you want me to I can change the function to pascalCase |
| 20:48 |
red-001 |
camelCase* |
| 20:48 |
red-001 |
for #10589 |
| 20:48 |
ShadowBot |
https://github.com/minetest/minetest/issues/10589 -- Block attempts to connect to the client. by red-001 |
| 20:49 |
Krock |
will merge #10589 in 10 minutes |
| 20:49 |
ShadowBot |
https://github.com/minetest/minetest/issues/10589 -- Block attempts to connect to the client. by red-001 |
| 20:50 |
Krock |
zughy[m]: looks good, didn't test yet |
| 20:51 |
Krock |
strictly spoken the lua_api.txt would now need an update to properly state optional arguments, though. |
| 20:52 |
Krock |
will test tomorrow |
| 21:20 |
Krock |
merging... |
| 22:07 |
nore |
hmmm, I can't find the place where we said why LuaJIT was dangerous to use in (SS)CSM? |
| 22:08 |
nore |
also, does anyone want to review #10451? (at least reviewing the code, I'll work on testing a bit more) |
| 22:08 |
ShadowBot |
https://github.com/minetest/minetest/issues/10451 -- Client-side translations: gettext support by Ekdohibs |
| 22:10 |
nore |
btw, another question about this PR: it does require a translation update in all languages if we want to get plurals working, maybe weblate changes should be merged? or the PR merged, weblate updated, and then getting all translations from weblate after I update them all with the translation of the plurals? |
| 22:28 |
red-001 |
I think it's just a generic concern about JIT implementations generating code and being more complex -> more edge cases that code result in exploitable bugs |
| 22:29 |
red-001 |
s/code/could |
| 22:30 |
|
proller joined #minetest-dev |
