| Time |
Nick |
Message |
| 04:00 |
|
MTDiscord joined #minetest-dev |
| 04:07 |
|
erle joined #minetest-dev |
| 04:18 |
|
Pexin joined #minetest-dev |
| 05:39 |
|
calcul0n joined #minetest-dev |
| 08:24 |
|
Baytuch_2 joined #minetest-dev |
| 10:19 |
|
HuguesRoss48 joined #minetest-dev |
| 11:36 |
Zughy[m] |
why are there some lua setters returning true when they succeed (e.g. set_sky, set_look_vertical) and some others that don't (set_velocity, set_fov)? Which one should be the standard? It's like 50-50 |
| 11:37 |
Zughy[m] |
also, those booleans are not documented, at least not inside the very functions |
| 11:39 |
Zughy[m] |
(as a modding point of view, they're useless) |
| 11:39 |
|
Fixer joined #minetest-dev |
| 11:47 |
erle |
Zughy[m] “when they succeed”, i.e. they can fail? |
| 11:48 |
erle |
would be surprising for me if anything of that could fail |
| 11:48 |
erle |
i mean a bunch of stuff fails under memory pressure, but then you just get random nil values everywhere |
| 11:48 |
erle |
and not necessarily in a return value |
| 11:55 |
Zughy[m] |
<erle> "Zughy “when they succeed”, i.e..." <- yes, for instance if you ask to set whatever of a non existent player, it returns 0 `(if player == nullptr= return 0;` |
| 11:55 |
Zughy[m] |
*`nullptr) |
| 12:13 |
|
qur joined #minetest-dev |
| 12:23 |
MTDiscord |
<luatic> Zughy: It returns nothing for non-existent players |
| 12:24 |
Zughy[m] |
oh c'mon you got what I mean |
| 12:25 |
MTDiscord |
<luatic> Zughy: where is it documented that set_look_vertical returns true on success? |
| 12:25 |
MTDiscord |
<luatic> https://github.com/minetest/minetest/blob/837cea6b4a2315966b9670ae50cff9d3679bcff4/doc/lua_api.txt |
| 12:25 |
Zughy[m] |
it's not, which is what I said |
| 12:26 |
MTDiscord |
<luatic> That's fine. It allows us to axe the bools without breaking compat. |
| 12:27 |
MTDiscord |
<luatic> Zughy: Theoretically modders could wrap the setters with assert or the like to crash the server if something they rely on doesn't work. |
| 12:27 |
MTDiscord |
<luatic> Or log it, whatever |
| 12:29 |
Zughy[m] |
That's a very unpractical (and not elegant) way to check that |
| 12:29 |
Zughy[m] |
I'd check the player, not the function |
| 12:30 |
MTDiscord |
<luatic> I'm not saying this should be used to check the player object |
| 12:30 |
MTDiscord |
<luatic> I think the best way to deal with invalid player objects / player methods being called on objrefs would be to throw an error |
| 12:30 |
MTDiscord |
<luatic> But we can't do that for backwards compat |
| 12:31 |
MTDiscord |
<luatic> Anyways, I suppose there ought to be a different way the setters can fail? |
| 12:32 |
MTDiscord |
<luatic> It seems set_sky literally just pushes true whenever the player object is valid :thonkeyes: |
| 12:34 |
MTDiscord |
<luatic> Zughy: Please open an issue "setters return unneeded & undocumented booleans" |
| 12:41 |
Zughy[m] |
Done #12177 |
| 12:41 |
ShadowBot |
https://github.com/minetest/minetest/issues/12177 -- Unneeded setters return |
| 12:43 |
MTDiscord |
<luatic> Thank you |
| 12:43 |
MTDiscord |
<luatic> Now I can make a PR |
| 12:46 |
erle |
<MTDiscord> <luatic> That's fine. It allows us to axe the bools without breaking compat. |
| 12:46 |
erle |
i hate this idea with a passion |
| 12:47 |
erle |
it's not fine to leave something undocumented as in “nothing is written about it”. |
| 12:47 |
erle |
the least you could write is that the return value is undefined |
| 12:47 |
erle |
if you want to keep it that way |
| 12:47 |
erle |
C style |
| 12:51 |
Zughy[m] |
You may be right, but last time I've looked around, MT didn't look that organised as a project |
| 12:51 |
Zughy[m] |
luatic: oh thanks, I can focus on other things then :D |
| 12:54 |
erle |
Zughy[m] at least “the result of this operation is undefined” will serve as a help for future devs to figure out that *someone* at *some point* looked at this thing and it isn't some historical baggage. |
| 13:13 |
MTDiscord |
<luatic> I suppose the truthy return value allows us to add more validation in the future |
| 13:18 |
Zughy[m] |
no, it's just useless, don't go towards that path |
| 13:24 |
MTDiscord |
<luatic> This is such swinecode |
| 13:25 |
|
proller joined #minetest-dev |
| 13:25 |
MTDiscord |
<luatic> where do the set_look_something funcs push the boolean? I don't know |
| 13:25 |
MTDiscord |
<luatic> They all call setLookPitchAndSend, but that doesn't push the boolean either |
| 13:25 |
MTDiscord |
<luatic> I'm literally performing a breadth-first search here looking for the func that pushes the boolean |
| 13:26 |
Zughy[m] |
lol you were fast, congrats |
| 13:26 |
|
qur joined #minetest-dev |
| 13:37 |
MTDiscord |
<luatic> Zughy: I think you were wrong about set_look_vertical |
| 13:37 |
MTDiscord |
<luatic> I can't find a pushboolean there |
| 13:37 |
MTDiscord |
<luatic> It should return 0, not 1 though |
| 13:37 |
MTDiscord |
<luatic> Because it always returns nothing |
| 13:45 |
Zughy[m] |
yep, you're right, I'll use another example |
| 13:48 |
sfan5 |
> it's not fine to leave something undocumented as in “nothing is written about it”. |
| 13:48 |
sfan5 |
the api documentation is not a formal specification and should not be one |
| 13:49 |
sfan5 |
if something is not written down that's pretty much equivalent to 'you are not supposed to observe the return value' already |
| 13:50 |
MTDiscord |
<Warr1024> "if something is not written down" <-- just add this to the top of the documentation, problem solved :-D |
| 13:56 |
sfan5 |
re "Code quality template for issues #12178": code quality issues are bugs |
| 13:56 |
ShadowBot |
https://github.com/minetest/minetest/issues/12178 -- Code quality template for issues by Zughy |
| 14:01 |
Zughy[m] |
I don't agree: for instance, current HUD structure is very unpractical, but you can't say it's a bug. It's poorly designed, code quality is bad. Or let's say I suggest to split a very long script into 2-3 smaller ones. That's not a bug, it's about readability (so code quality) |
| 14:05 |
sfan5 |
structural or design issues are deficits in the code which is close enough to the bug template that it should just be used IMO |
| 14:16 |
|
fluxionary joined #minetest-dev |
| 16:02 |
|
Extex joined #minetest-dev |
| 16:38 |
|
appguru joined #minetest-dev |
| 18:36 |
|
Desour joined #minetest-dev |
| 19:13 |
|
Extex joined #minetest-dev |
| 19:17 |
|
proller joined #minetest-dev |
| 19:41 |
|
Fixer joined #minetest-dev |
| 20:32 |
|
Alias joined #minetest-dev |
| 21:50 |
|
troller joined #minetest-dev |
| 22:08 |
|
troller joined #minetest-dev |
| 22:33 |
|
panwolfram joined #minetest-dev |
| 22:58 |
|
proller joined #minetest-dev |
| 23:14 |
|
Extex joined #minetest-dev |
| 23:25 |
|
Yad joined #minetest-dev |
| 23:25 |
|
AliasAlreadyTake joined #minetest-dev |
| 23:49 |
|
proller joined #minetest-dev |
