| Time |
Nick |
Message |
| 00:33 |
|
AliasAlreadyTake joined #minetest-dev |
| 00:40 |
|
erlehmann joined #minetest-dev |
| 01:05 |
|
proller joined #minetest-dev |
| 02:15 |
|
tekakutli joined #minetest-dev |
| 02:19 |
|
asdflkj_sh joined #minetest-dev |
| 03:29 |
|
queria joined #minetest-dev |
| 03:33 |
|
queria joined #minetest-dev |
| 03:49 |
|
asdflkj_sh joined #minetest-dev |
| 03:50 |
|
asdflkj_sh joined #minetest-dev |
| 03:59 |
erlehmann |
i have made some remarks on how to avoid breaking changes in the engine while not impeding progress. what would be a good venue for further discussion? https://github.com/minetest/minetest/issues/11989#issuecomment-1024796327 |
| 04:00 |
erlehmann |
(this text can not be in the forum because cheat clients, lol) |
| 05:00 |
|
MTDiscord joined #minetest-dev |
| 05:26 |
|
v-rob joined #minetest-dev |
| 05:39 |
|
Krock_ joined #minetest-dev |
| 06:40 |
Baytuch |
Good morning |
| 08:51 |
|
nemo42 joined #minetest-dev |
| 09:18 |
|
YuGiOhJCJ joined #minetest-dev |
| 09:26 |
|
calcul0n joined #minetest-dev |
| 10:39 |
sfan5 |
wow another three page rant from erlehmann |
| 10:41 |
|
Fixer joined #minetest-dev |
| 10:53 |
|
proller joined #minetest-dev |
| 11:02 |
MTDiscord |
<luatic> erlehmann: Open another issue, something like "Backwards compatibility policy" which then gets tagged Discussion Controversial |
| 11:03 |
|
nemo42 joined #minetest-dev |
| 11:03 |
MTDiscord |
<luatic> sfan5: is there any user-friendly way to "downgrade" maps from ZSTD to GZIP, or to prevent the upgrade in the first place? |
| 11:04 |
sfan5 |
compiling is not user friendly |
| 11:04 |
MTDiscord |
<Sublayer plank> there would be that thing to recompile minetest to save in the older format |
| 11:04 |
sfan5 |
an external program could potentially do it |
| 11:04 |
MTDiscord |
<Sublayer plank> but that's not user friendly |
| 11:04 |
MTDiscord |
<luatic> The map upgrading probably deserves it's own issue, not only related to the get_player_control() - many will try out 5.5 but might prefer sticking with 5.4 because of other breakage that will inevitably occur. |
| 11:05 |
MTDiscord |
<luatic> Was it rubenwardy who suggested that users be shown a dialog to ask them whether they want their map to be upgraded? |
| 11:06 |
MTDiscord |
<luatic> That'd sound like rather good UX to me, although it might get in the way. Probably add something like a "remember my decision" checkbox? |
| 11:06 |
sfan5 |
half of our users won't even know what that means |
| 11:06 |
MTDiscord |
<luatic> That's why we rephrase it understandably |
| 11:07 |
MTDiscord |
<luatic> Something like "Upgrade your map? You won't be able to use it on 5.4 and lower, but it will be smaller and faster." |
| 11:07 |
sfan5 |
the same half of our users would never think about downgrading |
| 11:10 |
|
HuguesRoss joined #minetest-dev |
| 11:19 |
sfan5 |
fyi I would be okay with a temporary setting that tells MT to write the old format but ? on anything with user interaction like you described |
| 11:33 |
|
nemo42 joined #minetest-dev |
| 11:46 |
|
appguru joined #minetest-dev |
| 12:15 |
|
nemo42 joined #minetest-dev |
| 12:32 |
|
appguru joined #minetest-dev |
| 12:46 |
Baytuch |
is better for me: https://github.com/minetest-mods/skinsdb/pull/64/files . I dont see when users change skin and grep db file. |
| 13:00 |
sfan5 |
this channel is for development of the minetest engine only, so please mention such things in #minetest in the future |
| 13:04 |
Baytuch |
okay, sfan5 |
| 13:27 |
erlehmann |
luatic sfan what about having a setting for “upgrade old maps” initially set to false? |
| 13:28 |
erlehmann |
i mean gzip compat has to be there forever anyway right? |
| 14:13 |
sfan5 |
no |
| 14:13 |
MTDiscord |
<Benrob0329> Why can't we just have a user dialog? Is informed consent that bad? |
| 14:14 |
sfan5 |
do you remember when Internet Explorer asked for consent everytime it connected to a HTTPS webpage? |
| 14:15 |
MTDiscord |
<Benrob0329> Thats a borderline strawman, this isn't for every world, just ones that haven't been upgraded yet. |
| 14:15 |
MTDiscord |
<Benrob0329> Its the opposite of that IE dialog, actually |
| 14:17 |
sfan5 |
it was meant to be an example of "asking for consent for trivialities that the average user will not understand and just click whatever gets them past the dialog" |
| 14:18 |
Pexin |
wasnt the suggestion from the other day to just not start the game if the user says no? after a dialog explicitly warning the conversion is not reversible? |
| 14:18 |
sfan5 |
dunno |
| 14:18 |
Pexin |
not continuing to support oldmode |
| 14:19 |
MTDiscord |
<Benrob0329> No matter what we do, there will be pain, but not making it irreversible pain would probably be best |
| 14:19 |
sfan5 |
writing an older map format is just toggling a value (it has to be supported anyway) so that'd be more practical |
| 14:19 |
Krock_ |
this is by far not the first time when map versions changed |
| 14:19 |
Krock_ |
it happens every now and then, and that's how software development goes |
| 14:19 |
MTDiscord |
<Benrob0329> No, but there has been concern |
| 14:20 |
MTDiscord |
<Benrob0329> I don't know why only now, but its here |
| 14:20 |
Pexin |
there are probably far more users now than there have ever been |
| 14:20 |
Krock_ |
the discussions will past as soon another new version is out |
| 14:21 |
Krock_ |
*pass *Minetest version |
| 14:22 |
MTDiscord |
<Benrob0329> My last 2¢ are this: We can't fix user stupidity, but we can give them a chance to not be stupid. |
| 14:23 |
sfan5 |
what happened to making backups? Do people not do this? |
| 14:23 |
Pexin |
so, a config value to write into oldmode would be trivial? |
| 14:24 |
sfan5 |
by the way determing "has this map been opened in 5.5 or newer" is a non-trivial problem, there can be any amount of mapblocks in any version inside a database |
| 14:24 |
sfan5 |
Pexin: correct |
| 14:27 |
MTDiscord |
<MisterE> correct, unfortunately |
| 14:27 |
freshreplicant[m |
Is it possible to make a copy of the map in the old format automatically and notify the user of this? Or at least to give them a prompt like "Make a copy of the original map?" |
| 14:27 |
Pexin |
freshreplicant[m: some maps are collossal |
| 14:28 |
rubenwardy |
Best solution is a tool to downgrade maps |
| 14:29 |
|
nemo42 joined #minetest-dev |
| 14:30 |
sfan5 |
would have to be written (though I have an unreleased one like that) |
| 14:31 |
sfan5 |
more importantly we need consensus in the dev team, with these things it's 2-3 vocal users arguing against 1 or maybe two active coredevs which is not great |
| 14:35 |
MTDiscord |
<Warr1024> Consent wouldn't be so bad, as long as it's not a long-term thing. Something like "warning: upgrading the map will make it incompatible with 5.4-. Not upgrading the map will make it incompatible with 5.6+" Users who choose not to upgrade will just have to dismiss the nag screen each time they start up until they either cave, or they upgrade to 5.6 and the not-upgrading option is removed. |
| 14:40 |
Pexin |
no mention of 5.6 is needed, it's just clutter |
| 14:43 |
erlehmann |
> Not upgrading the map will make it incompatible with 5.6+ |
| 14:43 |
erlehmann |
that's just bollocks, the conversion code does not magically bitrot |
| 14:43 |
erlehmann |
input and output formats are well defined and understood |
| 14:48 |
|
Taoki joined #minetest-dev |
| 14:54 |
MTDiscord |
<luatic> Another issue is that these async errors are buggy |
| 15:05 |
erlehmann |
luatic wdym? |
| 15:06 |
MTDiscord |
<luatic> #11698 |
| 15:06 |
ShadowBot |
https://github.com/minetest/minetest/issues/11698 -- Async "mapblock version mismatch error" seems to be stuck in deadlock sometimes |
| 15:08 |
erlehmann |
uh oh |
| 15:08 |
sfan5 |
rubenwardy, Krock, HuguesRoss: are you available tomorrow evening to assist with the release |
| 15:08 |
HuguesRoss |
timezone? |
| 15:08 |
rubenwardy |
Yes |
| 15:08 |
rubenwardy |
Cest |
| 15:08 |
MTDiscord |
<luatic> yep, I can still reproduce it |
| 15:08 |
Krock |
UTC+1 or so |
| 15:08 |
rubenwardy |
Well, UTC is usually used |
| 15:09 |
rubenwardy |
Cest is UTC+1 I think? |
| 15:09 |
Krock |
I will try to be around, but cannot yet promise it |
| 15:09 |
MTDiscord |
<luatic> erlehmann: it's simple really, sometimes instead of properly throwing the "async err" it's stuck at "shutting down" forever |
| 15:09 |
rubenwardy |
Oops, that's summer time |
| 15:09 |
erlehmann |
rubenwardy, UTC+2 is CEST |
| 15:09 |
HuguesRoss |
Yeah, I think I should be fine then. "Evening" CEST is early afternoon for me |
| 15:09 |
erlehmann |
but it is winter right now |
| 15:09 |
rubenwardy |
Have we closed all blockers? Are we reasonably sure it's been sufficiently tested? |
| 15:10 |
rubenwardy |
Yeah, CET is UTC+1 |
| 15:10 |
erlehmann |
rubenwardy could you please look at the thing where sfan5 aligns mapblock boundaries with mapgen boundaries? |
| 15:10 |
MTDiscord |
<luatic> #11365 is still open |
| 15:10 |
ShadowBot |
https://github.com/minetest/minetest/issues/11365 -- Shadowmaps cannot be disabled by the server |
| 15:10 |
HuguesRoss |
Anyway, I'm UTC-5 so I'll be around |
| 15:10 |
erlehmann |
it's a very simple change and it fixes a lot of problems |
| 15:11 |
sfan5 |
to be more exact everything from 18 to 21 UTC is fine for me |
| 15:11 |
erlehmann |
rubenwardy https://github.com/minetest/minetest/pull/11866 |
| 15:12 |
erlehmann |
the problem is that a bunch of code uses MAX_MAPGEN_LIMIT or how it's called for “where does the gameworld end” and it lies inside a mapblock, so logically the outermost mapblock is half-working only |
| 15:13 |
erlehmann |
because obviously the mapgen has generated past MAX_MAPGEN_LIMIT for literally forever |
| 15:13 |
erlehmann |
if you do not merge this, expect crashes with generated structures in the outermost mapblock and with lua mapgens in general |
| 15:14 |
sfan5 |
speaking of blockers #11699 needs a merge, #12001 an approval |
| 15:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/11699 -- Disable dynamic shadows for 5.5.0 release by SmallJoker |
| 15:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/12001 -- Update credits for 5.5.0 by sfan5 |
| 15:14 |
sfan5 |
#11831 and #11992 won't make it, not critical IMO |
| 15:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/11831 -- Don't go out of the map during raycast by savilli |
| 15:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/11992 -- Heap UAF in craft recipes |
| 15:14 |
MTDiscord |
<luatic> sfan5: the heap UAF is a vuln though? |
| 15:15 |
sfan5 |
well yea |
| 15:15 |
erlehmann |
does #11831 mean you can crash the game by using mcl2 buckets at the map border? or do you need to *start* out of bounds? |
| 15:15 |
ShadowBot |
https://github.com/minetest/minetest/issues/11831 -- Don't go out of the map during raycast by savilli |
| 15:15 |
erlehmann |
i can test this if this is important to anyone |
| 15:16 |
MTDiscord |
<luatic> this matters for everything that uses raycasts, including guns, and yes, it allows to send the game into an inf loop |
| 15:16 |
erlehmann |
fun times! |
| 15:16 |
MTDiscord |
<luatic> but wasn't the final "agreement" that instead of properly clipping the raycast, the overflow should have been fixed? |
| 15:16 |
sfan5 |
that was my suggestion yes |
| 15:16 |
erlehmann |
luatic geting people to agree on fixing overflows is notoriously difficult though, but yeah, it's a good suggestion. |
| 15:17 |
erlehmann |
i'm beginning to warm up to cora's “warn ppl to hold off on upgrading to 5.5, because you can not downgrade your maps” stance tbh |
| 15:17 |
erlehmann |
too many bugs still |
| 15:17 |
MTDiscord |
<Jonathon> Not patching the raycast isnt the biggest thing as it isnt a new bug. That said i would agree if possible a patch should be merged |
| 15:18 |
MTDiscord |
<luatic> erlehmann: many of these bugs have existed in 5.4 and lower as well though, if you're referring to the raycast bug? |
| 15:18 |
erlehmann |
luatic yeah, but i find it telling that “update credits” is a blocker but “fix a crash that is used in the wild to make servers unplayable” is none. |
| 15:19 |
sfan5 |
please can you just |
| 15:19 |
sfan5 |
not say anything |
| 15:19 |
erlehmann |
ok |
| 15:19 |
MTDiscord |
<luatic> the problem is that while the credits are manageable, fixing crashes takes forever# |
| 15:19 |
MTDiscord |
<luatic> there are tons of segfaults |
| 15:19 |
sfan5 |
while not disallowed here I would appreciate if you could keep tangentially development related comments in #minetest |
| 15:20 |
MTDiscord |
<luatic> I'd love it if I could sneak in #12000 |
| 15:20 |
ShadowBot |
https://github.com/minetest/minetest/issues/12000 -- Fix builtin statbar backgrounds by appgurueu |
| 15:20 |
sfan5 |
I just wanted to mention that |
| 15:20 |
sfan5 |
but only if we're confident that it works |
| 15:21 |
sfan5 |
regarding the craft recipes thing, yes it's a reliably triggerable C++ bug but not really the first and in the end people still choose mods they install |
| 15:21 |
MTDiscord |
<luatic> Well, the first commit (https://github.com/minetest/minetest/pull/12000/commits/a6b7ccf7c5866e4125427e08daa0ebf7a4a25d8f) is literally a trivial fix for the "half bg icons" |
| 15:22 |
sfan5 |
have you looked at all the translation PRs in game? |
| 15:22 |
sfan5 |
(I have not but we want two approvals anyway) |
| 15:22 |
MTDiscord |
<luatic> I'd merge the french one already |
| 15:23 |
MTDiscord |
<luatic> game#2922 that is |
| 15:23 |
ShadowBot |
https://github.com/minetest/minetest_game/issues/2922 -- Update french translations by dacmot |
| 15:23 |
MTDiscord |
<luatic> because Hugues approval should definitely count there |
| 15:25 |
sfan5 |
sure |
| 15:26 |
MTDiscord |
<luatic> oh yay, Calinou just reviewed too! |
| 15:28 |
sfan5 |
also the least thankful task: someone will have to write a changelog |
| 15:32 |
Pexin |
#11969 trivial fix and future-proofing for plans of mine. needs review. super simple. I'll make you a pie..? |
| 15:32 |
ShadowBot |
https://github.com/minetest/minetest/issues/11969 -- Use absolute value for bouncy in collision by pecksin |
| 15:33 |
sfan5 |
I think v-rob wanted to look at that |
| 15:37 |
sfan5 |
@luatic I looked at the other two now |
| 15:39 |
erlehmann |
can i try bribery as well? i'll send 25€ via SEPA to wherever celeron55 or rubenwardy tell me it would contributes to minetest or contentdb hosting if https://github.com/minetest/minetest/pull/11866 is included in 5.5 |
| 15:39 |
sfan5 |
don't think that'll work |
| 15:41 |
erlehmann |
well, the offer is out there! |
| 15:41 |
MTDiscord |
<Sublayer plank> lol, bug bounty but for merging a finished PR? |
| 15:43 |
erlehmann |
i think it neatly illustrates how much i think this fix is necessary. |
| 15:54 |
|
nemo42 joined #minetest-dev |
| 15:57 |
HuguesRoss |
I've approved #12000, my opinion is that it's a low enough priority bugfix that we may want to hold it back if we're actually planning to ship 5.5 tomorrow |
| 15:57 |
ShadowBot |
https://github.com/minetest/minetest/issues/12000 -- Fix builtin statbar backgrounds by appgurueu |
| 16:00 |
MTDiscord |
<luatic> HuguesRoss: The "half heart bg" thing is really low priority, agreed, but the "scale to default" is completely broken in it's current form |
| 16:01 |
MTDiscord |
<luatic> I suppose I should have opened two PRs to get the low priority thing merged as a trivial bugfix without the need for two approvals |
| 16:02 |
HuguesRoss |
For me personally, within 48hrs of a release I see the priority limit around the level of crashes, RCE, data destruction, etc |
| 16:03 |
HuguesRoss |
Otherwise it's easy to run into the "pushing to production on a Friday afternoon" problem |
| 16:03 |
sfan5 |
I agree |
| 16:05 |
erlehmann |
HuguesRoss do you want to look at 11866 then? it mitigates both crashes and data destruction (corrupted mapgen at boundaries) |
| 16:07 |
Pexin |
what about "one more week" for a rc2? |
| 16:08 |
* Pexin |
gets smacked |
| 16:08 |
HuguesRoss |
erlehmann: Just gave the diff a glance, that change seems suspiciously simple to me |
| 16:08 |
HuguesRoss |
I can give it a more thorough look, but my gut instinct is telling me it's going to break the shit out of something |
| 16:09 |
sfan5 |
it's merely raising the limit at which some function refuses to work |
| 16:09 |
sfan5 |
notable the function did not have this limit in 5.4 |
| 16:09 |
erlehmann |
HuguesRoss the issue is simple, shit was broken when sfan5 limited minetest.get_nodes_in_area() to accept MAX_MAPGEN_LIMIT, which is not actually the limit of the map. it lies within the outermost mapblock. so there is currently a shell around the map in which minetest can place nodes, but not find them. |
| 16:10 |
erlehmann |
HuguesRoss so with this fix sfan5 raises the limit so that all placed nodes in the outermost mapblock are found |
| 16:10 |
erlehmann |
and some other stuff that was weird in that area is also eliminated, since the area where nodes are placed but not found has gone |
| 16:11 |
erlehmann |
or at least, if it exists, it does not exist *within* a mapblock that is only partially working rn |
| 16:11 |
HuguesRoss |
hm, ok. I'll admit my concerns are probably more applicable to how many systems can potentially interact with map limits, if it's purely for placing/getting nodes it might be safe |
| 16:11 |
HuguesRoss |
I'll try to test it later today |
| 16:11 |
erlehmann |
oh, it fixes other bugs i have whined about forever too |
| 16:12 |
erlehmann |
the fundemental problem is that almost everyone thought that MAX_MAPGEN_LIMIT is actually something that the mapgen adheres to, but it is going over it a bit. so the more code that relied on that, the more code already is buggy. |
| 16:12 |
erlehmann |
like mineclonia has a structure that places tnt there. but *if* you place tnt there and manage to explode it, bad things can happen. |
| 16:13 |
erlehmann |
so if the pyramid generates at the map border and you trigger it, fun times! |
| 16:13 |
erlehmann |
uh-oh. i think tnt also does raycast explosions. :( |
| 16:16 |
HuguesRoss |
Yeah, as a modder I've just always acted under the assumption that things will break down if you hit the limits of the map. It's the kind of case that seems easy for folks to overlook |
| 16:16 |
erlehmann |
HuguesRoss the thing is, things were actually engineered pretty well until it was broken |
| 16:17 |
erlehmann |
the behaviour at the map boundaries is basically “these mapblocks never load” |
| 16:17 |
sfan5 |
(they still don't) |
| 16:17 |
erlehmann |
only the change to minetest.find_nodes_in_area() with wrong boundaries made it problematic – and i agree with sfan5 that changing the boundaries to lie on a mapblock will prevent most issues there. |
| 16:18 |
erlehmann |
sfan5 it is possible to emerge the area and place stuff there and some mod code does it by accident. i do it deliberately in mineclonia and output a warning at server start. |
| 16:18 |
erlehmann |
(if minetest fails to find nodes it can place) |
| 16:21 |
|
nemo42 left #minetest-dev |
| 16:26 |
erlehmann |
can you please tag #12005 bug bounty? |
| 16:26 |
ShadowBot |
https://github.com/minetest/minetest/issues/12005 -- Old maps are upgraded to ZSTD block compression, making it impossible to load the world in 5.4 |
| 16:27 |
erlehmann |
and yes, the user pain is worth 50€ to me. i haven't spent money on computer games in a while. |
| 16:27 |
erlehmann |
and minetest is really fun |
| 16:30 |
rubenwardy |
I don't like either of those solutions. Better to have a tool to allow downgrading |
| 16:31 |
MTDiscord |
<luatic> rubenwardy: that tool should be included in MT though |
| 16:32 |
sfan5 |
rubenwardy: advertising something as a "tool to allow downgrading" will be a failure as people will also expect it to do any potential migrations that games have done |
| 16:32 |
sfan5 |
e.g. node aliases |
| 16:33 |
sfan5 |
they only sure way to to downgrade a map is to restore from a backup |
| 16:33 |
sfan5 |
period |
| 16:56 |
sfan5 |
okay so it's actually only #11992 that'll go unsolved |
| 16:56 |
ShadowBot |
https://github.com/minetest/minetest/issues/11992 -- Heap UAF in craft recipes |
| 17:08 |
|
proller joined #minetest-dev |
| 17:10 |
|
olliy joined #minetest-dev |
| 17:15 |
MTDiscord |
<savilli> i have a very stupid fix for it |
| 17:16 |
MTDiscord |
<savilli> replace output.item with craftGetItemName(output.item) |
| 17:16 |
erlehmann |
why is this stupid |
| 17:17 |
erlehmann |
does it ruin books or enchantments or so? |
| 17:17 |
erlehmann |
or banners |
| 17:17 |
MTDiscord |
<savilli> it definitely can break smth |
| 17:18 |
MTDiscord |
<savilli> and it doesn't actually remove broken pointers from m_output_craft_definitions |
| 17:18 |
|
asdflkj_sh joined #minetest-dev |
| 17:19 |
rubenwardy |
I dread to think how many memory vulnerabilities that Minetest has |
| 17:20 |
MTDiscord |
<savilli> it's not only i caught with asan |
| 17:20 |
erlehmann |
rubenwardy it is possible to figure those out largely automatically, given good unit tests. see here for an example: https://mister-muffin.de/p/-J2g.html |
| 17:20 |
|
asdflkj_sh joined #minetest-dev |
| 17:20 |
MTDiscord |
<savilli> and i ran really huge modpacks |
| 17:20 |
MTDiscord |
<savilli> so probably it's not that bad |
| 17:20 |
erlehmann |
asan is really good at spitting out very usable things |
| 17:20 |
erlehmann |
savilli yeah but have you fuzzed minetest using afl-fuzz? |
| 17:21 |
erlehmann |
i have not yet gotten to that, but if i was tasked to exploit minetest, i would fuzz it using afl with a) vanilla compile options b) ubsan c) asan d) msan e) tsan |
| 17:21 |
erlehmann |
haven't used cfsan |
| 17:21 |
MTDiscord |
<savilli> s/not only/the only |
| 17:23 |
erlehmann |
savilli could you detail your test setup a bit more? |
| 17:23 |
erlehmann |
maybe make an issue about it |
| 17:23 |
erlehmann |
or a blogpost, whatever |
| 17:24 |
MTDiscord |
<savilli> 1) build minetest with asan, msan, tsan, etc 2) install a huge modpack aka mineclone5 or pandorabox 3) play a little 4) profit |
| 17:25 |
erlehmann |
then you are doing the same as me |
| 17:25 |
erlehmann |
also please report the other stuff you found |
| 17:25 |
MTDiscord |
<savilli> sure |
| 17:25 |
MTDiscord |
<savilli> well i found a lot of memory leaks on shutdown, too lazy to report them :) |
| 17:25 |
erlehmann |
oh, that is easily explainable |
| 17:26 |
erlehmann |
the leak sanitizer will *only* complain on shutdown |
| 17:26 |
erlehmann |
it has very little runtime overhead, but before shutdown, it complains about all leaks it found |
| 17:26 |
erlehmann |
that does not mean they only happen on shutdown, lsan is just not as aggressive in crashing your software as asan, ubsan, msan, tsan |
| 17:27 |
erlehmann |
to spell it out: most of that are probably real issues |
| 17:27 |
MTDiscord |
<savilli> nah, there were literally leaked global objects, like server env, lua env, sun, moon etc |
| 17:27 |
MTDiscord |
<savilli> not a big deal |
| 17:29 |
erlehmann |
well, i'd say the next level is torturing minetest by means of afl or afl++, are you interested in a collaboration on that? |
| 17:31 |
MTDiscord |
<savilli> maybe |
| 17:32 |
erlehmann |
i value enthusiastic content, so “maybe” means “no”. :P |
| 17:32 |
MTDiscord |
<savilli> i believe you can do it by yourself XD |
| 17:32 |
erlehmann |
i am less motivated to do things alone tbh |
| 17:34 |
erlehmann |
rubenwardy if you want an intro to finding memory vulnerabilities, i can give you a short demo. the gist of it: instrument your program using afl, then compile it so that asan or msan make it crash on address or memory issues. then throw afl at it and wait. usually a few hours of fuzzing should yield a dozen bugs in the average case, often you get at least 1 crash in the first few minutes. |
| 17:34 |
rubenwardy |
I might have a go with that |
| 17:34 |
rubenwardy |
but with my personal C++ game |
| 17:34 |
erlehmann |
and yes, i am writing this here and not in #minetest because i think *everyone* writing c or c++ should do that |
| 17:35 |
MTDiscord |
<savilli> minetest is bad for fuzzing |
| 17:35 |
erlehmann |
oh, you can abuse the unit tests (if you have some) |
| 17:35 |
erlehmann |
which is why i was upset about removal of irrlichtmt unit tests |
| 17:35 |
erlehmann |
i found the b3d crash by means of afl |
| 17:37 |
erlehmann |
rubenwardy to quickstart afl, see this: https://github.com/nmoskopp/afl-fuzz-talk – 00-get-afl.sh gets you afl and compiles it (but maybe replace the http with https) – ”10-fuzz.sh ./src/demo-printf.c” starts fuzzing an intentionally buggy program that reads a simple file format and writes found crashes to ./src/demo-printf.d/crashes or so |
| 17:38 |
erlehmann |
and by intentionally buggy, i mean: it does something like “printf(buffer);” which is a format string vulnerability and afl finds an input that crashes it |
| 17:47 |
rubenwardy |
I think we should push back the release date by a week, we're still fixing fairly big bugs. A week is a small amount of time compared to the 48 weeks since 5.4 |
| 17:49 |
rubenwardy |
We could start a `dev` branch. That's in our git guidelines, but hasn't really been used yet |
| 17:49 |
rubenwardy |
if people want to start 5.6 |
| 17:55 |
sfan5 |
I don't really agree, you are right the remaining fixes are not small but they are low risk |
| 18:06 |
|
JordanL2 joined #minetest-dev |
| 18:09 |
MTDiscord |
<savilli> actually the only good fix for #11992 is to move crafting system to lua |
| 18:09 |
ShadowBot |
https://github.com/minetest/minetest/issues/11992 -- Heap UAF in craft recipes |
| 18:09 |
MTDiscord |
<savilli> my server uses the custom one, so i was surprised it even exists in the engine |
| 18:31 |
Baytuch |
I added fix for slow machine based ARMv7. Set client_mapblock_limit as 250 blocks. System allocate mem for 350-400 blocks and reset to 250. |
| 18:32 |
Baytuch |
https://github.com/baytuch/minetest/commit/e6a60fff169e75e89e7edae57ff8c2a5a094f485 |
| 18:34 |
erlehmann |
Baytuch please make a pull request on github |
| 18:34 |
erlehmann |
very good |
| 18:40 |
|
Wuzzy joined #minetest-dev |
| 20:15 |
|
jordan4ibanez joined #minetest-dev |
| 20:25 |
|
HuguesRoss joined #minetest-dev |
| 21:08 |
ROllerozxa |
So I cleaned ~/.minetest and just realised Minetest doesn't show its logo when installed system-wide. Not a 5.5.0-dev regression, it's like this in 5.4 as well. Is this intentional? |
| 21:14 |
rubenwardy |
so the logo where? |
| 21:15 |
ROllerozxa |
in the menu outside of the "start game" tab |
| 21:16 |
rubenwardy |
the "Minetest" text? |
| 21:20 |
|
v-rob joined #minetest-dev |
| 21:20 |
|
olliy1or joined #minetest-dev |
| 21:22 |
ROllerozxa |
Yeah. So, I used to have remnants of my Windows installation in ~/.minetest, in particular the base texture pack at ~/.minetest/textures/base. While this was in my Minetest home folder, the Minetest logo (menu_header.png) shows up. I decided to remove this since it's unnecessary to be in the home folder, it's installed alongside Minetest in /usr/share/minetest/textures/base/ anyways. But now... The logo is gone! Occurs both on 5.5.0-dev and |
| 21:22 |
ROllerozxa |
5.4.1, and it is still able to load other textures from the base pack, it's just the logo that it doesn't want to load. |
| 21:25 |
ROllerozxa |
It is supposed to show up right? Like, on the settings tab the minetest logo should be at the top there. |
| 21:26 |
sfan5 |
works for me but I also have RUN_IN_PLACE=1 |
| 21:27 |
ROllerozxa |
yeah this occurs when installed system-wide, so the base texture pack wouldn't be in the same place as user-installed texture packs |
| 21:27 |
ROllerozxa |
super weird, I probably should check if I can get something useful out of the debug logs |
| 21:43 |
|
Yad joined #minetest-dev |
| 22:04 |
ROllerozxa |
found the issue, #12013 fixes it |
| 22:04 |
ShadowBot |
https://github.com/minetest/minetest/issues/12013 -- Fix Minetest logo when installed system-wide. by rollerozxa |
| 22:05 |
ROllerozxa |
apparently it has been like this for... 8 years? shocking. |
| 22:05 |
sfan5 |
I supposed core.get_texturepath() and core.get_texturepath_share() are equivalent for run_in_place? |
| 22:06 |
ROllerozxa |
yeah |
| 22:08 |
ROllerozxa |
both 'share' and 'user' are . when RUN_IN_PLACE=1, while 'share' is /usr/share/minetest and 'user' is ~/.minetest when RUN_IN_PLACE=0. so it has tried to load it from 'user', which works when RUN_IN_PLACE=1 but not when RUN_IN_PLACE=0 |
| 22:09 |
ROllerozxa |
so it has worked for portable windows builds and linux binaries people compile themselves that are contained to the source tree, but not for system-installed packages |
| 22:14 |
sfan5 |
rubenwardy: at this rate it does indeed look wise to delay the release by half a week |
| 22:14 |
sfan5 |
but I don't really want to |
| 22:14 |
rubenwardy |
it's annoying to delay even further, but it could be less pain in the long run |
| 22:16 |
sfan5 |
depends on what you're referring to |
| 22:16 |
sfan5 |
for fixes we can just do a 5.5.1 next week if necessary |
| 22:17 |
Wuzzy |
I'm done testing the return values of player functions when called on non-players |
| 22:17 |
Wuzzy |
so, most functions return nil |
| 22:18 |
Wuzzy |
due to the recent PR, get_player_control and get_player_control_bits also return nil |
| 22:18 |
sfan5 |
they return {} and 0 respectively now |
| 22:18 |
sfan5 |
did you miss that the issue was already fixes? |
| 22:18 |
sfan5 |
fixed* |
| 22:18 |
Wuzzy |
surprisingly, get_player_velocity returns {x=0,y=0,z=0} |
| 22:19 |
erlehmann |
quick, change it to something that breaks mods |
| 22:19 |
erlehmann |
before 5.5 |
| 22:19 |
Wuzzy |
xd |
| 22:19 |
erlehmann |
xxxD |
| 22:20 |
Wuzzy |
no, thats a feature request and we're in feature freeze. thi has to way for 5.6 |
| 22:20 |
Wuzzy |
well, {x=0,y=0,z=0} is *technically* correct i would say. right? ? |
| 22:20 |
sfan5 |
get_player_velocity is an alias of get_velocity |
| 22:20 |
sfan5 |
that's why |
| 22:20 |
erlehmann |
just do a refucktoring then |
| 22:21 |
erlehmann |
and hide it in the PR! |
| 22:21 |
erlehmann |
tried and tested method ;) |
| 22:21 |
Wuzzy |
i think get_player_velocity can stay |
| 22:22 |
rubenwardy |
get_player_velocity is deprecated anyway |
| 22:22 |
Wuzzy |
oh right |
| 22:22 |
Wuzzy |
forget it then |
| 22:22 |
rubenwardy |
I'm unable to clone the Minetest launch pad because they're still using sha1 |
| 22:23 |
Wuzzy |
what do you think? How difficult it would be to make default privileges revokable? |
| 22:23 |
sfan5 |
by the way Wuzzy since we've just had this discussion: is the addition of basic_debug a breaking change? |
| 22:23 |
sfan5 |
after all it requires games to make changes to work like before |
| 22:23 |
sfan5 |
they're not revokable? that'd be a problem |
| 22:23 |
Wuzzy |
i dont really know, but it might make players upset ... |
| 22:24 |
Wuzzy |
yeah. /revokeme all in singleplayer, you still have privs left |
| 22:24 |
Wuzzy |
i have no idea if this is by design or bug |
| 22:24 |
sfan5 |
"in single player the following cannot be revoked: shout" what does that mean? |
| 22:24 |
rubenwardy |
OK so this looks like a right pain |
| 22:24 |
sfan5 |
I don't think it's about default privs but rather the admin cannot strip himself of privs |
| 22:24 |
Wuzzy |
it means that the shout priv is irrevokable |
| 22:24 |
rubenwardy |
A MTG mod can just grant on_joinplayer |
| 22:25 |
sfan5 |
because you can absolutely forbid people to interact or shout |
| 22:25 |
rubenwardy |
er nevermind |
| 22:25 |
Wuzzy |
if a priv has give_to_singleplayer=true, then you can not revoke it from singleplayer |
| 22:25 |
Wuzzy |
so give_to_singleplayer actually does two things at once: Grant it to singleplayer by default AND make it irrevokable |
| 22:25 |
Wuzzy |
(irrevokable to sigleplayer) |
| 22:26 |
Wuzzy |
same for give_to_admin, of course |
| 22:26 |
|
v-rob joined #minetest-dev |
| 22:27 |
Wuzzy |
sfan5: the error message is just a user friendly way to show this limitation. the actual limitation lies in Minetest core, not builtin |
| 22:28 |
sfan5 |
okay wait what |
| 22:28 |
sfan5 |
are default privileges not a thing? |
| 22:28 |
Wuzzy |
what do you mean with "default priv"? |
| 22:28 |
Wuzzy |
give_to_singleplayer? |
| 22:28 |
rubenwardy |
default- |
| 22:28 |
sfan5 |
I would have expected a "give_by_default" in the privilege definition |
| 22:28 |
rubenwardy |
default_privs |
| 22:29 |
sfan5 |
but turns out this is only controlled by the setting "default_privs" |
| 22:29 |
Wuzzy |
wait, what? |
| 22:29 |
Wuzzy |
hmmmm let me check something |
| 22:29 |
sfan5 |
default_privs provides a way to actually specify defaults for privs |
| 22:30 |
sfan5 |
this is impossible in a priv definition which only provides a way to force for SP or the admin |
| 22:30 |
rubenwardy |
defaults also only apply on user register |
| 22:30 |
Wuzzy |
okay i just tested |
| 22:30 |
Wuzzy |
it seems that give_to_singleplayer 100% forces the privs on singleplayer and its impossible to revoke them |
| 22:30 |
rubenwardy |
It feels like you need a MTG mod that adds the priv and then uses meta to remember if it was automatically added |
| 22:31 |
Wuzzy |
even when default_privs is empty you always get the give_to_singleplayer privs |
| 22:31 |
rubenwardy |
so it doesn't readd after revocation |
| 22:31 |
sfan5 |
rubenwardy: it doesn't only 'feel' like that, that's the way |
| 22:32 |
rubenwardy |
how would an orienteering mod disable this behaviour? |
| 22:32 |
rubenwardy |
I guess opt-depend + a method call / variable set |
| 22:32 |
rubenwardy |
debug_priv.enabled = false |
| 22:33 |
Wuzzy |
so it sems the privileges are litereally impossible to get rid for singleplayer once added with give_to_singleplayer=true |
| 22:33 |
Wuzzy |
/revokeme doesn't wor |
| 22:33 |
Wuzzy |
not even minetest.set_player_privs("singleplayer", {}) works |
| 22:33 |
Wuzzy |
so i guess the engine forces the privilege or something |
| 22:35 |
Wuzzy |
sfan5: so the issue is not default_privs. this works fine. the issue is give_to_singleplayer/give_to_admin |
| 22:36 |
sfan5 |
rubenwardy: on_joinplayer and revoke it? not ideal to have a mod make these decisions |
| 22:36 |
Wuzzy |
i get the rationale that we dont want admin to accidentally revoke server/privs/etc |
| 22:36 |
Wuzzy |
maybe make "irrevokability" a separate parameter for privilege definition |
| 22:37 |
Wuzzy |
does this make sense? |
| 22:37 |
sfan5 |
what is the point of `default_privs` then if you can't influence game/mod choices? |
| 22:38 |
Wuzzy |
again: This has NOTHING do do with default_privs |
| 22:38 |
Wuzzy |
wait... |
| 22:38 |
sfan5 |
these two mechanisms interact so obviously it does |
| 22:38 |
Wuzzy |
what are you going to say? |
| 22:38 |
rubenwardy |
default_privs doesn't help here, because it only runs on new players |
| 22:38 |
Wuzzy |
OH |
| 22:38 |
Wuzzy |
right |
| 22:38 |
Wuzzy |
omg |
| 22:38 |
sfan5 |
"if the game makes the choices for you why do we have a default_privs setting?" |
| 22:39 |
Wuzzy |
then my debug approach is basically completely broken... |
| 22:39 |
Wuzzy |
there is no way to save it then ? |
| 22:39 |
sfan5 |
it should have been a hud flag |
| 22:39 |
Wuzzy |
yeah, do a full revert. nuke it from orbit ? |
| 22:39 |
erlehmann |
so back to basics |
| 22:39 |
sfan5 |
but I'm pretty this was discussed |
| 22:39 |
rubenwardy |
this was discussed before the PR was merged, in length |
| 22:39 |
Wuzzy |
yes it was discussed heavily |
| 22:39 |
erlehmann |
stuff can have been discussed and important points not be valued enough, happens all the time. |
| 22:39 |
sfan5 |
MTG registers two privileges, neither of which are forced on |
| 22:40 |
Pexin |
as I recall, the result was simply that different people have different perspectives on what MT is supposed to be |
| 22:40 |
sfan5 |
is there actually ANY usecase for forcing privs on? |
| 22:40 |
Wuzzy |
are those give_to_singleplayer/give_to_admin? |
| 22:40 |
sfan5 |
Wuzzy: that's what I mean by "forced on", both false |
| 22:40 |
Wuzzy |
yes if its false then its not forced |
| 22:40 |
Wuzzy |
its only forced if true |
| 22:40 |
sfan5 |
yes |
| 22:41 |
erlehmann |
Pexin the result of the discussion does not matter if players think it sucks. |
| 22:41 |
Wuzzy |
regardless of the debug thing, i think the "forced privilege" is a discussion in its own right... |
| 22:41 |
Wuzzy |
it always stuck me as very weird to have some privileges impossible to revoke |
| 22:41 |
Wuzzy |
well for admin you kinda sorta can argue it sucks when you accidentally "lock yourself out" or whatever ... hmmm |
| 22:42 |
sfan5 |
>“Why not using HUD flags?”. HUD flags are controlled by mods. This implies that mods can screw around with the debug info at will. All it takes is one poorly-coded mod to strip away the extended debug info from the admin, which is not fun to track down. Privileges, by comparison, are much more reliable and predictable and there are no ugly |
| 22:42 |
sfan5 |
surprises. |
| 22:42 |
sfan5 |
? |
| 22:42 |
Wuzzy |
My mistake was that I have completely not taken default_privs into account ... argh |
| 22:42 |
Wuzzy |
default_privs ruins everything |
| 22:43 |
erlehmann |
sfan5 what does that mean for the use case of “as soon as one uses mcl_maps players should have a motivation to make maps for orientation and i want a mod to remove their access to coordinates”, how do you envision that to be implemented? |
| 22:43 |
sfan5 |
"Mods should have NO business in touching the debug info at all. Debug info must work without mods, it's a very essential feature. " so rubenwardy's question of what an orienteering mod would do is just the wrong question |
| 22:43 |
Wuzzy |
so basically no way how I try to "tweak" my PR, all players on old servers will not get the basic_privs unless the server admin MANUALLY adds it |
| 22:43 |
sfan5 |
erlehmann: you should ask Wuzzy because that's his text :) |
| 22:43 |
rubenwardy |
A mod like orienteering breaks if debug info is enabled by a MTG mod |
| 22:43 |
Wuzzy |
yeah, that was always my concern |
| 22:43 |
Wuzzy |
that was the whole reason |
| 22:44 |
Wuzzy |
my goal is to separate debug info from gameplay info |
| 22:44 |
sfan5 |
but in your PR description you argued that mods shouldn't touch the debug info priv |
| 22:44 |
sfan5 |
this is a fundamental conflict |
| 22:44 |
erlehmann |
the debug screen contains too little info tbh, there could also be light level of pointed node and node meta |
| 22:44 |
Wuzzy |
in the perfect world, games/minetest would have exposed stuff like coordinates in a "clean" way (no ugly debug screen) |
| 22:44 |
sfan5 |
I mean it got approved and merged |
| 22:44 |
sfan5 |
but still ??? |
| 22:45 |
Wuzzy |
lol erlehmann. node meta can get out of hand fast |
| 22:45 |
Wuzzy |
is there a size limit for node meta? |
| 22:45 |
sfan5 |
not really |
| 22:45 |
Wuzzy |
then not really a good idea to show the node meta in debug ? |
| 22:45 |
erlehmann |
if anyone introduces one you are going to make a lot of people with chests full of books very angry |
| 22:46 |
Wuzzy |
unless its really simple like "empty/not empty" |
| 22:46 |
erlehmann |
or chests full of shulkers full of shulkers full of shulkers |
| 22:46 |
Pexin |
crated digtrons |
| 22:46 |
Wuzzy |
yeah, an "advanced debug screen" would be very useful |
| 22:46 |
sfan5 |
rubenwardy: I think an orienteering mod can do basically two things: 1) ask the server admin to adjust default privs and everything 2) just go and revoke the priv itself |
| 22:46 |
rubenwardy |
https://irc.minetest.net/minetest/2021-06-21#i_5836890 |
| 22:46 |
rubenwardy |
here's the very very long discussion we had |
| 22:47 |
Wuzzy |
so at this stage it's obviously too late to make a better debug screen for 5.5, so the only way forward is nuking it |
| 22:47 |
Wuzzy |
i mean the PR, not the debug screen!!! |
| 22:48 |
sfan5 |
so was the 5.4 <-> 5.5 thing solved? |
| 22:48 |
sfan5 |
who even approved the PR |
| 22:48 |
Wuzzy |
One idea I had for a better coordiante display would be something like a "coordinate key". it would replace F5 and the debug screen moves to a different key, basically |
| 22:48 |
sfan5 |
couldn've been me |
| 22:49 |
Wuzzy |
the #1 reason why all players use F5 is because of coords |
| 22:49 |
ShadowBot |
https://github.com/minetest/minetest/issues/1 -- GlowStone code by anonymousAwesome |
| 22:49 |
sfan5 |
it was c55 and ruben |
| 22:49 |
rubenwardy |
If you connect to a 5.5 server with a 5.4 client, it pretends you have debug_basic |
| 22:49 |
sfan5 |
well that's good |
| 22:49 |
Wuzzy |
oh |
| 22:49 |
Wuzzy |
that still doesnt fix default_privs tho |
| 22:50 |
Wuzzy |
if a 5.5 player that already registered years ago joins, no basic_debug |
| 22:50 |
sfan5 |
fixable |
| 22:50 |
Wuzzy |
how? |
| 22:50 |
rubenwardy |
wait, otherway around |
| 22:50 |
rubenwardy |
If you connect to a 5.4 server with a 5.5 client, it pretends you have debug_basic |
| 22:51 |
Wuzzy |
yes this makes sense |
| 22:51 |
sfan5 |
minetest.register_on_joinplayer(function(player) if player:get_meta():get("foo") == "" then minetest.grant_privilege(player:get_player_name(), "basic_debug"); player:get_meta():set("foo", 1) end end) |
| 22:51 |
sfan5 |
that's how |
| 22:51 |
Wuzzy |
? |
| 22:51 |
Wuzzy |
that's a mod |
| 22:51 |
sfan5 |
ofc |
| 22:51 |
Wuzzy |
meaning that server operators have to do something |
| 22:51 |
sfan5 |
we'd put that into MTG |
| 22:51 |
Wuzzy |
therefore it will never happen |
| 22:51 |
Wuzzy |
server operator can be slow to change :/ |
| 22:52 |
sfan5 |
if they upgrade to 5.5 they will also upgrade MTG |
| 22:52 |
Wuzzy |
unless you suggest to put this code in builtin ? |
| 22:52 |
sfan5 |
and if not then there's no issue with 5.4 |
| 22:53 |
Wuzzy |
not every server is MTG |
| 22:53 |
sfan5 |
well idk |
| 22:53 |
Wuzzy |
sorry i believe this is a case of "back to the drawing board". just nuke my basic_debug PR |
| 22:53 |
sfan5 |
you got your breaking change PR merged 6 months ago |
| 22:53 |
sfan5 |
and apparently only now realize it's a problem |
| 22:54 |
Wuzzy |
yes but BEFORE release |
| 22:54 |
Wuzzy |
its not entirely true, i always thought about it... it was a splinter in my mind ... but had no idea what to do |
| 22:54 |
Pexin |
I can verify several people were unhappy, but the devs had Spoken |
| 22:54 |
Wuzzy |
the final word has not been spoken |
| 22:55 |
MTDiscord |
<exe_virus> It's a reasonable change |
| 22:55 |
sfan5 |
I knew about the basic_debug thing before and made a mental note to add it as a default priv in MTG |
| 22:55 |
sfan5 |
but the concept being totally broken is new to me |
| 22:55 |
sfan5 |
would've been useful to know earlier |
| 22:55 |
Wuzzy |
I KNOW ? |
| 22:56 |
rubenwardy |
I find it funny how adament you were when we suggested alternatives that let it remain on by default, insisting that it shouldn't be available by default |
| 22:56 |
rubenwardy |
and then changing your mind 1 day before a planned release |
| 22:56 |
MTDiscord |
<MisterE> just put basic_debug in the list of default privs... |
| 22:57 |
Wuzzy |
well that'S the thing... |
| 22:57 |
MTDiscord |
<MisterE> in MT.conf |
| 22:57 |
rubenwardy |
MisterE: won't work, default_privs only grants to _new_ players |
| 22:57 |
Wuzzy |
that won't really work |
| 22:57 |
MTDiscord |
<MisterE> yeah ok |
| 22:57 |
Wuzzy |
It's not just "changing my mind" it's realizing there was a fatal flaw in the logic |
| 22:57 |
MTDiscord |
<MisterE> Well I went and added that code to grant the priv to everyone in a server-special mod |
| 22:58 |
MTDiscord |
<MisterE> logic dies |
| 22:58 |
Wuzzy |
I thought that it would be posible for servers to somehow magically react to the change to make it work again |
| 22:58 |
Wuzzy |
but I did not actually TEST this... haha. that's the reason |
| 22:58 |
Wuzzy |
so yeah, lesson learned, don't make claims that something works if not tested ? |
| 22:59 |
MTDiscord |
<MisterE> ... Im not following |
| 22:59 |
rubenwardy |
test what? |
| 22:59 |
rubenwardy |
The two solutions I see are: |
| 22:59 |
rubenwardy |
1. Add a mod to MTG to grant to players |
| 22:59 |
rubenwardy |
2. Revert the PR, make a replacement for 5.6 |
| 22:59 |
Wuzzy |
I believed that servers could automagically grant the basic_debug by default by a simple parameter like default_privs |
| 22:59 |
Wuzzy |
but i didn't realize it was for NEW players only, this ruins everything |
| 23:00 |
Wuzzy |
why is everyone so obsessed with MTG? lol not every server is MTg |
| 23:00 |
Wuzzy |
do solution 2: Nuke it from orbit, its complete trash, sorry |
| 23:00 |
MTDiscord |
<MisterE> just FYI, most server owners will not update MTG with the server. Most servers are heavily modified, and will implement any necassary fixes themselves, I think. Anyhow thats what I did when I started running 5.5dev |
| 23:01 |
Pexin |
such an autogrant mod, would track whether it has run for each player? so it can be revoked and stay revoked? either way seems a little messy |
| 23:01 |
MTDiscord |
<MisterE> nonono... Just add a mod on cdb that grants the priv by defalut! dont revert the PR! |
| 23:02 |
MTDiscord |
<MisterE> Its a really good idea to have basic_debug un-accessable |
| 23:02 |
Pexin |
MisterE: that still relies on server owners paying attention |
| 23:03 |
sfan5 |
Pexin: it would |
| 23:03 |
MTDiscord |
<MisterE> Pexin, They will pay attn when their players start complaining, like mine did. Then they will make the decision to add the mod or to leave as-is and not allow the debug info |
| 23:03 |
MTDiscord |
<MisterE> which, should be a decision the server owners should make |
| 23:03 |
sfan5 |
dealing with this in MTG is no problem at all (even though it's a bit messy) |
| 23:03 |
Pexin |
someone said possibly put it in builtin? |
| 23:04 |
sfan5 |
the question is just: is it okay that other games will have to apply changes if they want to preserve coordinates? |
| 23:04 |
Wuzzy |
how about this: auto-grant the priv via builtin? would this work? games can later manually revoke it |
| 23:04 |
MTDiscord |
<MisterE> debug info can be considered a "cheat". |
| 23:04 |
MTDiscord |
<MisterE> no |
| 23:04 |
sfan5 |
and actually I think this is salvageable |
| 23:04 |
sfan5 |
consider the many game jam games, none of them will want coords shown |
| 23:04 |
MTDiscord |
<MisterE> builtin is for code that is applicable to all games |
| 23:04 |
MTDiscord |
<MisterE> yes |
| 23:04 |
sfan5 |
the games that do can just grant the priv |
| 23:05 |
MTDiscord |
<MisterE> completely agree |
| 23:05 |
sfan5 |
it *is* messy because it's not a hud flag |
| 23:05 |
sfan5 |
and it *is* a breaking change |
| 23:05 |
Wuzzy |
nuke it then |
| 23:05 |
sfan5 |
no, it does the job |
| 23:05 |
MTDiscord |
<MisterE> stop with the nuclear option! |
| 23:05 |
Pexin |
sfan5: wouldnt those games just deny the F5 menu completely? |
| 23:05 |
Wuzzy |
F5 still works, it just shows less info by default (like FPS) |
| 23:06 |
|
asdflkj_sh joined #minetest-dev |
| 23:06 |
erlehmann |
<rubenwardy> 1. Add a mod to MTG to grant to players |
| 23:06 |
erlehmann |
this does not salvage non-mtg games, just to point it out |
| 23:06 |
MTDiscord |
<MisterE> this is an improvement to the engine, leave it be! Debug should be a priv, and it makes a lot of sense to not grant it by default. Games should have the option but not the default value of allowing in for all players |
| 23:06 |
Wuzzy |
I really don't like the MTG-only solution. this is a massive hack |
| 23:06 |
MTDiscord |
<exe_virus> Built-in is overridable |
| 23:07 |
MTDiscord |
<MisterE> most games do not want the debug |
| 23:07 |
Pexin |
most players do |
| 23:07 |
MTDiscord |
<MisterE> debug is for... debugging |
| 23:07 |
Wuzzy |
heh we could just call the next version 6.0.0 so everything is allowed ? |
| 23:07 |
erlehmann |
this ties greatly into the amazing map incompatibility! not only does mineclone5 no longer have reliable maps due to API change, players can not work around that using debug priv due to another API change! |
| 23:07 |
Wuzzy |
wait, map incompability? one more argument for 6.0.0 instead of 5.5.0 ? |
| 23:07 |
erlehmann |
Wuzzy mcl_maps |
| 23:08 |
sfan5 |
Wuzzy: you can'tz |
| 23:08 |
erlehmann |
i sent it to you in jabber |
| 23:08 |
erlehmann |
the dynamic media thing |
| 23:08 |
erlehmann |
where the api changed twice |
| 23:08 |
MTDiscord |
<MisterE> Pexin, yes, but then they may play on servers which allow it, or they may play singleplayer. never should what players want come before what game designers want, in the context of said games. The players have the option to try to influence the dev of said games tho |
| 23:08 |
Wuzzy |
MAP_LIMIT? |
| 23:08 |
sfan5 |
Wuzzy: you can't trust a word erlehmann says, he will omit or misrepresent facts whenever it serves him |
| 23:08 |
MTDiscord |
<Jonathon> ^ |
| 23:09 |
Wuzzy |
i am not interested in personal drama, please ... |
| 23:09 |
Wuzzy |
stick to the fact |
| 23:09 |
sfan5 |
well he isn't doing that |
| 23:09 |
sfan5 |
that's not personal drama |
| 23:09 |
Wuzzy |
i'm not interested in normal drama either. just facts plz ? |
| 23:09 |
erlehmann |
Wuzzy, mcl_maps does not work on 5.5. it is a maps mod. leave aside why. but debug does not give coords on 5.5. |
| 23:09 |
sfan5 |
erlehmann raised an issue for an API that had changed after an ~1 year deprecation period |
| 23:09 |
MTDiscord |
<MisterE> thats silly, erlehmann, mineclone can and should grant the basic_debug priv by defalut, if that is what they want to do |
| 23:09 |
erlehmann |
i was trying to make fun of the issue |
| 23:10 |
sfan5 |
he calls this an "incompatbility" |
| 23:10 |
erlehmann |
it's about this https://github.com/minetest/minetest/issues/119971 |
| 23:10 |
erlehmann |
can we go back to privileges please? |
| 23:10 |
sfan5 |
which is not technically incorrect but perfectly fine by Minetest standards specifically because there was a deprecation period |
| 23:10 |
erlehmann |
it has been discussed to death |
| 23:10 |
sfan5 |
erlehmann: guess who brought it up |
| 23:11 |
erlehmann |
look, i think it is the stupidest API change i have ever seen in the last 10 years or so and wanted to make fun of it. |
| 23:11 |
Wuzzy |
1 year deprecation is a long time. its ok |
| 23:11 |
MTDiscord |
<MisterE> so, please leave the basic_debug as is |
| 23:11 |
erlehmann |
i'll stop making fun |
| 23:11 |
MTDiscord |
<MisterE> its an annoying but necassary change |
| 23:11 |
erlehmann |
back to the debug thing, will i be reverted or not? |
| 23:11 |
sfan5 |
for "making fun of things" instead of constructive talking go to #minetest |
| 23:11 |
erlehmann |
ok |
| 23:11 |
MTDiscord |
<exe_virus> So what is the broken part of basic_debug? |
| 23:12 |
MTDiscord |
<MisterE> erl... only you can answer this question |
| 23:12 |
erlehmann |
MisterE what question? |
| 23:12 |
sfan5 |
games have to take action to preserve old behaviour |
| 23:12 |
MTDiscord |
<exe_virus> Gotcha |
| 23:12 |
sfan5 |
or in other words a breaking change |
| 23:12 |
Wuzzy |
its not exactly broken its more like... disruptive when players upgrade to 5.5.0 |
| 23:12 |
MTDiscord |
<exe_virus> But it doesn't break games? |
| 23:12 |
|
v-rob joined #minetest-dev |
| 23:12 |
MTDiscord |
<exe_virus> It's a minor inconvenience |
| 23:12 |
MTDiscord |
<Jonathon> moveresult broke old mods, big deal |
| 23:12 |
sfan5 |
the argument was that debug info is not part of a game |
| 23:12 |
MTDiscord |
<MisterE> yes, but it is not a large burden. its a few lines of code |
| 23:12 |
sfan5 |
or something like that |
| 23:12 |
MTDiscord |
<exe_virus> Like, it doesn't cause games to crash or anything |
| 23:12 |
erlehmann |
sfan5 but turns out it is |
| 23:12 |
erlehmann |
like people expect it |
| 23:13 |
erlehmann |
has been there for a while etc. pp. |
| 23:13 |
sfan5 |
also the second issue with basic_debug is that it's messy if a mod wants to ensure that debug info is visible / not visible |
| 23:13 |
erlehmann |
only this time actual users can not do much about it than stay on 5.4 |
| 23:13 |
Wuzzy |
hmm so if i get this right you are now proposing that EVERY game adds boilerplate code? heh |
| 23:13 |
MTDiscord |
<exe_virus> Sure, but they can learn new behavior, and server owners can make the old default the new defulat |
| 23:13 |
MTDiscord |
<exe_virus> No, I won't need it for most games I make |
| 23:13 |
MTDiscord |
<MisterE> no, only the few games that actually want the basic_debug |
| 23:13 |
MTDiscord |
<exe_virus> Just Mt and MCL, nodecore doesn't care |
| 23:14 |
Pexin |
few? |
| 23:14 |
HuguesRoss |
Not to interrupt this spirited debate, but are there any objections to merging #11699 ? |
| 23:14 |
HuguesRoss |
I'll do it in 15 otherwise |
| 23:14 |
ShadowBot |
https://github.com/minetest/minetest/issues/11699 -- Disable dynamic shadows for 5.5.0 release by SmallJoker |
| 23:14 |
MTDiscord |
<MisterE> ^MTG, mineclone |
| 23:14 |
Wuzzy |
the reason why I made it a priv was because I actully wanted to have a more "clean" way to expose coords to players, completely indepentend from debug screen |
| 23:14 |
Pexin |
most server owners wont care, and players will suffer |
| 23:14 |
rubenwardy |
HuguesRoss: go ahead |
| 23:14 |
Wuzzy |
like with hud_add or even a "coordinate key" |
| 23:14 |
MTDiscord |
<MisterE> Ill object, if no one else wants to |
| 23:15 |
MTDiscord |
<exe_virus> Lol, no feel free to keep shadows off by default, but enable-able |
| 23:15 |
sfan5 |
Pexin: are there server owners who will update to 5.5 but not maintain their game? |
| 23:15 |
Wuzzy |
yes |
| 23:15 |
MTDiscord |
<exe_virus> That's on them? |
| 23:15 |
Wuzzy |
absolutely |
| 23:15 |
erlehmann |
MisterE object to what? |
| 23:15 |
MTDiscord |
<exe_virus> How many Mt servers are there? 25? |
| 23:15 |
Wuzzy |
hundreds |
| 23:15 |
MTDiscord |
<exe_virus> That actually have players |
| 23:15 |
Pexin |
sfan5: hmmm.. |
| 23:15 |
erlehmann |
; minetest-servers name |wc -l |
| 23:15 |
erlehmann |
346 |
| 23:16 |
sfan5 |
like 50 with active players roughly |
| 23:16 |
Wuzzy |
yes, exactly 50 right now |
| 23:16 |
sfan5 |
that is without subtracting multicraft |
| 23:16 |
MTDiscord |
<exe_virus> Of those, how many will care and not be able to deal with the change? 4? |
| 23:16 |
sfan5 |
or 0.4 |
| 23:16 |
MTDiscord |
<MisterE> HuguesRoss: to be clear, does this PR allow you to toggle on shadows with an advanced setting? |
| 23:16 |
Wuzzy |
oh wait its a bit ore than 50 but whateveer |
| 23:16 |
sfan5 |
@MisterE it does not |
| 23:17 |
MTDiscord |
<MisterE> then ir seriousness, I kinda object |
| 23:17 |
MTDiscord |
<MisterE> *In |
| 23:17 |
MTDiscord |
<exe_virus> So just a straight up, rip it out? At least let players experiment with it |
| 23:17 |
erlehmann |
; minetest-servers clients |cut -f2 |grep -v '^0$' |wc -l |
| 23:17 |
erlehmann |
104 |
| 23:17 |
Wuzzy |
dumb question: will auto-granting basic_debug in builtin work? |
| 23:17 |
erlehmann |
currently 104 servers have at least 1 client |
| 23:17 |
MTDiscord |
<exe_virus> Yep, and then games can override that behavior |
| 23:17 |
MTDiscord |
<MisterE> yes, I really think that you should be allowed to test the shadows with an advanced setting |
| 23:17 |
sfan5 |
we've had this already, players can experiment with a dev build |
| 23:17 |
sfan5 |
it absolutely must be disabled in the release |
| 23:17 |
MTDiscord |
<exe_virus> Oh, it breaks things? |
| 23:18 |
HuguesRoss |
@exe_virus I believe the intent is to rip it out and re-enable it immediately after release |
| 23:18 |
sfan5 |
yes, it will be |
| 23:18 |
Wuzzy |
this is also an option |
| 23:18 |
erlehmann |
you have to actually double the number of servers probably |
| 23:18 |
Wuzzy |
nuke then rebuild from the ashes ? |
| 23:18 |
MTDiscord |
<exe_virus> Players don't have the ability to run Dev builds. Like less than 10% haha |
| 23:18 |
erlehmann |
about half the servers i play on are not in the public server list |
| 23:18 |
MTDiscord |
<MisterE> if you want to explain why it absolutely must be completely disabled in the release, im interested... if not, proceed |
| 23:18 |
sfan5 |
I feel sorry for whoever has to read the logs of today |
| 23:19 |
sfan5 |
@MisterE the gist is games have no control over users enabling it, ruining the art direction of games |
| 23:19 |
sfan5 |
for details look for hecktest's issue |
| 23:19 |
HuguesRoss |
#11365 to be precise, there's been a lot of back-and-forth |
| 23:19 |
ShadowBot |
https://github.com/minetest/minetest/issues/11365 -- Shadowmaps cannot be disabled by the server |
| 23:19 |
MTDiscord |
<MisterE> I have read that in detail. THat does not give a reason why it should not be in the advanced settings |
| 23:20 |
MTDiscord |
<MisterE> imo |
| 23:20 |
MTDiscord |
<exe_virus> Wait, that's why? I can override art with texture packs already |
| 23:20 |
Pexin |
personally, i'm more concerned about whether performance is impacted even when disabled in menu. is this still the case? |
| 23:20 |
Wuzzy |
tbh shadowmaps isn't the core of the problems actually. the core is poor usability overall. settings are such a mess |
| 23:20 |
sfan5 |
don't think so |
| 23:21 |
MTDiscord |
<MisterE> I mean, 3d mode is/was broken, and also completely ruins the experience for many of the settings. YOu can enable 3d mode in advanced settings, and it will break every game you try to play. And its on you. |
| 23:21 |
MTDiscord |
<MisterE> *settings -> i meant games |
| 23:21 |
MTDiscord |
<MisterE> same with shadows |
| 23:21 |
Wuzzy |
is it marked as experimental at least? then it should be fine, right? |
| 23:21 |
sfan5 |
past mistakes, future mistakes etc. honestly this is a done deal |
| 23:21 |
MTDiscord |
<MisterE> yeah |
| 23:22 |
sfan5 |
there are more important issues to be discussed |
| 23:22 |
Pexin |
wasnt 3d mode fixed? |
| 23:22 |
sfan5 |
it was |
| 23:22 |
erlehmann |
shadows occur in the end in mcl* mods and it sucks |
| 23:22 |
erlehmann |
MisterE that argument shoud be enough |
| 23:23 |
erlehmann |
sunless sky, deep underground, still you get shadows |
| 23:23 |
erlehmann |
understand? |
| 23:23 |
MTDiscord |
<MisterE> sfan5: could you at least make a build for windows, debian, and ubuntu that has shadows enableable, but is otherwise 5.5? That would be greatly appreciated I think |
| 23:23 |
MTDiscord |
<Jonathon> thats literally against the point of disabling them |
| 23:24 |
sfan5 |
you can download all of those from a gitlab pipeline as soon as the change is reverted for 5.6 :) |
| 23:24 |
MTDiscord |
<MisterE> eal: yes I completely understand. And if someone had to go digging thru the settings enable shadows, then they can go dig thru the settings to disable them again too when they realize that their personal choice has broken their experience |
| 23:25 |
erlehmann |
hey hey rubenwardy ar shadows in ctf cheating? |
| 23:25 |
Wuzzy |
so what's the stance on basic_debug now? nuke? try to make a workaround? do nothing? |
| 23:25 |
erlehmann |
after all you can basically see around corners with them |
| 23:25 |
erlehmann |
nuke nuke nuke nuke |
| 23:25 |
sfan5 |
Wuzzy: nothing yet, more opinions needed |
| 23:26 |
MTDiscord |
<luatic> why should basic_debug be nuked? |
| 23:26 |
MTDiscord |
<Jonathon> im wondering at what point does erlehmann get muted again |
| 23:26 |
MTDiscord |
<luatic> because it removes coords? |
| 23:26 |
sfan5 |
I'll post in the issue and try to summarize it |
| 23:27 |
MTDiscord |
<MisterE> sfan5, could you make the pipelines more official? Ι tried to show that to someone as a way to try out the latest version, and they doubted that is was safe |
| 23:28 |
sfan5 |
dunno how we'd go about that |
| 23:28 |
Pexin |
fwiw, I rely on F5 to know what node I'm pointed at a lot |
| 23:28 |
sfan5 |
I believe https://gitlab.com/minetest/minetest is mentioned as our official mirror somewhere |
| 23:29 |
MTDiscord |
<MisterE> sfan5 like a link on the minetest website that has a download button for the latest dev build. Plenty of FOSS programs do this, like gimp, blender, etc |
| 23:30 |
sfan5 |
hm |
| 23:32 |
MTDiscord |
<MisterE> a benefit is that more people will try out the latest so you will catch bugs faster |
| 23:33 |
MTDiscord |
<MisterE> anyhow, for the record, I dont like shadows being completely disabled for 5.5, and I dont want the basic_debug changed, and now go ahead and do those things if you want cause, really, its not all that important |
| 23:41 |
Wuzzy |
https://github.com/minetest/minetest/pull/12014 |
| 23:42 |
Wuzzy |
#12014 |
| 23:42 |
ShadowBot |
https://github.com/minetest/minetest/issues/12014 -- Update builtin translation by Wuzzy2 |
| 23:42 |
Wuzzy |
(I assume we're in string freeze) |
| 23:43 |
sfan5 |
basically |
| 23:47 |
erlehmann |
HuguesRoss https://github.com/minetest/minetest/pull/11866#issuecomment-1025007529 |
| 23:53 |
sfan5 |
Krock HuguesRoss opinions wanted -> https://github.com/minetest/minetest/issues/12011#issuecomment-1025009227 |
| 23:54 |
sfan5 |
~tell v-rob opinions wanted -> https://github.com/minetest/minetest/issues/12011#issuecomment-1025009227 |
| 23:54 |
ShadowBot |
sfan5: An error has occurred and has been logged. Please contact this bot's administrator for more information. |
| 23:54 |
sfan5 |
thanks for nothing, ShadowBot |
| 23:54 |
sfan5 |
^ @luatic here's what was being discussed anyway |
| 23:56 |
MTDiscord |
<luatic> thanks |
| 23:57 |
erlehmann |
sfan5 this commit seems to be entirely unrelated to the PR it is in. any reason it's there? https://github.com/minetest/minetest/pull/11866/commits/197c40ca86fc1a73c3f1bcdf37f5518ec8c95e39 |
| 23:57 |
sfan5 |
convenience |
| 23:58 |
erlehmann |
well i hope you do not squash this |
| 23:58 |
sfan5 |
I hope so too |
| 23:59 |
erlehmann |
i do not wish to go into it much rn, but i have actually had a fair bit of problems in other projects with topical branches containing cleanup work. it makes porting changes a bit difficult. |
| 23:59 |
erlehmann |
i can talk about this later, if you want to, it's … uh … unpleasant. |
