| Time |
Nick |
Message |
| 00:10 |
|
v-rob joined #minetest-dev |
| 00:37 |
|
Alias2 joined #minetest-dev |
| 01:29 |
|
v-rob joined #minetest-dev |
| 02:30 |
|
tekakutli joined #minetest-dev |
| 03:28 |
|
queria joined #minetest-dev |
| 03:34 |
|
queria joined #minetest-dev |
| 04:06 |
|
v-rob joined #minetest-dev |
| 04:24 |
|
v-rob joined #minetest-dev |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 05:53 |
|
v-rob joined #minetest-dev |
| 06:02 |
|
Pexin joined #minetest-dev |
| 07:16 |
|
v-rob joined #minetest-dev |
| 07:31 |
|
v-rob joined #minetest-dev |
| 07:45 |
|
olliy joined #minetest-dev |
| 08:24 |
|
calcul0n joined #minetest-dev |
| 08:32 |
|
nemo42 joined #minetest-dev |
| 09:17 |
|
basxto joined #minetest-dev |
| 09:20 |
|
basxto joined #minetest-dev |
| 09:40 |
|
proller joined #minetest-dev |
| 10:10 |
|
Fixer joined #minetest-dev |
| 10:35 |
|
erlehmann joined #minetest-dev |
| 11:07 |
|
appguru joined #minetest-dev |
| 11:19 |
|
HuguesRoss joined #minetest-dev |
| 11:38 |
|
proller joined #minetest-dev |
| 12:31 |
erlehmann |
the response to #11989 really irritates me. has anyone ever proposed a rule to *not* do breaking API changes if at least two games or mods can be pointed out that would break? |
| 12:31 |
ShadowBot |
https://github.com/minetest/minetest/issues/11989 -- Incompatible API change in get_player_control() causes server crashes |
| 12:32 |
erlehmann |
it would shortcut all these ridiculous debates about “oh this is not really a breaking change because while game devs and mod devs know it, we haven't ever written it down in 10 years, so they are totally wrong in assuming stuff works – THOSE FOOLS!” |
| 12:53 |
|
jordan4ibanez joined #minetest-dev |
| 14:34 |
sfan5 |
having it return a string if not called on a player is literally a copy-paste mistake from whoever implemented this |
| 14:34 |
sfan5 |
changing the API is already hard enough as is, that we have to seriously discuss the implications of a change like this says a lot |
| 14:35 |
erlehmann |
IMO there is literally no cost with reverting the breakage and maybe outputting a warning about it |
| 14:36 |
sfan5 |
and in case it isn't obvious I do not support the "Microsoft" approach to compatibility |
| 14:36 |
erlehmann |
the problem is changing it in a minor release, with no warning |
| 14:36 |
erlehmann |
and no deprecation period |
| 14:36 |
sfan5 |
we only have minor releases |
| 14:36 |
sfan5 |
>deprecation period |
| 14:37 |
erlehmann |
then stop breaking stuff |
| 14:37 |
sfan5 |
please don't make me laugh |
| 14:37 |
erlehmann |
hey deprecation worked fine for the alpha texture spam thing |
| 14:37 |
erlehmann |
mod devs were like how do i fix this and fixed it |
| 14:37 |
sfan5 |
the minetest.deserialize semi-vulnerability is just as much of a "changed in a minor release with no warning" like this |
| 14:37 |
MTDiscord |
<luatic> erlehmann: how should such a warning work? |
| 14:37 |
sfan5 |
yet you don't seem to care about that one |
| 14:38 |
celeron55 |
if someone's wondering how that code actually came to be, i think it was written by RBA in 2012-11, tested by Taoki and merged by myself. apparently i didn't catch the mistake |
| 14:38 |
sfan5 |
@luatic you put a log_deprecated(L, "stupid"); in the right part of the if, easy |
| 14:38 |
MTDiscord |
<luatic> sfan5: not easy |
| 14:39 |
erlehmann |
sfan5 a) i did not notice b) the only thing you can deduce from that is that i may be a hypocrite, not that what i say is wrong c) security is a good reason to break an API, random musings about API return values over 8 years after the API was first implemented, is probably one of the worst reasons |
| 14:39 |
MTDiscord |
<luatic> because it has come to be expected that player methods can be called for entities without spamming logs |
| 14:39 |
erlehmann |
luatic may be right |
| 14:39 |
erlehmann |
i have seen code in node on punch handlers |
| 14:39 |
erlehmann |
obviously anything can punch a node right? |
| 14:39 |
sfan5 |
"it has come to be expected" that sounds like the actual problem |
| 14:39 |
erlehmann |
so people ask for puncher:get_player_control().sneak orwhatever |
| 14:40 |
MTDiscord |
<luatic> sfan5: the header literally says it |
| 14:40 |
MTDiscord |
<luatic> https://github.com/minetest/minetest/blob/master/doc/lua_api.txt#L6646 |
| 14:40 |
MTDiscord |
<luatic> "no-op for other objects" |
| 14:40 |
erlehmann |
which is a lie |
| 14:40 |
MTDiscord |
<luatic> now logging a warning isn't exactly no operation, is it? |
| 14:40 |
erlehmann |
the documentation lied about other stuff too |
| 14:40 |
sfan5 |
is returning an empty string a "no-op"? |
| 14:40 |
erlehmann |
luatic you are right, we can't do the warning probably |
| 14:41 |
erlehmann |
the empty string wins on terms of not breaking a bunch of unrelated shit that the person who introduced this change is probably not going to fix by themselves |
| 14:41 |
erlehmann |
this would be different if someone would go around and fix all those mods that go crashy and ensure they caught all of them |
| 14:41 |
MTDiscord |
<luatic> I have an idea for the warning, but it's not clean either. If we returned an empty table, we could set a metatable on it that logs the warning if the table is indexed. |
| 14:41 |
sfan5 |
minetest development is hell, no wonder nobody wants to do it |
| 14:42 |
sfan5 |
you change literally anything and you get yelled at by devs and server owners |
| 14:42 |
rubenwardy |
the fix for mods is really simple |
| 14:42 |
sfan5 |
AND you are expected to either fix their code for free or revert the change |
| 14:42 |
erlehmann |
hey i submitted a patch |
| 14:42 |
erlehmann |
for free |
| 14:42 |
erlehmann |
and i spent time helping users to fix their worlds |
| 14:42 |
sfan5 |
submit a patch to your broken game instead |
| 14:42 |
erlehmann |
that were going crashy |
| 14:42 |
erlehmann |
i will *also* do that |
| 14:43 |
erlehmann |
but come on, you can't casually break 3 games and claim it's someone elses fault for not subscribing to your idea of API aesthetics |
| 14:43 |
erlehmann |
i am pretty sure there are more cases of that, in particular node punch handlers |
| 14:43 |
erlehmann |
and for server admins it is really ugly |
| 14:45 |
erlehmann |
from my POV (game/mod dev), minetest development has been hell whenever someone changed stuff that previously worked. literally all of the issues i had that frustrated me were of the “someone thinks this should be different for aesthetic reasons, code goes crashy or works different” type |
| 14:45 |
erlehmann |
it is perfectly possible to move forward without breaking things |
| 14:46 |
sfan5 |
puncher:real_get_player_control_v2_fixed() |
| 14:46 |
erlehmann |
it's just that it is ugly and apparently enough people think looking good is better than not creating a ton of work for mod devs |
| 14:46 |
erlehmann |
yeah, puncher:get_player_control_2() |
| 14:46 |
erlehmann |
exactly that, then remove the old function in some major release down the road |
| 14:47 |
erlehmann |
at least then the bugs make sense. no one will get bug reports with “i updated minetest and your game crashes at startup” |
| 14:47 |
erlehmann |
(it was a wolf that was a boat passenger, btw) |
| 14:49 |
erlehmann |
the worst thing about this is that everyone of the devs is perfectly able to not casually break stuff. it is not hard work, no one is too stupid for it. but time and time again, aesthetics trump practical choices, creating more bugs. |
| 14:52 |
MTDiscord |
<luatic> PlayerRefs and ObjRefs should be seperated entirely for a clean API. PlayerRefs should extend ObjRefs. Same for object properties; players should have player properties instead. |
| 14:52 |
MTDiscord |
<savilli> it's legal to change api between major releases |
| 14:53 |
MTDiscord |
<luatic> the entire player:get_player_something() is ugly already |
| 14:53 |
MTDiscord |
<luatic> next major release is 6.0, just saying |
| 14:53 |
MTDiscord |
<savilli> nah, 5.5 is also major |
| 14:53 |
sfan5 |
it's not |
| 14:53 |
MTDiscord |
<luatic> ^ |
| 14:53 |
MTDiscord |
<luatic> semver doesn't work that way, @savilli |
| 14:53 |
MTDiscord |
<luatic> I mean, technically it might be |
| 14:53 |
erlehmann |
but minetest uses sentimental versioning, not semantic versioning! |
| 14:54 |
MTDiscord |
<luatic> But according to semver it may not be |
| 14:54 |
erlehmann |
increase the numbers how it feels right |
| 14:54 |
sfan5 |
but applying semver by-the-letter would eternally stall progress |
| 14:54 |
MTDiscord |
<savilli> it's clearly major bc it has new features |
| 14:54 |
sfan5 |
so we don't do it |
| 14:54 |
MTDiscord |
<savilli> not bc some numbers |
| 14:54 |
MTDiscord |
<luatic> you don't understand semver |
| 14:54 |
MTDiscord |
<luatic> read https://semver.org/ |
| 14:54 |
erlehmann |
sfan5 applying semver by the letter does not stall progress. it just means that it is immediately obvious when devs can't be bothered to make code backwards compatible. |
| 14:55 |
MTDiscord |
<luatic> new features allow increasing the minor version number |
| 14:55 |
erlehmann |
IMO the x.y.z numbers are best interpreted in way of risk. x is a big risk when upgrading, y is a small risk, z is next to none. |
| 14:55 |
sfan5 |
erlehmann: in reality applying semver by the latter would mean you only ever do major bumps |
| 14:55 |
sfan5 |
which helps nobody |
| 14:56 |
sfan5 |
letter* |
| 14:56 |
MTDiscord |
<savilli> changing the discussed api looks like a small risk to me |
| 14:56 |
MTDiscord |
<savilli> you don't need to rewrite everything |
| 14:57 |
MTDiscord |
<luatic> If it can be shown that a technically breaking change is highly unlikely to cause mod breakage and yields a considerable improvement, it's fine to just increment the minor version I believe. |
| 14:57 |
erlehmann |
sfan5, i know that argument and yeah, everyone who either can't write or can't be bothered to write future-proof code thinks this is 100% the truth. the reality is that breaking changes are tested and batched. |
| 14:58 |
erlehmann |
and then applied in the next major release |
| 14:58 |
MTDiscord |
<luatic> savilli: it will lead to immediate crashes for all MCL users with outdated or MCL2 |
| 14:58 |
erlehmann |
i am not sure if the fixed mcl mods are even on contentdb lol |
| 14:58 |
MTDiscord |
<savilli> well, should have read release notes |
| 14:58 |
MTDiscord |
Command sent from Discord by Luatic: |
| 14:58 |
MTDiscord |
!? |
| 14:58 |
erlehmann |
yeah the users should have read the release notes lol |
| 14:58 |
MTDiscord |
<luatic> MCL2 has 160k downloads |
| 14:59 |
MTDiscord |
<savilli> then MCL2 doesn't work with 5.5, and users shouldn't update until it will be fixed |
| 14:59 |
MTDiscord |
<savilli> that's a good contract |
| 15:00 |
erlehmann |
you know what, i am not saying this lightly, but in my opinion, every single one of you who thinks normal users should suffer to satisfy your sense of aesthetics has a bit of an empathy problem. it's the same as with the maps: there are people out there who have no idea what lua or c++ is who happily play minetest and they have no idea what to do if they upgrade and it fails. |
| 15:00 |
MTDiscord |
<luatic> It isn't!? |
| 15:00 |
MTDiscord |
<luatic> Side note, MT 5.5 aggressively upgrading maps is still frustrating |
| 15:00 |
MTDiscord |
<luatic> It has lead to me having both a 5.5 and 5.4 test world for my mods now |
| 15:01 |
erlehmann |
luatic i meant the tga maps actually. but yes lol. it is frustrating. it means you can not even go back to 5.4 if minetest changes make your world crashy! |
| 15:01 |
erlehmann |
that makes it worse lol |
| 15:01 |
MTDiscord |
<luatic> didn't even think of that, poor users |
| 15:02 |
erlehmann |
as a game dev, i am willing to update the check for get_player_controls(), of course. but think of users please. |
| 15:02 |
erlehmann |
get_player_control() sorry |
| 15:02 |
sfan5 |
I am pretty sure MCL contains a bunch of engine-version specific workaround that rely on undocumented or internal things |
| 15:03 |
sfan5 |
supposed those break suddenly, would it means you also have an empathy problem? |
| 15:03 |
sfan5 |
-s |
| 15:04 |
erlehmann |
well in this case it costs literally nothing to not upset users. also i am pretty sure mcl2 is a bunch of bad code held together with duct tape and chewing gum, because most of mineclonia development consists of cleaning that up. |
| 15:05 |
|
proller joined #minetest-dev |
| 15:06 |
erlehmann |
sfan5 if other mods or games broke with 5.5 in a way that users can not easily fix it or downgrade to 5.4 (because the world gets converted) i would say, that, yes: whoever broke it evidently does not care about other people's problems enough and does not value their time. i would be surprised if that is a controversial statement, even. |
| 15:07 |
erlehmann |
i'd prefer to not go into it deeper, because i do not want to turn into kilbith |
| 15:08 |
sfan5 |
does "easily fix" include updating the game? |
| 15:09 |
erlehmann |
not really, since for that the game would have to be fixed first. |
| 15:09 |
erlehmann |
and you can't expect users to figure out which mod has this behaviour. |
| 15:09 |
erlehmann |
the mcl* mods are probably not the only ones that break, just the only ones i know about. |
| 15:10 |
erlehmann |
a user having mod soup of like 15 mods is the case i envision |
| 15:10 |
sfan5 |
they don't have to, open the content tab and click all green squares until none are left |
| 15:10 |
sfan5 |
or I think there's an "Update all" button actually |
| 15:10 |
celeron55 |
the mod name is probably on the error message shown in the dialog, right? |
| 15:11 |
erlehmann |
AsyncErr: Lua: Runtime error from mod '??' in callback luaentity_Step(): ...netest/games/mineclonia/mods/ENTITIES/mcl_boats/init.lua:263: attempt to index local 'ctrl' (a nil value) |
| 15:11 |
erlehmann |
stack traceback: |
| 15:11 |
erlehmann |
...netest/games/mineclonia/mods/ENTITIES/mcl_boats/init.lua:263: in function 'on_step_old' |
| 15:11 |
erlehmann |
...mes/mineclonia/mods/ENVIRONMENT/mcl_void_damage/init.lua:17: in function <...mes/mineclonia/mods/ENVIRONMENT/mcl_void_damage/init.lua:16> |
| 15:11 |
celeron55 |
personally i would go with "ok whatever, return a string then and write to the docs 'sorry this API was designed in 2012'" |
| 15:11 |
erlehmann |
i agree with celeron55, it's the simplest thing that works |
| 15:12 |
erlehmann |
having assisted server owners in fixing crashes, i can say that whatever is obvious to us now is not obvious to someone who just set up a server in their spare time for their friends or kids. |
| 15:12 |
celeron55 |
this isn't something that prevents making good design choices in the future |
| 15:13 |
erlehmann |
also, there are kids out there who have no idea about all of this stuff. maybe the error message will make some of them into programmers! or primitivists, if they conclude software was a mistake. |
| 15:13 |
MTDiscord |
<luatic> erlehmann: BTW, I think the proper fix for mods is to check for ref:is_player() instead of ref:get_player_controls() or {}, ref:get_player_controls() and ref:get_player_controls().something or the like. |
| 15:14 |
erlehmann |
luatic how to fix mods is not the issue here. |
| 15:15 |
erlehmann |
i mean, i appreciate the input, but i know perfectly well what the boat code does wrong. |
| 15:15 |
sfan5 |
anyway it's not like there isn't a good way forward for this issue, but it would have likely turned out different if the text wasn't the same "noo you can't change this undocumented and never intended behaviour, now my mod broke, go revert it!!!" we are very tired of hearing |
| 15:15 |
sfan5 |
@luatic it just crossed my mind that you can return a table which throws a warning when it's indexed |
| 15:16 |
sfan5 |
(not a serious suggestion) |
| 15:16 |
erlehmann |
sfan5 i provided literally every bit of information and wrote a patch which i tested. what more do you want me to do? |
| 15:16 |
erlehmann |
if it is “fix your game”, yes, i will |
| 15:16 |
erlehmann |
regardless of what minetest does |
| 15:17 |
MTDiscord |
<luatic> sfan5: lol I suggested that 35 min ago "I have an idea for the warning, but it's not clean either. If we returned an empty table, we could set a metatable on it that logs the warning if the table is indexed." |
| 15:17 |
erlehmann |
yeah and then you have to scan contentdb to ensure that no one had the admittedly stupid idea to check for the empty string |
| 15:17 |
MTDiscord |
<luatic> or for the type |
| 15:17 |
erlehmann |
indeed |
| 15:17 |
MTDiscord |
<luatic> unfortunately such scans are practically impossible due to Lua's dynamic nature |
| 15:18 |
sfan5 |
I'd say mineclone is the only game/mod where this can reasonably cause problems in practice |
| 15:18 |
erlehmann |
hahahahaha |
| 15:18 |
MTDiscord |
<luatic> I did in fact grep around a bit, and driver:get_player_control().something seems to be a common idiom in mods which provide "driveables". |
| 15:18 |
erlehmann |
sfan5, based on what? gut feeling? |
| 15:18 |
erlehmann |
i greped too, this is how i found the puncher:get_player_control() pattern |
| 15:18 |
MTDiscord |
<luatic> OFC I don't have the time to analyze the content to determine whether driver can be an object and whether driver:is_player() is checked earlier on. |
| 15:18 |
erlehmann |
in nodecore i think |
| 15:19 |
sfan5 |
erlehmann: how many games do you know where entities can "ride" other objects? |
| 15:19 |
sfan5 |
don't answer "three" |
| 15:19 |
MTDiscord |
<luatic> I know of a fourth ;) but it's 0.4.x-only |
| 15:19 |
MTDiscord |
<luatic> the most frequent "idiom" seems to be placer:get_player_control().sneak |
| 15:20 |
erlehmann |
sfan5 uh no idea about games, but you are focusing too much on my example. see the thing that luatic showed. |
| 15:20 |
erlehmann |
like 4 or 5, but does it matter? |
| 15:22 |
sfan5 |
of course it does |
| 15:22 |
sfan5 |
nothing changed about calling get_player_control() on players |
| 15:23 |
MTDiscord |
<luatic> if you wanted to fight fire with fire, continue returning empty strings but set a metatable on the string table such that attempting to index a string with any of the control names will result in a warning |
| 15:23 |
erlehmann |
look, people call this not only for boats |
| 15:25 |
erlehmann |
lib_mount has |
| 15:25 |
erlehmann |
local ctrl = entity.driver:get_player_control() |
| 15:25 |
erlehmann |
if ctrl.aux1 then |
| 15:25 |
erlehmann |
painting has |
| 15:25 |
erlehmann |
if puncher:get_player_control().sneak |
| 15:25 |
MTDiscord |
<luatic> the question is just whether entity.driver can even be a non-player objref |
| 15:25 |
MTDiscord |
<luatic> I believe this showcases something else though |
| 15:25 |
MTDiscord |
<luatic> Modders relying on playerrefs not changing during the session |
| 15:26 |
MTDiscord |
<luatic> Hence storing the player instead of the player name |
| 15:26 |
|
nemo42 joined #minetest-dev |
| 15:26 |
MTDiscord |
<luatic> Some modders even compare players by reference xD |
| 15:26 |
sfan5 |
if you have one ref it cannot reasonably expire |
| 15:26 |
MTDiscord |
<luatic> Therefore, I believe the docs should guarantee this, as it also makes code a lot neater. |
| 15:26 |
sfan5 |
(because why would it?) |
| 15:26 |
MTDiscord |
<luatic> Theoretically, the engine might swap it out |
| 15:27 |
sfan5 |
no? |
| 15:27 |
sfan5 |
the userdata object itself cannot be changed |
| 15:27 |
sfan5 |
only it's contents / what it points to |
| 15:27 |
MTDiscord |
<luatic> I know |
| 15:27 |
MTDiscord |
<luatic> That's the point, what if the engine decided to recreate it's internal player object, invalidating the obj ref mods hold? |
| 15:28 |
sfan5 |
erlehmann: how do you know that those refs can be non-players? is it "gut feeling?" |
| 15:28 |
MTDiscord |
<luatic> Listen, I just want a guarantee that player refs stay valid for the session duration xD |
| 15:30 |
erlehmann |
sfan5 for the puncher case, i am pretty sure that non-player entities can punch others, right? |
| 15:30 |
erlehmann |
firing an arorw on a travelnet box or something would punch it, no? |
| 15:30 |
erlehmann |
arrow |
| 15:30 |
sfan5 |
depends on the implementation |
| 15:31 |
erlehmann |
yeah, depends. we have two ways forward: choose something incompatible and make sure it is not a problem in practice … or revert to the compatible API and discuss it whenever 6.0 is a thing |
| 15:31 |
erlehmann |
i prefer the thing where we can stop worrying and just apply my patch |
| 15:31 |
erlehmann |
and discuss other problems |
| 15:32 |
MTDiscord |
<luatic> I hope that we will properly distinguish players by 6.0 |
| 15:32 |
erlehmann |
i actually hope not! |
| 15:32 |
erlehmann |
i find it pretty useful to be able to have one code path for players and mobs |
| 15:32 |
MTDiscord |
<luatic> not by, with* |
| 15:32 |
erlehmann |
but then again i am the person who wrote code so that boats can enter minecarts, so … |
| 15:33 |
erlehmann |
(the trick is to punch the minecart in the right direction when the boat goes forward) |
| 15:33 |
MTDiscord |
<luatic> By distinguishing I mean inheritance |
| 15:33 |
MTDiscord |
<luatic> Players could still be used as ObjRefs, but ObjRefs not as players |
| 15:34 |
MTDiscord |
<luatic> So if you want MCL boats to be rideable by any nearby ObjRef, that still works and requires just one code path |
| 15:34 |
erlehmann |
above everything else, i wish for minetest to never ruin existing setups except for security/crash/hang reasons – and then only as much as ncessary. |
| 15:34 |
MTDiscord |
<luatic> In this one case, actually a Lua "quirk" is to blame (the overly permissive string metatable) |
| 15:35 |
MTDiscord |
<luatic> for those who want strictness in a dev environment: you can do setmetatable(string, {__index = function() error"attempt to index a string value" end}) |
| 15:35 |
Pexin |
0.4.x "we're not even 1.0, dont expect stability" 5.0 "WELP nevermind!" haha |
| 15:41 |
MTDiscord |
<Jonathon> Seems like erlehmann is determined to piss off/drive away the main dev behind minetest |
| 15:42 |
|
unexploredtest[m joined #minetest-dev |
| 15:42 |
MTDiscord |
<Jonathon> As if there wasnt already a lack of them |
| 15:42 |
MTDiscord |
<josiah_wi> Agreed. |
| 15:49 |
MTDiscord |
<Bastrabun> sfan5: re 11976 Here's what the profiler gives me: https://pastebin.com/UK6eDrQV |
| 15:49 |
MTDiscord |
<Bastrabun> About 18 players results in 2022-01-25 15:58:29: INFO[Main]: Server: map saving (sum) [ms] . . . . . . . 1x 5860 |
| 15:52 |
erlehmann |
I just encountered a crash where a paleotest carnotaurus got punched by a non-player. |
| 15:52 |
erlehmann |
The new API is so *fun*! |
| 15:53 |
erlehmann |
and yes, it is because it assumes get_player_control() on the puncher |
| 16:01 |
erlehmann |
jonathon, i am not determined to piss off anyone. which is why i made it as easy as possible this time to fix this bug. |
| 16:04 |
rubenwardy |
could rename it to get_player_controls() and deprecate the singular form, which is gramatically invalid anyway |
| 16:04 |
rubenwardy |
totally not confusing |
| 16:09 |
erlehmann |
just so you know, the paleotest crash also makes worlds unusable. as soon as i rejoin, the dinos fight again. |
| 16:13 |
MTDiscord |
<luatic> @Greens Sleep Paralysis Demon you're dinos are being broken |
| 16:16 |
|
behalebabo joined #minetest-dev |
| 16:22 |
|
behalebabo joined #minetest-dev |
| 16:27 |
sfan5 |
@Bastabun and it freezes for about 5 seconds regularly right? |
| 16:28 |
MTDiscord |
<Jonathon> @Bastrabun ^ |
| 16:30 |
MTDiscord |
<Bastrabun> It freezes in the interval of whatever I set server_map_save_interval to. If I leave it the default 5.3, we get freezes every 5.3 seconds, currently it's on 30 and we get freezes every 30 seconds. |
| 16:46 |
|
v-rob joined #minetest-dev |
| 16:46 |
sfan5 |
how long is the freeze itself? |
| 16:51 |
MTDiscord |
<Bastrabun> That depends on the number of players and the value of server_map_save_interval |
| 16:52 |
MTDiscord |
<Bastrabun> Roughly 30 players and server_map_save_interval 5.3 gives me a 2 to 3 second lag, while server_map_save_interval 30 gives me a 3 to 4 second lag. |
| 16:58 |
|
Yad joined #minetest-dev |
| 16:59 |
v-rob |
Sheesh, maybe we should invite Microsoft over to do our compatibility for us. Having to support undocumented behaviour that you're obviously not supposed to do... |
| 16:59 |
MTDiscord |
<Bastrabun> This is what the log says over the past two hours: https://pastebin.com/0H5u1CU5 |
| 17:28 |
celeron55 |
v-rob: i think people are probably overreacting to this |
| 17:29 |
celeron55 |
modders probably tried out if the result works for mobs when used like a table with true-ish and false-ish items, and it does, so they assume it returns such a table for mobs also |
| 17:29 |
celeron55 |
turns out it's actually a string, but a string just happens to behave like one |
| 17:29 |
celeron55 |
thus, changing it into actually being an empty table will work fine |
| 17:32 |
erlehmann |
“will probably work out fine” is what got us into this mess |
| 17:33 |
celeron55 |
not really |
| 17:33 |
erlehmann |
well, the API was changed without even greping contentdb for its usage (which would have revealed that problem) |
| 17:33 |
celeron55 |
you're all overreacting, basically |
| 17:33 |
MTDiscord |
<Jonathon> When is he not? |
| 17:34 |
rubenwardy |
yeah, I think modders will assume that this just returns {} |
| 17:35 |
Krock |
get_player_by_name returns nil when there's no player |
| 17:35 |
Krock |
the player control and information should do the same |
| 17:35 |
MTDiscord |
<Jonathon> Minetest: consistently inconsistent |
| 17:37 |
celeron55 |
Krock: arguably it can be different because it needs to return an object (of sorts) |
| 17:38 |
celeron55 |
the rest are just values |
| 17:38 |
MTDiscord |
<luatic> I consider {} and 0 a decent solution |
| 17:38 |
celeron55 |
anyway, this is the wrong time to design this api |
| 17:38 |
celeron55 |
this api was already designed. 8 years ago |
| 17:40 |
celeron55 |
any energy you have for api design should be focused on those apis currently in waiting to be released as stable |
| 17:41 |
Krock |
what was the previous behaviour? did it provide garbage values, or none at all? |
| 17:41 |
celeron55 |
the empty string |
| 17:42 |
celeron55 |
then sfan5 changed it to nil |
| 17:42 |
celeron55 |
and it broke stuff |
| 17:42 |
celeron55 |
2 weeks ago |
| 17:42 |
erlehmann |
curiously, no one ever complained about that empty string. i mean, it's obviously garbage design. but it works. |
| 17:42 |
celeron55 |
that's because the empty string looks like an empty table |
| 17:42 |
Krock |
? |
| 17:43 |
Krock |
"" ~= {} |
| 17:43 |
MTDiscord |
<luatic> not exactly, but due to the string metatable, as long as you don't try indexing it with "sub", "gmatch" etc it does |
| 17:43 |
MTDiscord |
<luatic> {} ~= {} though |
| 17:43 |
celeron55 |
yes not exactly. but you can even do # on it to count the items. and it says 0 like for an empty table |
| 17:43 |
Krock |
fair point |
| 17:43 |
celeron55 |
actually |
| 17:43 |
erlehmann |
Krock so people are doing puncher:get_player_control().sneak in mobs that turn enemy when they are punched or node on_punch handlers |
| 17:43 |
MTDiscord |
<luatic> I don't see why anyone would be calling # on a dict |
| 17:43 |
celeron55 |
i think it would say 0 for the player table also as that's used as a dictionary |
| 17:44 |
Krock |
well then {} would be a saner option, provided that mods do not explicitly rely on this string bs |
| 17:44 |
Krock |
erlehmann: yes, within callbacks the player object must be valid and contain this data |
| 17:45 |
erlehmann |
Krock the thing is, non-players can also punch |
| 17:45 |
MTDiscord |
<luatic> ^ |
| 17:45 |
erlehmann |
or be passengers in a boat, which is how the mcl thing crashed |
| 17:45 |
Krock |
get_player_control() is undefined behaviour for non-players |
| 17:45 |
MTDiscord |
<luatic> wrong |
| 17:45 |
erlehmann |
it has been perfectly well defined in the code for a long time and was changed |
| 17:46 |
MTDiscord |
<luatic> the docs say it's a "no-op" |
| 17:46 |
MTDiscord |
<luatic> so yes, returning nothing (which is actually not the same as returning nil) is reasonable for a "no-op" |
| 17:46 |
celeron55 |
i think basically everyone is agreeing on {} now |
| 17:46 |
MTDiscord |
<luatic> yes |
| 17:46 |
celeron55 |
so there's no need to discuss this more |
| 17:46 |
celeron55 |
someone just needs to commit the patch |
| 17:46 |
Krock |
oh right. no-op would then indicate return 0; (i.e. no arguments, no action) ? |
| 17:47 |
erlehmann |
i do not agree, because i am not in the mood to file another bug as 5.5.0 is released |
| 17:47 |
erlehmann |
it will probably work out, but some mad person will have relied on this being a string |
| 17:47 |
Krock |
celeron55: as a hacky solution, yes. |
| 17:47 |
MTDiscord |
<luatic> then that's on them honestly |
| 17:47 |
erlehmann |
does not matter who is at fault |
| 17:47 |
celeron55 |
the api didn't say it's a string. you are supposed to file a bug at that point |
| 17:47 |
erlehmann |
it literally does not matter who is guilty of shitty code, be it engine or mod devs. the users have crashes. |
| 17:49 |
erlehmann |
will make sure that mineclonia does not crash with whatever 5.5 does anyway |
| 17:49 |
celeron55 |
erlehmann: you're doing more bad than good to MT by draining the time from devs that have more productive use for the time |
| 17:49 |
MTDiscord |
<Jonathon> ^this |
| 17:49 |
erlehmann |
celeron55 i provided a solution and then everyone else had an opinion on a thing that does not hinder them. i will be silent. |
| 17:49 |
erlehmann |
until the next breakage! |
| 17:50 |
celeron55 |
it's good that you brought the bug up |
| 17:50 |
celeron55 |
but as the core team is responsible for the solution, you have to give the core team the freedom to make the solution |
| 17:51 |
erlehmann |
and sorry that i posted so much about how it is bad. i just really thing it has to be solved *somehow*, even if it is not my preferred solution. it seems you have found one. |
| 17:52 |
celeron55 |
sorry for the shitty code made in 2012, i'm trying to not let it happen anymore 8) |
| 17:53 |
celeron55 |
whether that's succesful, well, we'll see in 2020 |
| 17:53 |
celeron55 |
eh |
| 17:53 |
celeron55 |
i mean 2030 |
| 17:54 |
erlehmann |
well … i doubt you can anticipate what someone else will find ugly years later. but if you ever come up with an API everyone likes, maybe ask zughy about it ;P |
| 18:11 |
jordan4ibanez |
Hmm, perhaps you could preemptively break it to avoid future breakage? |
| 18:40 |
|
JordanL2 joined #minetest-dev |
| 18:47 |
|
BuckarooBanzai joined #minetest-dev |
| 18:57 |
|
Guest99 joined #minetest-dev |
| 18:58 |
|
Guest99 joined #minetest-dev |
| 18:59 |
|
Guest99 joined #minetest-dev |
| 19:21 |
|
v-rob joined #minetest-dev |
| 20:06 |
|
proller joined #minetest-dev |
| 20:08 |
|
nemo42 joined #minetest-dev |
| 20:40 |
sfan5 |
I'll import weblate commits later and stuff so translators have a chance to translate before release |
| 20:41 |
sfan5 |
@luatic how's the "send gui scale to server" PR going |
| 20:41 |
|
v-rob joined #minetest-dev |
| 20:52 |
MTDiscord |
<luatic> sorry, not going at all currently, 5.5 will probably have to do without it |
| 20:58 |
sfan5 |
then so be it |
| 22:05 |
|
v-rob joined #minetest-dev |
| 22:20 |
sfan5 |
https://github.com/minetest/minetest/commits/ci translation commits, will push after build finishes |
| 22:31 |
|
jordan4ibanez joined #minetest-dev |
| 23:00 |
|
proller joined #minetest-dev |
| 23:17 |
|
proller joined #minetest-dev |
