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 |