Minetest logo

IRC log for #minetest-dev, 2021-12-18

| Channels | #minetest-dev index | Today | | Google Search | Plaintext

All times shown according to UTC.

Time Nick Message
01:15 v-rob joined #minetest-dev
02:33 v-rob joined #minetest-dev
03:29 queria joined #minetest-dev
03:33 queria joined #minetest-dev
03:46 v-rob joined #minetest-dev
04:29 v-rob joined #minetest-dev
04:34 v-rob joined #minetest-dev
05:00 MTDiscord joined #minetest-dev
06:17 v-rob joined #minetest-dev
06:58 v-rob joined #minetest-dev
08:36 nrz_ joined #minetest-dev
09:03 calcul0n_ joined #minetest-dev
09:31 Fleckenstein joined #minetest-dev
10:31 YuGiOhJCJ joined #minetest-dev
11:01 tekakutli joined #minetest-dev
11:55 Fixer joined #minetest-dev
12:11 proller https://github.com/minetest/minetest/pull/11843 - simple type rename!
12:15 MTDiscord <Sublayer plank> it is also woefully untested!
12:17 v-rob joined #minetest-dev
12:26 calcul0n__ joined #minetest-dev
12:29 proller its okay.
12:31 MTDiscord <Sublayer plank> does it still segfault when reaching 31000 in any given direction?
12:32 erlehmann uh
12:32 erlehmann that should not happen at all
12:33 proller nothing changed by default.
12:33 MTDiscord <Sublayer plank> I was able to go beyond in devtest although in MTG it segfaults
12:33 erlehmann prove it with unit tests
12:34 proller but if you compile with USE_POS32=1 - you can walk to any coords up to 2^31
12:34 nrz_ it's just a typedef replacing types, it's pretty simple, you just can break the game mayby by changing the map coordinates
12:34 erlehmann no one cares about that
12:34 erlehmann AFAIK minetest has *never* crashed when reaching 31000
12:34 erlehmann and also, again, it is ridiculously stupid to stop any behaviour at 31000
12:35 nrz_ proller i suggest on your PR to just comment out or add a cmake warning for people configuring USE_POS32 that it's very experimental
12:35 nrz_ and also not retrocompatible with old clients
12:35 erlehmann i suggest proller you do actually add in proper boundary checks
12:35 nrz_ or regular clients ?
12:35 erlehmann yeah lol
12:35 nrz_ proller what happen if you connect a MT client with USE_POS32=0 on a server with USE_POS32=1 ?
12:36 nrz_ i think we should have a crash, same for inverted flags i think
12:36 proller actually you will stop at some 1000000 positions because float precision - increment on small values gives old value, but can walk fast or use teleport
12:36 erlehmann proller that has to be fixed anyway
12:37 proller nrz_, option(USE_POS32 "32 bit positions (experimental, not compatible with 16 bit)" FALSE)
12:37 erlehmann even at 32767 the delta on floats is too much for some mods
12:37 nrz_ i think if we only have type rename and we discourage people to increase for a moment it can be nice yeha
12:37 proller nrz_, cant connect because protocol packets have bigger length
12:37 nrz_ at a moment maybe POS instead of a typedef should be a normal structure with backward compat
12:37 nrz_ proller: what i thought
12:38 nrz_ tbh proller i like your PR but we should merge it after 5.5 release
12:38 nrz_ and after that maybe we can worker harder on the engine in order to manage both compat modes
12:38 proller erlehmann, i making option to switch floats to doubles by same way, its already works except some bugs
12:40 erlehmann proller ok but PLEASE add in proper boundary checks. the problem is that minetest devs never add in the correct checks afterwards
12:40 proller but good solution is use int base coord + float exact coord lime m_camera_offset  - but for all positions calculations
12:40 nrz_ it's not really the purpose of its PR, it's just a rename, it doesn't reduce or increase code risks
12:40 erlehmann and by never, i mean “the general attitude is that putting some arbitrary limit somewhere in the stack will suffice”
12:40 erlehmann nrz_ of course it does increase risks if a lot of the conversion code implicitly assumes 16 bit
12:41 proller erlehmann, there is still LOT OF todo after this pr, it just makes all further work possible
12:41 nrz_ it's why i think we should just keep this PR for the 5.6 merge window (and sorry proller, it's a good PR but not the good moment to merge it)
12:41 erlehmann proller, what i am telling you is if you do not get it the right time it takes a lot of effort to fix tihs
12:41 erlehmann proller, what do you think of my idea of handling old clients by fake teleporting them?
12:42 erlehmann that would give you backwards compat, but break map saving
12:42 nrz_ erlehmann, i suggest to just stop talking about this PR as it's not the good moment to work with it, as all you request is more improvement we must add after reviewing & merge it
12:42 proller maybe ok, need to try, simplest is teleport them to spawn
12:44 proller maybe break completely network code for 6 release?
12:44 proller 1. switch to usrsctp
12:44 proller 2. stole network packets from freeminer OR use protobuf
12:45 erlehmann nrz_ you just need to look at https://github.com/minetest/minetest/pull/11858 to see that the simplest bugfix ever has no chance of being merged if there is a somewhat workable (but buggy) solution
12:45 erlehmann therefore proller should try to do everything right the first time IMO
12:46 erlehmann nothing lasts as long as a provisorium
12:48 erlehmann proller breaking backwards compatibility is often the solution if ppl are not motivated or able to figure out better ideas. users don't really love it.
12:49 proller users still suffers from slowest networking code
12:49 erlehmann proller, sctp snice though
12:50 erlehmann devs keep saying that thing, but minetest is surprisingly playable on a 15 year old laptop with slow internet, once the cache is filled with textures and models and so on
12:50 proller not true sctp, it not completely supported, just sctp over udp with all sctp benefits
12:50 erlehmann i know i know
12:50 erlehmann you need to do that bc windows can't do it right?
12:51 proller not only windows, it can be removed from android kernel, can be banned by network providers
12:52 erlehmann oof
12:52 erlehmann well
13:21 celeron55 enet was another option that was tested to give better transfer rates, i think the problem back then was no official ipv6 support and questionable project maintenance, would have had to make a fork
13:23 celeron55 usrsctp seems pretty good
13:24 celeron55 i would like to see performance numbers comparing USE_POS32 enabled and disabled
13:29 proller joined #minetest-dev
13:51 Fleckenstein joined #minetest-dev
14:04 erlehmann celeron55 btw regarding your reservations regarding the performance impact of bounds checks, would you accept the output of the --speedtests option as evidence if something does or does not impact performance?
14:05 erlehmann because of course you are right that more checks impact performance
14:06 erlehmann some people actually think that -fwrapv is an easy cop-out for making algorithms bounds-safe
14:06 erlehmann but it inserts bounds checks EVERYWHERE lol
14:07 erlehmann so the queston i have is just what benchmark would satisfy you
14:07 erlehmann ig --speedtests is ok i'll go do my research
14:39 Taoki joined #minetest-dev
15:38 m42uko joined #minetest-dev
17:01 MTDiscord <luatic> I don't get how https://github.com/minetest/minetest/pull/10329 ever got merged. TL;DR: - We're not sticking to math conventions (scalar multiplication, vector addition), as scalar addition/subtraction stayed accepted - Dubious claims of the Schur product being useless were taken for granted, no "vector.schur_product" / "vector.scale" or the like were added - Upvotes / downvotes appear to have been ignored Now that I need Schur product
17:01 MTDiscord / Schur quotient, I consider it slightly frustrating that it got marked as deprecated. Please revert.
17:12 celeron55 erlehmann: --speedtests doesn't do anything useful for this
17:13 celeron55 a test for this should run the mapgen on the server, with lighting generation, and then the meshgen on the client - i think those are some of the heaviest users of node coordinates
17:15 celeron55 networking doesn't particularly concern me altough it could (maybe should) concern someone
17:20 erlehmann tbh networking wise the very impactful change “do not send lua entity or player properties if they are set to a value that they already have and emit a warning, so mods can fix their the ridiculous globalstep loops that do this kind of nonsense” is taken about as seriously as all my other proposals, i.e. not at all, despite mineclonia proving that this naive approach can reduce traffic by several orders of magnitude.
17:21 erlehmann i.e. cora has managed to make “idling in a singlenode world” produce about 100 bytes per player per second instead of 10kb per player per second
17:21 erlehmann (both playerplus and the engine are at fault here, since bones seem to move without playerplus, but playerplus also spams bone position setting)
17:23 erlehmann fixing that is a) easier b) more impactful … than changing some network protocol in an incompatible way
17:35 Fleckenstein joined #minetest-dev
17:44 celeron55 that change sounds reasonable
17:48 MTDiscord <Warr1024> I myself made that change but didn't do the warning.  The API is instead intended to just accept naive set calls and accepts responsibility for making it so.
17:49 MTDiscord <Warr1024> Specifically this is regarding set_properties, I don't think I touched bone stuff.
17:49 erlehmann the problem of doing it without the warning is that there is a lot of code that becomes expensive no-ops
17:49 MTDiscord <Warr1024> unless that's in there.
17:49 erlehmann technically it is already expensive no-ops
17:50 erlehmann almost every setting of a property that already has the value to the same value that i debugged happened in some kind of logic that could be entirely avoided if you sth is checked before
17:50 MTDiscord <Warr1024> In my experience much of what I need to do to determine whether the properties need to be changed or not is what I need to do to determine what to change them to.  That would mean that I'd just basically add another layer of the same logic to fix the warnings.
17:51 erlehmann well for stuff like field of view you are not really forced to check it before
17:51 MTDiscord <Warr1024> I'm basically already doing the check, and that's what my patch IS.  I moved the check into set_properties because it made no sense to leave it outside and thus risk not having all cases covered.
17:52 erlehmann Warr1024 are you rounding floaty values though?
17:52 erlehmann cora had to round the bone positions to 2 digits after the decimal point
17:52 erlehmann bc the engine does something fucky with some float values (not normal rounding)
17:52 MTDiscord <Warr1024> I'd be fine with making the warning optional, e.g. by sending a allow_same=true property or something, but if it's mandatory then I'll end up just patching around the patch with a de facto copy of the patch anyway.
17:52 MTDiscord <Jonathon> the engine really shouldnt be holding peoples hands. it should do as instructed. also if a game wants this they can override the api and do the check before passing the data onto the api
17:54 MTDiscord <Warr1024> The way I handle floats is I compare the ratio between them and allow them to be off 1 part in 100k
17:55 erlehmann ah
17:55 MTDiscord <Warr1024> If doing sane things by default is "holding peoples' hands" then yes, the engine absolutely should be doing that.
17:56 erlehmann i agree. i have seen the entity/player thing so often
17:56 erlehmann most devs do not even know this can lead to their mod being the difference between a player lagging with 5 players or wih 50 players
17:56 erlehmann a server lagging i mean
17:57 MTDiscord <Warr1024> If it were object:send_properties_packet_to_client() then it would make sense to blindly send the packet every time, but it's set_properties() which implies that it sets the state of a thing, and the engine should be responsible for determining how to replicate that state to the client.
17:57 erlehmann yeah also it should be idempotent in the first place
17:58 MTDiscord <Warr1024> set_* methods should probably all by implicitly idempotent.
17:59 erlehmann well, my experience is that working around such stuff in lua is safer than trying to get it fixed in the engine
17:59 MTDiscord <Warr1024> If we really want to expose things like sending packets or writing to disk to the modders then it seems like those should be their own APIs.
17:59 erlehmann so i am kinda ok with no one doing stuff about it
18:00 erlehmann bc if it goes as well as the map border stuff the chance is higher it would break sth than fix it
18:00 erlehmann (i would bet money that someone who approaches the network code finds an opportunity to insert “clever” further optimizations)
18:00 MTDiscord <Warr1024> I mean it should be the "engine's" responsibility in the sense that it should be fixed centrally instead of requiring everyone to install the same hack.  The "engine" to me includes builtin, and I think that may be the sanest place to implement this, and certainly the cheapest in terms of code maintenance costs.
18:01 erlehmann IIRC luatic and me have made this point in the past, but core devs disagree
18:01 MTDiscord <Warr1024> I don't like getting too clever, but set_properties seems close enough to no-brainer...
18:01 erlehmann the problem is not you
18:01 MTDiscord <Warr1024> Heh, I think I opened an issue about it myself at one point.  What other issues or PRs are there for it?
18:01 erlehmann the problem is the person who thinks: this is simple and there i can totally improve on it by 5% with this one simple trick
18:02 erlehmann which then is neither simple nor improves on it
18:02 celeron55 i think making mods simpler by allowing them to do stupid stuff by cutting their calls to no-ops on the C++ side is fine
18:02 MTDiscord <Warr1024> I've done that shit myself, I just have no qualms about throwing that work out if my hypothesis is disproven.
18:02 erlehmann celeron55 cool can you convince someone else of that too?
18:03 erlehmann i want to reiterate that having a warning is absolutely necessary for improving server performance
18:03 MTDiscord <Warr1024> I'm convinced.
18:03 celeron55 i don't agree with the warnings
18:03 MTDiscord <Warr1024> I want to reiterate that having a warning should be a decision that we should not force upon our users.
18:03 celeron55 requiring lua code to always check to not update to the existing value makes lua code look ugly
18:04 erlehmann that is an aesthetic argument though
18:05 MTDiscord <Warr1024> The engine's job is to enable game creators to create games.  We should not critique the quality of their work, especially in cases where we don't actually know whether the thing we're measuring relates to quality or not.
18:06 erlehmann i have yet to see a case where it was not an error to set something to the same value every server tick
18:07 MTDiscord <Warr1024> Basically all of NodeCore is a counterexample.
18:07 erlehmann oh tell
18:07 erlehmann i am curious
18:07 MTDiscord <Warr1024> just did
18:07 erlehmann i thought you had wrapped the method and that was it?
18:07 erlehmann mineclonia wraps stuff too
18:07 erlehmann with the wrapper, you should not get any warning then
18:07 MTDiscord <Warr1024> I thought we were making an argument about why the wrapping should be in core?
18:08 MTDiscord <Warr1024> If my having wrapped it in my game is going to still be necessary then putting it in core doesn't really help.
18:08 erlehmann i see
18:08 erlehmann well then not a warning, a DEBUG whatever
18:08 erlehmann just a way for devs to figure out if their game is running NOP loops
18:08 erlehmann idc about the loglevel
18:08 erlehmann but currently it is next to impossible to figure all that stuff out easily
18:09 MTDiscord <Warr1024> I'd be okay with a debug/verbose, i.e. anything below ACTION ... but I also don't want log spamming if it's happening constantly, so either only the first one, like "hey some mod is doing bad stuff" or summaries, like a "happened X times in the past T"
18:09 MTDiscord <Warr1024> The problem with WARNINGS is that some server owners take those quite seriously, and mods shouldn't have a bad mark for doing what they're supposed to.
18:10 MTDiscord <Warr1024> Ultimately it's the experience with using a mod is the mod author's responsibility and we can't do much to prevent most forms of dumb/naive, and this one is a case where there are significant use-cases that are NOT naive.
18:10 erlehmann "happened X times in the past T" would be perfectly fine
18:11 MTDiscord <Warr1024> Basically I don't want it as a warning because you're making a big assumption that letting engine/builtin do the deduping work is not ALREADY the most optimal it can be.  If you want to demand that an author improve something you need to be sure it CAN be.
18:11 erlehmann but warning on the first occurance would also help
18:11 erlehmann i understand
18:12 erlehmann i should add that all cases i have seen are cases that happened in player or entity steps.
18:12 erlehmann that probably narrows my perspective a lot
18:12 erlehmann because only those cases are likely to create ridiculous performance problems.
18:12 erlehmann i guess nodecore will not do such a folly
18:13 MTDiscord <Warr1024> Heh, NodeCore is tricky. I have 39 registered_playersteps, which are all run by 1 globalstep, which actually has something like 2 layers of automatic idempotence.
18:13 erlehmann celeron Warr1024 i am convinced a warning is too buch and debug/verbose is good. thx for explaining your positions.
18:14 MTDiscord <Warr1024> I'm sure it could be performance-optimized more, in theory, but performance isn't the only thing I have to worry about optimizing for.
18:14 erlehmann Warr1024 you are more careful and more talented than a lot of mod devs i guess, from all i have heard so far.
18:14 erlehmann Warr1024 if you ever do a writeup about all the cool optimizatons in nodecore, i'd love to know about it. especially regarding unexpected engine behaviour.
18:15 MTDiscord <Warr1024> The idea that I'm more careful than many is another reason why I think it makes a lot of sense to make the engine do the sanest things by default.  The fact that I may be more careful is also one reason why I'm skeptical about the value of warnings, i.e. the people who actually pay attention to them are already more likely to not need them.
18:15 erlehmann from my experience that last thing is not generally true
18:15 erlehmann ppl who had no idea about textures suddenly got into alpha shenanigans bc of the warnings
18:16 erlehmann and understood their own mods beter that way
18:18 MTDiscord <Warr1024> I suppose that could be.  I do take warnings seriously myself.  I just want to make sure that nobody feels compelled to chase down a warning that's actually counterproductive.
18:18 celeron55 a more pedantic engine might have a separate method for when you intend to set a new value vs. when you intend for the engine to check the value and determine whether anything has to be done, but it's not really a design MT uses and it seems bloated to me
18:18 MTDiscord <Warr1024> I ran into one with set_player_velocity being deprecated in 5.4+, but mandatory in 5.3-.  I had to put in a hack to make set_velocity work on players in 5.3- without breaking 5.4+
18:19 MTDiscord <Warr1024> At least in that case, though it really CAN'T be fixed in the engine/builtin
18:20 erlehmann celeron55 what you specify as a more pedantic engine to me looks like a cop-out that tries to outsource optimization to mod code, with predictably horrible results
18:22 celeron55 i mean it would have both options with different method names
18:22 erlehmann and i stand by my assertion
18:22 celeron55 but given lazy modders wouldn't care what method they'd use, you'd end up with the exact same problem
18:22 MTDiscord <josiah_wi> Warr, what's your opinion on the GCC warnings about memset or memcpy with a non-trivial type? Minetest has at least 2 IIRC.
18:22 erlehmann yeah
18:22 celeron55 as they'd use the wrong one
18:23 MTDiscord <Warr1024> I hate warnings and want to fix them all.  The question is really just (1) are we sure the warnings are valid for our situation, and (2) what cost would it be to fix them properly?
18:24 erlehmann josiah_wi do you mean the null pointer thing or something else?
18:25 erlehmann sfan5 could you maybe try to use the “possible close” label either less or add another tag to it more often? i have seen you use it for at least three purposes: when you could not reproduce a bug, when you were convinced something might not be a bug, when you consider an alternative proposal to be better.
18:26 erlehmann i am not sure what the solution here is, it just seems a bit confusing
18:26 MTDiscord <josiah_wi> I appreciate your sense of humor and your opinions on programming, Warr. I think I could learn a lot from you.
18:26 erlehmann Warr1024 can you write an AOSA chapter on nodecore?
18:29 erlehmann celeron55 how do you propose to move forward with the property setting idempotence performance idea so that it does not suffer the same fate as every map border discussion i have started?
18:29 erlehmann like what is the next step to get it into minetest
18:32 celeron55 someone has to make a PR that matches this discussion 1:1
18:32 celeron55 so as to not require a new discussion
18:34 erlehmann Warr1024 would you do that? while you have no need for this thing, i trust you more to start with something good than myself, since you have the wrappers already.
19:06 sfan5 erlehmann: dunno, do you think the label isn't accurate?
19:09 erlehmann sfan5 i think it is not helpful in grouping tasks by what needs to be done next. in particular, i would be happy if you would stop applying it as the sole tag to stuff you can not reproduce like he i386 floating point thing.
19:14 sfan5 I could use it more sparingly on unconfirmed bugs but I usually only apply it when I'm reasonably sure that it's not an actual bug
19:25 sfan5 merging #11859, #11867, #11868 in 10m
19:25 ShadowBot https://github.com/minetest/minetest/issues/11859 -- Restructure devtest's unittests and run them in CI by sfan5
19:25 ShadowBot https://github.com/minetest/minetest/issues/11867 -- [no squash] Mod security improvements by sfan5
19:25 ShadowBot https://github.com/minetest/minetest/issues/11868 -- Protect font initialization with mutex by sfan5
19:35 erlehmann sfan5 maybe just stop using it when you try to reproduce with a different setup than the submitter or sth dunno
19:35 erlehmann sfan5, it's not a big deal, i just thought maybe you immediately had a better idea how to handle it
19:37 MTDiscord <josiah_wi> erlehmann, I don't know whether you saw, but I've PRd some of Irrlicht's unit tests back to IrrlichtMT now.
19:37 erlehmann josiah_wi i did not know. link?
19:37 erlehmann thx for the notice
19:47 rubenwardy Ironically, logging a warning would make it slower than the redundant call
19:55 erlehmann well maybe just do it in a debug build or sth
19:56 erlehmann if dispatching the log message is a problem in itself
20:12 tekakutli joined #minetest-dev
20:40 MTDiscord <josiah_wi> erlehmann, irr#84
20:40 ShadowBot https://github.com/minetest/irrlicht/issues/84 -- Restore some unit tests by JosiahWI
20:56 erlehmann looks nice, but i do not have the energy to look into it much rn
21:33 proller joined #minetest-dev
22:09 Taoki joined #minetest-dev
22:14 book` joined #minetest-dev
22:33 proller joined #minetest-dev

| Channels | #minetest-dev index | Today | | Google Search | Plaintext