Time  Nick       Message
04:04 erlehmann  <sfan5> so have we determined that except tga nobody actually used the other formats that were removed?
04:05 erlehmann  sfan5 i don't know, do we have a list of stuff that was removed to compare with the list of stuff on cdb?
04:05 erlehmann  i only saw a list of stuff that remained
04:06 erlehmann  > you seem to think that BMP is easier to write than TGA, it's the other way around
04:06 erlehmann  lol good that someone brought that up
04:07 erlehmann  <pgimeno> I don't know if mineclone* needs to ever read back the data; if it does, and the data is in PNG format, that'd mean decoding the PNG on the Lua side, therefore a PNG writer might not be suitable
04:07 erlehmann  pgimeno is right, it might need to read back the data if the mapping table gets added
04:07 erlehmann  to zoom out i guess
04:07 erlehmann  i had not thought of that
04:10 erlehmann  i think the lesson learned is: to remove a fence you have to know who built it
04:19 erlehmann  why was this merged? https://github.com/minetest/irrlicht/pull/52 “Confirm it works on OSX OSX building is broken at the moment.” and “Android testing (lack of manpower; I tacitly assume it will work” … is this some kind of joke?
04:21 erlehmann  i mean good that hecktest documents this but HOW can such fundamental stuff that relies son “i assume it will work” be acceptable?
04:21 erlehmann  also it introduces a new crash?
04:22 erlehmann  i really feel lost here
04:22 erlehmann  <erlehmann> i think the lesson learned is: to remove a fence you have to know who built it
04:22 erlehmann  <erlehmann> i think the lesson learned is: to remove a fence you have to know why it was built
04:22 erlehmann  so
04:22 erlehmann  corrected myself
06:47 MTDiscord  <Sublayer plank> all minetest platforms apart from linux are supported on a case of whether anyone is willing to support it
07:00 pgimeno    @Warr1024 no, BMP can do it up-down too by using a negative vertical size, and TGA has a flag for the vertical direction
07:13 erlehmann  the BMP file format is a bit complex tbh https://en.wikipedia.org/wiki/BMP_file_format
07:14 erlehmann  lets say it is a mess to write anyway https://en.wikipedia.org/wiki/BMP_file_format#Pixel_storage
07:14 erlehmann  > The bits representing the bitmap pixels are packed in rows. The size of each row is rounded up to a multiple of 4 bytes (a 32-bit DWORD) by padding.
07:15 erlehmann  > The pixel array is a block of 32-bit DWORDs, that describes the image pixel by pixel. Usually pixels are stored "bottom-up", starting in the lower left corner, going from left to right, and then row by row from the bottom to the top of the image.
07:15 erlehmann  > Unless BITMAPCOREHEADER is used, uncompressed Windows bitmaps also can be stored from the top to bottom, when the Image Height value is negative.
07:15 erlehmann  > Padding bytes (not necessarily 0) must be appended to the end of the rows in order to bring up the length of the rows to a multiple of four bytes.
07:15 erlehmann  *not necesarily 0*
07:15 erlehmann  seriously
07:16 erlehmann  > A 24-bit bitmap with Width=1, would have 3 bytes of data per row (blue, green, red) and 1 byte of padding, while Width=2 would have 6 bytes of data and 2 bytes of padding, Width=3 would have 9 bytes of data and 3 bytes of padding, and Width=4 would have 12 bytes of data and no padding.
07:19 erlehmann  regarding the removal of the TGA writer … maybe you didn't ever use, but it *might* make manipulating maps faster? i have not tested how slow the pure lua thing is, so please do not believe anything regarding that. also if it was never used, there is no bug to fix here. https://github.com/minetest/irrlicht/pull/64/commits/e8032f84237b16d9756002cb3ccddacc6cccbef4
07:20 erlehmann  funny how small it is though
07:21 erlehmann  https://en.wikipedia.org/wiki/Truevision_TGA
07:21 erlehmann  > Uncompressed 24-bit TGA images are relatively simple compared to several other prominent 24-bit storage formats: A 24-bit TGA contains only an 18-byte header followed by the image data as packed RGB data. In contrast, BMP requires padding rows to 4-byte boundaries, while TIFF and PNG are metadata containers that do not place the image data or attributes at a fixed location within the file.
07:22 erlehmann  in conclusion, fleckenstein did nothing wrong: TGA does seem to be the easiest file format for any kind of generated texture
07:24 pgimeno    both formats are a bit complex to interpret, because of the many features, but they are very easy to write for a certain specific format, and if you assume you'll only read what you wrote, it's straightforward
07:25 pgimeno    by "for" I mean if you stick to just one
07:25 erlehmann  i am thinking of having treasure maps
07:25 erlehmann  that would be fun
07:26 pgimeno    but BMP has the drawback of the padding per scanline
07:26 erlehmann  the padding thing is obnoxious
07:27 pgimeno    anyway, we should probably stop flogging that dead horse, it's already been agreed that TGA will be put back
07:27 erlehmann  the best thing i learned about TGA: 4 bytes 	Postage stamp offset 	Number of bytes from the beginning of the file to the postage stamp image if present
07:27 erlehmann  pgimeno, i just think it is interesting
07:27 erlehmann  and important for everyone to understand why it was not a bad choice
07:28 erlehmann  it seems at least some devs are very eager to axe features they do not personally use because they have no idea why or how they were chosen my mod or game devs
07:28 erlehmann  helping people to understand that might prevent such stuff in the future
07:28 erlehmann  my → by
07:30 erlehmann  in general i think it happens often that people are eager to conjure baseless statements of “this is not needed” when they do not understand it.
07:31 erlehmann  i think at least some reactions to fleckensteins attempt to fix overlong item meta problems were of that nature.
07:31 erlehmann  (though it is true that this is mostly a mcl* game problem)
07:34 MTDiscord  <Sublayer plank> about the tga writer, best would probably be incorporating the lua tga writer into builtin and expose it to the api if there's enough demand for it to be provided by the engine, which I don't think there really is
07:35 erlehmann  well usually if game devs do some weird hacks to get around engine limitations it is ignored because it is too far out, or not?
07:36 erlehmann  at least that is my experience with the mcl* games, they are full of weird hacks (and not all of them good, of course)
07:36 erlehmann  i have the feeling the engine says what is possible and the gamedevs follow, not really the other way around
07:37 erlehmann  see particles, there are years of weird hacks and low performace
07:37 erlehmann  performance
07:37 pgimeno    I think that exposing a PNG reader to Lua would be enough
07:37 erlehmann  btw the “reading the file back” use case follows from the fact that the current map implementation shows the whole map, but to be EXACTLY LIKE MINECRAFT you would need to reveal only the parts near the player, piece by piece
07:38 erlehmann  and you can't just store the positions and render them, because the map is supposed to store what was seen at the time
07:38 erlehmann  (otherwise someone could grief the game world to grief your map art)
07:42 erlehmann  sfan5 i often switch between branches and worlds. is there a way to make minetest force to use old non-zstd format so i can keep using the same world?
07:42 erlehmann  otherwise “test if this works in 5.4 as well as 5.5” is a bit of a hassle
07:45 pgimeno    I think that the current PNG writer is only a quick hack and not flexible enough; it doesn't support paletted images for one
07:46 pgimeno    or RGB images, only RGBA
07:58 erlehmann  oh hi
07:59 celeron55  pgimeno: the reasoning for that was png will compress generally just fine with rgba, even if you don't use alpha or use only a few colors
07:59 celeron55  complexity for no good reason
08:00 pgimeno    doesn't the presence of an alpha channel force the use of transparency?
08:00 erlehmann  why do you hate palette cycling? ;)
08:01 pgimeno    I like palettes, but not because of their cycling capabilities; in this case for their ability to represent a block colour in just one byte
08:02 erlehmann  actually i think node variants are also pretty cool with palettes, but i am not sure how that is handled
08:02 pgimeno    PNG does not compress all that well if filters are not used, by the way - libpng contains heuristics for guessing what filters will compress best
08:03 erlehmann  yes filters are very good
08:03 pgimeno    and of course, filters are unused in hecks' implementation
08:03 erlehmann  http://www.libpng.org/pub/png/book/chapter09.html#png.ch09.div.3
08:04 erlehmann  celeron55 pgimeno which leads me to the question “has the performance of hecks PNG writer ever be examined at all”
08:04 erlehmann  like how good it compresses etc.
08:05 celeron55  no, it was chosen because of the simplest possible implementation
08:05 celeron55  you can propose to replace it with something else that you deem better
08:06 pgimeno    er, TGA?
08:06 celeron55  however all this palette arguing doesn't make much sense as MT doesn't support any kind of palette tricks
08:06 * pgimeno  hides
08:06 erlehmann  celeron55 the only palette trick i have seen is the grass node in mcl* games
08:06 celeron55  (only the better compression argument applies)
08:07 erlehmann  the better compression argument is supported by real world tests though
08:07 pgimeno    celeron55: with the palette I'm thinking about 1) compressed size and 2) reading back the PNGs. If you have to expand the RGBs of an image which is "naturally" a palette image, you don't get so much compression.
08:07 erlehmann  the thing i linked shows that unoptimized PNGs can be often bigger than GIF files, but optimized beat GIF by a noticeable margin usually
08:08 erlehmann  > But perhaps the most striking feature of Table 9-6 is just how poorly the original encoder did on its PNG images. Realizable savings of 40% to 75% are unusual, but thanks to poor encoding software, they are not as unusual as one might hope.
08:09 erlehmann  > the owner of http://www.feynman.com/ found that when he converted 54 nonanimated GIFs to PNGs, the collection grew in size from 270,431 bytes to 327,590 bytes. Insofar as all of the original images had depths of 8 bits or less--and even the worst PNG encoder will, on average, do as well or better than GIF on colormapped PNG images--the most likely explanation for the 21% increase in size is that the conversion
08:09 erlehmann  utility produced 24-bit RGB PNGs
08:10 erlehmann  it also gives tips for programmers
08:10 erlehmann  > Set the compression and filtering options intelligently
08:10 erlehmann  > For programs that use libpng (discussed at length in Part III, "Programming with PNG"), this is not a serious issue; it will automatically do the right thing if left to itself. But if you are writing custom PNG code, follow the guidelines in the PNG specification for matching filter strategies with image types.
08:10 erlehmann  > In particular, use filter type None for colormapped images and for grayscale images less than 8 bits deep. Use adaptive filtering (the ``minimum sum of absolute differences'' heuristic) for all other cases.
08:11 erlehmann  IMO if you can depend on libzstd for performance gains, you can depend on libpng for the same
08:11 celeron55  as i see this, all that's needed basically is palette support for compression reasons
08:11 erlehmann  the latter is battle tested and available everywhere
08:11 celeron55  all else is minor
08:11 erlehmann  well, filter suspport is important for gradients i think
08:12 celeron55  but libpng could be used instead i guess
08:12 pgimeno    filter support is only important for compression
08:12 celeron55  someone has to design the API for it
08:12 celeron55  the rest is just an implementation
08:12 erlehmann  if i remember correctly, writing PNG with gradients will blow up file size quite a bit unless you use filters
08:13 erlehmann  let me look it up
08:13 celeron55  this does bring libpng as a dependency to the server, which didn't have it before, but i guess that's not a huge deal
08:14 pgimeno    erlehmann: ah yes, I thought you meant gradient effects
08:14 erlehmann  pgimeno no, this section contains an image https://en.wikipedia.org/wiki/Portable_Network_Graphics#Filtering
08:14 erlehmann  here look at this amazing thing https://en.wikipedia.org/wiki/File:PNG-Gradient.png
08:14 erlehmann  > A small PNG file with a color gradient, demonstrating the effect of prefiltering on file size.
08:15 erlehmann  128 × 68 pixels, file size: 251 bytes, MIME type: image/png
08:15 erlehmann  > A PNG with 256 colors, which is only 251 bytes large with pre-filter. The same image as a GIF would be more than thirteen times larger.
08:15 erlehmann  celeron55, now imagine the same thing as an RGBA image
08:16 erlehmann  with no filters
08:17 celeron55  so far i'm mostly imagining you won't have energy left for implementation after spending it all arguing
08:18 celeron55  but yes, i can see it will make a very large difference in a very large set of images
08:18 sfan5      erlehmann: you can edit src/serialization.h to only write format 28, this will keep your map compatible
08:18 erlehmann  ok i just tested this and while the clever PNG above is 251, the stupidest equivalent of it that i could make was 26284
08:19 erlehmann  for storing 128 (W) × 68 (H) × 4 (RGBA) = 34816 bytes of payload + header
08:20 celeron55  that is mostly because all of the rows in the image are equal though
08:20 celeron55  in a real world (well, minetest world) example that's usually not the case, altough the filter will definitely help
08:21 erlehmann  i am literally trying to work out upper and lower boundaries, of course i am using the worst and best case
08:21 erlehmann  i can try some minetest textures though
08:22 erlehmann  i will do now
08:22 celeron55  the png writer wasn't really intended for writing regular textures, just some special ones
08:23 celeron55  as you extend the indended use case here, the favoured implementation changes
08:23 celeron55  obviously
08:23 erlehmann  hmmm, should i test with maps?
08:23 celeron55  i would like it if you test it with something that belongs to its use cases today, like the maps
08:24 erlehmann  ok
08:24 celeron55  you can alter the outcome by applying a background texture to the maps of course
08:24 celeron55  in which case it will perform about the same as just testing on the background texture
08:24 erlehmann  so far i was mostly trying to emphasize the point that a naive PNG encoder will not be of that much use
08:24 erlehmann  a background texture?
08:24 erlehmann  in game there are overlays
08:25 celeron55  well that's a wise choice
08:25 erlehmann  the texture will need to be zoomed out again once the mapping table is implemented i guess
08:26 erlehmann  let me check
08:42 erlehmann  https://gist.github.com/appgurueu/63ffccbdd72c18506dc22d5206394de1 shows many md files
08:42 erlehmann  but if md is so good, why is there no md2? o.0
08:43 MTDiscord  <Sublayer plank> questions science still can't answer
08:44 erlehmann  checkmate atheists
08:45 erlehmann  sfan5 btw you asked about md2 animations i think. as far as i see, it mostly means vertex animations, i.e. nothing has bones, basically you interpolate between different static phases of a model.
08:46 erlehmann  sfan5 you have to ask appguru about what exactly they plan to achieve with it though
08:46 erlehmann  if b3d does not offer that (which i do not know about), it could be important for some kind of expanding, contracting, or “organic looking” animation
08:47 sfan5      the important question is do these animations even work with MT?
08:48 erlehmann  that's a weird question, the answer is of course “no”, since the code for it was recently removed. or do you mean if they ever *did* work?
08:49 erlehmann  i can look that up, but first i finish the map-as-png-comparison
08:49 pgimeno    erlehmann: what is md and what is md2? I thought md is markdown and md2 is quake model, totally unrelated
08:49 sfan5      I'm sure you can successfully use your intuition to determine what I meant
08:53 erlehmann  pgimeno, it was a joke
08:53 erlehmann  pgimeno, basically the joke format goes “if X is so good, why is there no X 2”, solve for X
08:54 sfan5      if Minetest 5.4 is so good why is there no 5.4.2
08:54 pgimeno    ah k
08:54 erlehmann  and there were indeed many markdown files but no quake2 models
08:54 erlehmann  tbh i'd rather work with vertex animation than with bone animation for stuff that is not a mob
08:55 erlehmann  like “breathing” plants or something
08:55 erlehmann  or explosions that are not particle effect clouds
08:56 erlehmann  earth 2150 did vertex animation explosions amazingly in 1999: their explosions were basically expanding half-transparent textured … spheres would be an exaggeration
08:56 erlehmann  it only ever looked janky if you paused the game and rotated around it, but even then it had its charme
08:56 erlehmann  i wonder what jordach thinks about it
08:59 erlehmann  celeron55 quite unsurprisingly, naive TGA RGB maps are about 75% of the size of naive PNG RGBA maps
08:59 erlehmann  i have a hunch why lol
09:05 erlehmann  celeron55 for experimentation purposes, i have made a zip file with 2 TGA files from the file cache and a) the stupidest RGBA version i could make b) the smartest version i could make c) the version where i applied pngcrush(1) to the smartest version, yielding a further 0.2% to 0.6% of file size savings: https://mister-muffin.de/p/eZmy.zip
09:06 erlehmann  this also would not have been easily possible if the “store the cache in a sqlite db” PR had been merged. i hope it never will be.
09:06 sfan5      so where those files written using Minetest's PNG encoder?
09:06 sfan5      were* damn english
09:07 erlehmann  no, but that is why i provided you with the source files too.
09:07 erlehmann  they were generated with gimp
09:09 celeron55  what exactly is "the stupidest RGBA version i could make?"
09:09 erlehmann  0 compression
09:09 erlehmann  vs 9 compression
09:09 erlehmann  also i forced pixel format to RGBA
09:10 celeron55  0 compression makes absolutely no sense in this comparison
09:10 erlehmann  look i am still trying to work out boundaries of the format, i.e. “how bad or good can RGBA PNG be?”
09:11 celeron55  what is the compression used in the PNG writer? is it something like 6 by default
09:11 erlehmann  turns out, it can be very good, like 10% or 20% of the naive TGA file size
09:11 celeron55  you have just absolutely proven boundaries don't matter at all
09:11 erlehmann  i guess the conversation moved from “is PNG is always a good format?” to “the writing code matters”
09:12 erlehmann  celeron55 where would said png writer code be located?
09:12 erlehmann  i can look again
09:12 pgimeno    it has a parameter for compression level, I guess the default is whatever zlib's default is
09:12 sfan5      celeron55: 4 by default, configurable from lua
09:12 celeron55  well, i just found it by typing find src -iname *png*
09:12 erlehmann  ok
09:12 erlehmann  thx
09:12 pgimeno    also https://github.com/minetest/minetest/pull/11485/files
09:12 erlehmann  thx
09:15 erlehmann  does anyone have games/devtest/mods/testnodes/textures/testnodes_generated_*.png for me?
09:16 sfan5      if you join a devtest world you'd have those yourself
09:17 erlehmann  yeah but then i have to recompile the client again and i am *still* struggling to get to a state where TGA works again because of the horrible PR that removed it and probably me being too stupid to figure out which minetest / irrlichtmt combination restores it
09:17 erlehmann  (i want to play online and i kinda have difficulties if i can't see maps)
09:18 celeron55  oh yeah, compiling minetest or anything at all on the core duo... must be enjoyable
09:18 erlehmann  its not particularly slower than on a newer machine tbh
09:18 celeron55  it absolutely is :D
09:18 erlehmann  well, things on computers are either instant, take under a minute, or are “let's take a nap”
09:19 erlehmann  and compiling minetest on any machine i have is naptime
09:19 celeron55  the CPU is literally 20 times slower than that of a regular laptop of today
09:19 erlehmann  yeah but the annoyance at long running processes is not linear
09:20 MTDiscord  <Sublayer plank> here's testnodes_generated_{ck,mb}.png
09:20 MTDiscord  <Sublayer plank> https://cdn.discordapp.com/attachments/747163566800633906/885816921352732672/textures.zip
09:20 erlehmann  like if something takes 5 or 15 minutes is much less important than if it takes 0.3 seconds or a second
09:20 erlehmann  thx
09:20 MTDiscord  <Sublayer plank> just from a quick glance, the generated checkerboard image has little to no compression at all
09:21 MTDiscord  <Sublayer plank> 4,9kb is really large for a 512x512 image that's just a basic checkerboard pattern, the zip file compresses it down to just 284 bytes
09:21 erlehmann  lol yes
09:22 erlehmann  you can see in my zip file that the pngcrushed zip files are not compressible further
09:22 sfan5      an uncompressed 512x512 RGBA image would be 1 MB
09:22 erlehmann  well … there are repeating rows. a lot of them ;)
09:22 erlehmann  and repeating pixels
09:25 erlehmann  celeron55, how do you get to “20 times lower” btw? core duo has 1.83 GHz, the work machine i have access to (i.e. literally belonging to my employer) has a core i7 with 1.80GHz
09:26 erlehmann  celeron55, how do i compare this fairly?
09:26 MTDiscord  <Sublayer plank> processor frequencies are generally not something you can compare between generations
09:27 MTDiscord  <Sublayer plank> even when taking into account the amount of cores and threads
09:27 erlehmann  compare bogomips?
09:27 sfan5      like this for example https://www.cpubenchmark.net/compare/-/730vs1937
09:28 erlehmann  core duo has 3657 bogomips, core i7 has 3984 bogomips, according to linux
09:29 erlehmann  i guess it is not a good metric for speed if different generations either
09:29 sfan5      it's called "bogomips" because it's not an actual indicator of how many million instructions per second your cpu does
09:29 erlehmann  ah
09:29 erlehmann  bogus
09:30 erlehmann  sfan5 thank you for the cpubenchmark link. but according to that, the core i7 is like 7 times the speed of the core duo, not 20 times.
09:30 pgimeno    can we get back to discussing minetest development, please?
09:30 erlehmann  yes back to the png stuff
09:31 sfan5      erlehmann: unless I guessed your exact CPUs the result of "the" core i7 is irrelevant, compare the exact models
09:31 erlehmann  ok
09:32 sfan5      pgimeno: if productive discussion was happening here the png topic would be long gone because it's obvious
09:33 pgimeno    I'm all for dropping it, I think I've made my points clear enough
09:38 erlehmann  ok, last message for the PNG topic for now, since just worked it out: the 5k checkerboard PNG can be 236 bytes (LOL), the 123K fractal PNG can be 119K bytes, both proven using optipng. using GIMP turned out to be a bad idea, since it strangely seems to *increase* the file size of the fractal picture. as always, benchmark.
09:38 erlehmann  (preferably with real world tasks like minetestmapper maps, not fractals and checkerboards)
09:40 MTDiscord  <Sublayer plank> gimp really likes embedding junk into exported files if you specify not to in the export options
09:41 MTDiscord  <Sublayer plank> not if, I mean unless
09:46 sfan5      reminder: #10729 already has one approval and could use a review
09:46 ShadowBot  https://github.com/minetest/minetest/issues/10729 -- Allow Enabling The Touch UI In A Desktop Build by TheBrokenRail
09:59 erlehmann  Sublayer plank i disabled the junk, it is still bigger
09:59 MTDiscord  <Sublayer plank> oh
10:01 erlehmann  sfan5, the first thing i see is that it references a build script that is not in the repository. that was only a cursory glance though. why would a build script not be in the repo? have i missed something?
10:01 erlehmann  this one here https://gist.github.com/TheBrokenRail/d1ef86d115fa8012cff13af350c5fdfd
10:02 sfan5      why would it be in the repo?
10:03 erlehmann  so it can be … well … built?
10:04 erlehmann  it makes little sense to me to NOT have build rules in the repo
10:04 erlehmann  maybe i should read it thoroughly
10:04 erlehmann  but i see no reason to use an external build script
10:04 sfan5      you can argue that it would be useful to have but there is no precedent for the minetest repo containing build scripts for every configuration under the sun
10:05 MTDiscord  <Sublayer plank> isn't that the build script for making it run on a pinephone?
10:07 celeron55  people make new build scripts for different purposes all the time, it's not possible to include all of them
10:07 celeron55  and they get outdated just as often as they get created
10:07 erlehmann  sfan5 in my opinion “having it right there” beats “having to search the issue tracker for random pastebins” anytime
10:08 celeron55  having said that, erlehmann will be free to include all build scripts he can manage to maintain in working order if he's made a core developer
10:09 erlehmann  well if i were asked to review this i'd say “give me a build command, then we talk” – even if its just a long cmake invocation. but this build script actually changes #define in headers using sed(1)
10:09 erlehmann  which is at least a bit weird
10:09 erlehmann  enable_feature() {
10:09 erlehmann  sed -i "s/^#ifdef NO$1.*\$/#ifndef $1\n#define $1\n#endif\n#if 0/g" include/IrrCompileConfig.h
10:09 erlehmann  }
10:09 erlehmann  like seriously
10:09 MTDiscord  <Sublayer plank> the only thing in the build script that makes it different from the normal compilation workflow is enabling opengles and disabling other backends
10:10 celeron55  having said all those, i guess we could maintain a desktop build configuration with touch support, at least for reference purposes
10:10 erlehmann  yeah
10:10 MTDiscord  <Sublayer plank> which makes me assume it's for building minetest for the pinephone, not necessarily with just touch
10:10 erlehmann  well to work that out we would have to have a reference
10:10 erlehmann  i'd definitely take responsibility for build scripts, even if i can not test them
10:10 erlehmann  i have extensive shell script experience and written a build system myself
10:11 erlehmann  in this case, i'd like to build the interface on my laptop, which is definitely not a pinephone
10:11 erlehmann  and on my friends tablet, my friend has windows 10
10:11 erlehmann  with a touchscreen
10:12 sfan5      irrmt doesn't support touch on windows, I once tried but don't have device to test it myself
10:12 erlehmann  ah damn
10:12 erlehmann  i can ask my friend
10:13 sfan5      also editing irrcompileconfig.h is the intended workflow
10:13 erlehmann  can you give me a test .bat file or something?
10:13 erlehmann  oh ok
10:13 erlehmann  i thought cmake would have a facility for ifdefs
10:13 sfan5      just that you could do it easier that that sed stuff
10:13 sfan5      erlehmann: I'd have to 1) find the code whereever I put it and 2) integrate it into irrlichtmt so perhaps next week
10:13 sfan5      s/that that/than that/
10:14 erlehmann  ok
10:14 erlehmann  i can not guarantee my friend will want to do it, he is hit or miss with mt dev work (sometimes overwhelmed), but i will meet him in holidays
10:14 erlehmann  so i'll try to remember it!
10:14 erlehmann  well he does testing mostly
10:15 erlehmann  so no commits from him as far as i know
10:19 erlehmann  meanwhile, i have tried building minetest with this: https://github.com/minetest/irrlicht/pull/52 … i have no idea what exactly does it, but it seems to me like a huge performance loss
10:19 erlehmann  not sure
10:20 erlehmann  wait, i might have done something wrong
10:20 sfan5      it does nothing so far so any claimed effects are placebo
10:20 erlehmann  well i used the minetest-gl-test thing from hecktest
10:20 erlehmann  i bet the difference is in me having a nonstandard minetest.conf
10:21 sfan5      afaik that doesn't do much other than print a few pointers either
10:26 erlehmann  i'll check!
10:26 erlehmann  thank you sfan5
10:39 Wuzzy      I've updated #10810, I have decoupled liquidtype now into liquidtype (for liquid flow) and liquid_move_physics (for movement inside liquids)
10:39 ShadowBot  https://github.com/minetest/minetest/issues/10810 -- Split liquid_viscosity to liquid_viscosity and move_resistance by Wuzzy2
10:39 erlehmann  i am interested in liquids, will look
10:41 erlehmann  btw, in practical terms it seems that building on my old thinkpad under load takes like 5 times the amount than on a contemporary machine (an x390 belonging to my employer) under no load (measured by “time -v make”, cmake invocation is very short), just in case someone wanted to know.
10:41 Wuzzy      It always bothered me for years that liquid flowing and movement in liquid are controlled by the same field (liquidtype)
10:42 erlehmann  i.e. negligible for incremental builds, but a pain for complete rebuilds
10:42 Wuzzy      IMHO every node field should only do one thing at once
10:42 erlehmann  Wuzzy i am with you. liquids are overloaded in a bad way.
10:42 Wuzzy      this gives most flexibility
10:42 Wuzzy      Previously, I successfully decoupled liquidtype from liquid drawtypes. this got accepted =)
10:42 erlehmann  well, technically there is one thing missing for the flow, the speed at which the liquid vanishes. if you look at a falling lava column, it will go extinct.
10:43 Wuzzy      i dont understand ...
10:43 erlehmann  i try to explain better
10:44 erlehmann  currently the way by which liquids spread has different speed sideways. but consider a falling flowing liquid without source.
10:44 erlehmann  if it has high viscosity, it may “fall” slowly, but disappear from the top
10:44 erlehmann  i.e. lava column falling disappears eventually and may not reach the ground, once the source is removed
10:45 erlehmann  water though has no such issue, since it ”falls” at the same speed as it disappears from the top
10:45 erlehmann  i want to warn you though that fixing the flowing behaviour may provide a venue of turning entire worlds into cobble
10:46 erlehmann  Wuzzy was that understandable?
10:46 Wuzzy      I didnt touch flow physics at all
10:46 Wuzzy      all i did was decouple
10:47 Wuzzy      i tried to make sure the behavior of old liquids doesnt change
10:47 erlehmann  good :)
10:47 Wuzzy      and no, it was not understandable ?
10:47 Wuzzy      define "water" and "lava". those words mean nothing
10:48 erlehmann  water flows fast, lava flows slow
10:48 erlehmann  but they disappear at the same rate
10:48 Wuzzy      ah the old viscosity
10:48 Wuzzy      ok viscosity is split into viscosity (spread speed) and move_resistane (player movement)
10:49 erlehmann  yes i see that, but if spread speed behaviour was not changed this is not an issue
10:49 erlehmann  i just want to prevent people messing with the liquid code to accidentally end up enabling this https://2b2t.miraheze.org/wiki/File:1000X1000_Range_of_Spawn.jpg
10:49 Wuzzy      ?
10:50 Wuzzy      we can already have this by launching anarchy servers ?
10:50 Wuzzy      you just need a lot of players ?
10:50 erlehmann  no you can not
10:50 erlehmann  the spawn of clamity anarchy has a big lavacast, but the liquid implementation does not allow for the high walls and triangles
10:50 Wuzzy      okay even a few players can ruin the world, right
10:51 erlehmann  clamity spawn has some obsidian walkways and rails into each cardinal direction. above it is a grid of sponges. above that is a cloud of sponges.
10:51 sfan5      whether players can make funny triangles is of no concern of the engine
10:51 Wuzzy      anyway i dont see how high-viscosity nodes "fall" slower :/
10:51 erlehmann  Wuzzy they do not. which is what prevents the shenanigans i just showed you.
10:51 Wuzzy      i literally see no diffeence. they "fall" 1 node per second
10:52 sfan5      the behaviour just needs to stay the way it was
10:52 Wuzzy      it does.
10:52 erlehmann  good
10:52 Wuzzy      so, viscosity is split into viscosity and move_resistance
10:53 Wuzzy      but the trick is that move_resistance will default to the value of viscosity if unset
10:53 Wuzzy      this ensures backwards-compability
10:53 Wuzzy      yes i am very aware that changing stuff like liquid flow is very dangerous (which is why i didnt do it)
10:53 erlehmann  the origin of the clamity sponge sky might be relevant to the engine though: we all know that minetest particle behaviour is abysmal – and mineclone2 loves to spam particles manually (probably because long lived particle spammers constitute a coordinate leak, which means some clown will come to your base and fill it with TNT, then drop a match)
10:54 Wuzzy      you probably still want to test to make sure i didnt overlook something
10:54 erlehmann  so people were filling spawn with water, which created many drip particles
10:54 erlehmann  now minetest AFAIK does not allow you to turn off particles (am i wrong here?), so players were lagged at spawn.
10:54 erlehmann  people reacted differently to it.
10:54 erlehmann  fleckenstein made a CSM i call „spongebot“ that fills areas with sponges to soak up the water.
10:54 erlehmann  cora modified her client to optionally turn of particles.
10:55 erlehmann  and this is how clamity got the sponge sky, because of performance reasons tied to particles ^^
10:55 Wuzzy      hmmm there prob needs to be a minetest setting to reduce or limit particles ...
10:55 Wuzzy      like a hard cap or somting, idk
10:55 erlehmann  look no further than the waspsaliva cheat menu i guess
10:56 sfan5      Wuzzy: game makers will be delighted to know that smoke effects made using particles can be trivially cheated away
10:56 erlehmann  dragonfire (flecks client) is the griefing client, waspsaliva (coras client) is the builder client (as in “i want to spawn a structure from a template and have all the building materials, it should be 1 lua command away”)
10:56 Wuzzy      but it will be bad for games that have important particles (those that convey iformation)
10:57 Wuzzy      maybe an attribute like "important particle" can be added to tell minetet this particle should not be removed. i.e. particles that display some HUD info or whatever
10:57 Wuzzy      besides, game makers should themselves make sure that the game cannot be spammed in particles anyway
10:57 erlehmann  oh btw, i think it might be that it was not necessarily particles, i have to double check the story. maybe it was also drip entities? no idea. i the end, vanilla minetest lagged and waspsaliva did not.
10:58 erlehmann  to be fair, the way mineclone2 spams particles is over the top
10:58 MTDiscord  <luatic> Yes, particle rendering is very inefficient currently.
10:58 erlehmann  i mean, people disable rain on their servers because of vanilla clients lagging
10:58 Wuzzy      you probably meant water droplets, right?
10:58 erlehmann  Wuzzy, i guess! sorry for telling the wrong story then.
10:58 Wuzzy      those are actually entities. :/
10:58 erlehmann  i mixed up two things then
10:59 Wuzzy      obviously entity performance is even worse
10:59 erlehmann  so the sponge sky is because of lots of cute little drop entities prob and the disabling rain thing is because of particles.
10:59 erlehmann  i think the new particle spammers will probably help
10:59 sfan5      can I remind you that this is the development channel
10:59 erlehmann  oh sorry :/
10:59 erlehmann  i will look at the liquid
11:00 erlehmann  PR
11:00 Wuzzy      thanks
11:01 erlehmann  btw, rain is kinda important particle gameplay-wise, so i don't see a real solution there.
11:02 erlehmann  i will wait for the better builtin particle spammers. but note that no game will ever want to set the lifetime of a particle spammer to >1s because then it gets sent to all clients, which means having a coord leak.
11:02 Wuzzy      For the P: i recommend you set movement_liquid_fluidity to a different value to get a difference in the move-resistant nodes in DevTest
11:02 Wuzzy      For the PR*
11:02 erlehmann  ok!
11:02 Wuzzy      but also test with default value ofc ?
11:03 Wuzzy      wait, all particles are sent to all players no matter how far away? that explains a lot  ?
11:03 erlehmann  Wuzzy only if lifetime is >1s
11:03 erlehmann  so don't do that
11:04 erlehmann  the balance is easy: if you are far away and it is long-lived, say 30s, you may come closer.
11:04 erlehmann  then you could not see the particles if your client did not get them.
11:04 erlehmann  but if it is short-lived, you are putting a beacon on your location.
11:04 Wuzzy      grrrrr of course thats undocumented >_>
11:04 erlehmann  unless it is fixed, which it is, due to sfan5
11:04 erlehmann  he made the 1s limit in the code i think
11:05 erlehmann  i think the beacon mod for example has 3s particle lifetime … let's say those beacons shine so damn bright ppl are going to show up eventually
11:07 erlehmann  Wuzzy sfan5 i believe it is undocumented on purpose to not give cheaters any ideas?
11:07 sfan5      Wuzzy: it's undocumented because it's an implementation detail and an estimate
11:07 erlehmann  ah
11:07 erlehmann  IMO it should be documented because games rely on it. in mcl* games all particle lifetimes have been cut off at that limit.
11:07 erlehmann  because ppl make particles when eating or running.
11:07 erlehmann  basically clamity had an incident where all bases except 1 were griefed because of that
11:08 Wuzzy      this needs improvement. sending particles to everyone even if 10000 away feels very wrong. some "max player distance" might make sense
11:08 erlehmann  yes
11:08 erlehmann  coord exploits can make a single evil player destroy a server entirely
11:09 sfan5      Wuzzy: it is not easily improveable while remaining correct, otherwise it would have been done
11:10 Wuzzy      what if the modder decides on distance? e.g. particle is only sent to players within $RADIUS. would be very useful for long-lived but just decorative particles like sprinting or eating
11:10 Wuzzy      or in that case not long-lived, actually "medium-lived" ?
11:10 sfan5      we have max_send_distance with sounds and people also consider that broken
11:11 Wuzzy      yes but for different reasons
11:11 sfan5      no, for the same reasons
11:11 Wuzzy      one big reason it because the fade-out distane is constant, something that doesnt apply to particles
11:12 Wuzzy      by default the fade-out distance is corret (32 nodes i guess?) but if you change the max_hear_distance the fade-out distance is still 32
11:12 Wuzzy      e.g. with max_hear_distance of 16 the sound is "half" as loud at about 15.9 nodes away and then suddenly silent at 16.1 nodes away ?
11:13 Wuzzy      What are the other reasons?
11:13 erlehmann  Wuzzy, a player that is 10k nodes far away can still step into a travelnet box or be pushed by a redstone pistone teleporter line (those are really cool btw, players defeating the physics)
11:13 erlehmann  or just fall downwards 10k nodes in an instant
11:14 Wuzzy      what does that have to do with particles? *confusedface*
11:14 erlehmann  or respawn … there are lots of reasons
11:14 erlehmann  well the player would not be in range first and later they would be
11:14 erlehmann  the code would have to handle them *arriving*
11:15 erlehmann  which i guess would be the sensible solution, but it is not easy
11:15 MTDiscord  <luatic> We could just give control over this to modders I guess.
11:15 Wuzzy      i dont see the problem, if ur away, ur away
11:16 erlehmann  say you make a long-lived particle fountain next to a nether portal. what would be the correct radius? i believe that currently if you limit the radius, the particle fountain would only be sent once over the network, at creation. sfan5 am i wrong?
11:17 Wuzzy      oh wait... youre talking about particle SPAWNERS!!!
11:17 Wuzzy      thats a huge difference!
11:17 erlehmann  i think both constitute risks
11:17 erlehmann  the same risk, actually?
11:17 erlehmann  unintended coord leaks
11:18 Wuzzy      you mean like particles that live forever? that could be a problem :/
11:19 erlehmann  mineclone2 in fact does contain particles that live forever, flame particles. but we could not prove a coord leak there, only a performance problem, because they will be respawned sometimes.
11:19 erlehmann  also flame particles are turned off by default
11:19 erlehmann  i think it was basically, if you go away and then return, you get a new particle?
11:19 Wuzzy      ok thats a challenge then. the server would probably have to watch all players and even re-send particles once a player is in range. hmmmmmmmmmmmmmmmmm
11:19 erlehmann  i think modders should have more control, but it should be made easy to do the right thing
11:20 erlehmann  yeah, it would be better if the engine handled it
11:20 Wuzzy      yes this problem clearly cant be solved with mods alone
11:20 Wuzzy      oh and depending on undocumented features is rarely a good idea ?
11:20 Wuzzy      anyway, did you manage to compile my PR so far?
11:21 erlehmann  https://git.minetest.land/MineClone2/MineClone2/issues/1855 “I'm playing on Windows 10, rtx 2070s + ryzen 3600, and my fps eventually drops down to zero with particles enabled.”
11:21 Wuzzy      offtopic
11:21 erlehmann  not yet
11:21 erlehmann  ok
11:21 erlehmann  back to fluids, wait for compile
11:22 Wuzzy      thx ?
11:24 sfan5      Wuzzy: particles are considered short lives enough that max_block_send_distance is applies to them, the discussion is indeed about particle spawners
11:24 sfan5      short lived*
11:25 Wuzzy      interesting
11:25 erlehmann  yes i am sorry, but note that particle spawners also need to be short lived because of the coordinate leak
11:26 erlehmann  every mod that has a >1s particle spawner is a huge security risk
11:26 sfan5      holy shit will you stop talking about MCL issues in here
11:26 erlehmann  ok
11:26 sfan5      you have been annoying me the entire morning with this bullshit
11:26 erlehmann  ok sorry i take it to #minetest
11:27 sfan5      please do
11:27 erlehmann  guess i limit myself to a) really important engine issues that need solving right now b) pr discussion
11:29 MTDiscord  <luatic> Wait you DON'T send particles to clients of they're not nearby when the particle is spawned? That's fallacious.
11:29 MTDiscord  <luatic> if*
11:30 sfan5      it's not entirely correct either, true
11:30 sfan5      so far nobody has every complained about it
11:32 sfan5      (I should really spellcheck my messages...)
11:35 sfan5      consider that usually the time required to traverse one max_block_send_distance is much larger than the time a single particle should live
14:19 Wuzzy      We've just reached 1,000 open issues on GitHub
14:21 sfan5      we did that a few days ago but then I closed some again
14:24 erlehmann  Wuzzy, i found a deadly incompatibility in your liquid PR: as expected, a client from before your patch does not observe the speed limit in the nodes. this is only a big problem in case of fall damage. people are going to use “Move-resistant node, liquidlike” as lifts probably (in fact, i have seen both mods and players utilizing liquids as lifts).
14:25 erlehmann  Wuzzy, TL;DR: minetest 5.4 clients will fall to their death
14:25 Wuzzy      so?
14:26 erlehmann  this is unfortunate
14:26 Wuzzy      We don't really have full compability between minor releases
14:26 sfan5      could send move_resistance in the viscosity field for old clients, couldn't you?
14:26 sfan5      the client does not care about actual flow parameters of liquids
14:26 erlehmann  well, since the *obvious* thing is to build a lift with it, it would be a bit weird if it was a deathtrap for older clients that are otherwise fully compatible
14:27 erlehmann  i like the sfan5 suggestion
14:27 Wuzzy      hmmmm it sounds a bit hacky but it might work
14:27 erlehmann  sfan5 would that also solve it for non-liquidslike speed limit? would be sweeeet
14:27 sfan5      idk
14:33 Wuzzy      dumb question, but HOW much compability is required betweeen two minor versions?
14:35 erlehmann  i don't care, i just think if i as a gamedev would use this feature and my players died i would feel pretty dumb about using it
14:35 erlehmann  and probably stop using it
14:35 sfan5      that's a big question but I'll answer the part you actually want to know about: if new features can be easily and reasonably made to work with older versions they should, if not then not
14:35 erlehmann  if it works in earlier clients with the sfan5 hack though it is pretty much an instant “let's do this instead of the hacks we have now”
14:36 erlehmann  sending some field depending on client version does sound easy tbh
14:36 erlehmann  it is prob already done in other places?
14:39 sfan5      (yes)
14:40 erlehmann  Wuzzy, i have not yet tested with different “movement_liquid_fluidity”, but so far, apart from the “if someone uses this as intended, players fall to their death randomly” issue, the behaviour looks sound. i will look at the code at a later time, probably during the weekend, preferably when some fix for the player killing bug is in it.
14:41 erlehmann  and i will of course test different “movement_liquid_fluidity” values
14:41 erlehmann  well, i can look at the code a bit now
14:43 erlehmann  sfan5, how do i compile minetest with ubsan?
14:45 sfan5      you set compiler flags via -DCMAKE_CXX_FLAGS="..."
14:46 erlehmann  ok i try thx
14:59 MTDiscord  <Warr1024> As a gamedev I only support MT versions back a certain distance, and this goes for both client and server.  If you add a feature in 5.5 that breaks for 5.4 clients, then I may not be able to use it immediately ... but once I end-of-support 5.4 I can start using it then.
14:59 MTDiscord  <Warr1024> This does mean I actually have a backlog of new features I can't start leaning on yet :-)
15:00 erlehmann  i guess that matches my “if its backwards compatible, i'd use it immediately”
15:03 MTDiscord  <Warr1024> Yeah, same here.  I guess I'm just saying it's a bit more nuanced than just "use or not use"; there's value in adding things that aren't immediately useful, since sometimes there's just no way to get the value out of something other than biting the bullet and adding it in, and then having to wait until it's safe to actually use.  add_player_velocity is maybe another example of this.
15:04 erlehmann  the thing with this is that i'd probably have to add workarounds myself if the engine does not do it
15:05 MTDiscord  <Warr1024> I'll add workarounds or try to make the feature degrade gracefully for old-but-supported clients, but sometimes there isn't a good way to do that and I actually just have to hold off on designing a dependent feature entirely.
15:06 erlehmann  yeah
15:06 erlehmann  engine devs have huge responsibility
15:07 erlehmann  well, i'll wait for a hopfully working backwards compatible version of the PR! it seems a very nice thing :)
15:07 erlehmann  and overloading attributes seems bad
15:07 erlehmann  so separating them is good, naturally
15:10 erlehmann  sfan5 is this a known bug? https://mister-muffin.de/p/EUy3.txt
15:10 erlehmann  this is ubsan output
15:10 erlehmann  i think?
15:41 sfan5      I can't tell if you if it's a bug but it's not known
15:42 rubenwardy looks like a bug
15:52 erlehmann  i get a ton of them with -fsanitize=undefined
15:52 erlehmann  guess minetest is full of holes
15:52 erlehmann  even found some overflows
16:51 sfan5      while looking at something unrelated I found that just about every database backend needlessly makes a temporary copy of every map block loaded
16:51 sfan5      std::string::assign is right there, how hard could it be...
16:53 MTDiscord  <Jonathon> Curious, which backend(s) does not?
16:54 erlehmann  procedural question: in the right now entirely hypothetical case that i start to insist on boundary checking at some points in the code, would i have to show an actual crash or exploit?
16:54 sfan5      @Jonathon all of them
16:55 pgimeno    if they are active in debug builds only, maybe not?
16:56 erlehmann  sfan5, is it the fault of each and every database backend or maybe some abstraction over them?
16:56 sfan5      @Jonathon nvm since you asked "not", none of them
16:56 erlehmann  like are they all stupid in the same way
16:56 sfan5      erlehmann: every database backend made this mistake on their own
16:56 erlehmann  lol
16:57 sfan5      of course the reason probably is that the basic code was all just copy pasted from eachother
16:57 MTDiscord  <Jonathon> ah, asking since you said just about
16:57 rubenwardy erlehmann: bounds checking?
16:57 rubenwardy generally, everything should be bounds checked. You should need to have a good case not to do so
16:57 rubenwardy std::array and std::vector help with this
16:57 sfan5      that's a bad statement to make
16:58 rubenwardy performance at the cost of safety and correctness isn't good
16:59 sfan5      point is there are a million places where the value can never be out of bounds but you could do a check "just in case"
17:00 erlehmann  imagine if there was a signed integer overflow in getBlockSeed2, for example, mayb in line 241
17:00 erlehmann  of mapgen.cpp
17:00 sfan5      then we can't fix it because it affects world consistency
17:01 erlehmann  uh
17:02 erlehmann  the problem here is that i have seen things being generated that might be due to this bug
17:02 erlehmann  let's say it's not pretty
17:02 erlehmann  (can't give you the seed & coordinates, ppl would grief a base)
17:03 erlehmann  sfan5, what exactly is consistent about a signed integer overflow anyway?
17:03 erlehmann  it's undefined behaviour
17:04 sfan5      it being undefined behaviour in the C standard does not say anything about what the generated assembly actually does
17:04 erlehmann  it's about as useful as division by zero or faulty pointer arithmetic in terms of what you can expect from the compiler
17:04 erlehmann  oh no, are you one of those ppl who think undefined behaviour equals implementation-defined behaviour?
17:04 erlehmann  that's a common misconception
17:04 erlehmann  but easy to rectify
17:05 sfan5      it clearly does not because UB gives the compiler the right to mess up your entire function
17:05 erlehmann  yeah but that is not addressing the question “what is UB” in terms of what assembly is output
17:05 erlehmann  also it can mess up your entire program
17:06 pgimeno    <sfan5> it being undefined behaviour in the C standard does not say anything about what the generated assembly actually does  <--  that's a poor argument; such assembler can change between versions of a compiler, and depend on things like uninitialized memory, which can be random
17:06 erlehmann  i have seen UB that results in functions being called that are *never* called in the normal flow of the program
17:06 erlehmann  yes, undefined behaviour often means the optimizer might assume “this code path can never be executed without having UB” and then turns it into a NOP
17:06 erlehmann  leaving only the defined behaviour
17:06 sfan5      pgimeno: I know, it can but it must not
17:07 pgimeno    if some worlds depend on a certain behaviour, then by all means implement it explicitly, rather than depending on what the compiler does in that case
17:07 erlehmann  what i am saying is the undefined cases are already affecting world consistent
17:07 erlehmann  what pgimeno says
17:07 sfan5      okay, write a bug report then
17:08 erlehmann  well what is the bug? i can't even reproduce it easily
17:08 erlehmann  i'd have to iterate through map seeds having ubsan activated (i.e. an extremely laggy game) probably until i hit one that hits it?
17:09 erlehmann  or hmm, i could unit test the function
17:09 sfan5      I thought ubsan only told you about UB that actually happens at runtime
17:11 erlehmann  that's the thing, to encounter an actual bug i would have to figure out how to trigger it at runtime
17:11 erlehmann  but here, second line i that function https://mister-muffin.de/p/xcod.txt
17:12 erlehmann  there is only a limited amount of v3s16 vectors and s32 integers, so you can test them all i guess?
17:12 erlehmann  i'll try something brb
17:38 erlehmann  i hope this counts as evidence: https://mister-muffin.de/p/b1eT.c
17:39 erlehmann  no idea if i got the typedefs right
17:39 erlehmann  i am just poking holes here
17:40 sfan5      you didn't
17:40 erlehmann  damn
17:40 erlehmann  so what are they?
17:40 celeron55  use stdint.h
17:41 erlehmann  ok
17:41 celeron55  there you get int16_t, int32_t and uint32_t
17:41 celeron55  i don't really even remember the last time i wrote C without those
17:42 celeron55  the standard types are barely useful
17:44 erlehmann  how about this https://mister-muffin.de/p/w7k2.c
17:45 sfan5      block coordinates only go up to 4096
17:47 erlehmann  how about this https://mister-muffin.de/p/5S4T.c
17:48 erlehmann  sfan5, did i get it right this time?
17:49 sfan5      yes
17:49 erlehmann  good, is this enough evidence for “this needs a fix” in your opinion?
17:50 sfan5      somewhat low priority but sure
17:50 erlehmann  good
17:51 erlehmann  why woudl UB be low priority though?
17:52 erlehmann  i mean except for the seed these inputs flowing into the function are fed by remote users
17:52 erlehmann  load a block, bam
17:57 erlehmann  regarding priority, do low priority bugs magically turn into high priority bugs if someone who is not me (i swear) develops a way to crash the engine or expioit it? that is basically the core of the “do i have to deliver an exploit so it gets fixed” question.
17:57 erlehmann  i wat to caution you that i am probably not the first person to find whatever i find
17:57 erlehmann  wat → want
17:58 erlehmann  (if you say yes i might try to find a way to crash minetest using a fuzzer)
18:00 sfan5      if you think that's a good use of your time I won't stop you
18:00 erlehmann  it's probably not, so i hope i don't have to
18:01 erlehmann  but i am talking with my reliability & security hat, you have another hat
18:03 erlehmann  i think the best way to eliminate these kind of bugs is have the CI compile minetest with different sanitizers (ubsan, asan, thread sanitizer) and run it for a few minutes.
18:04 erlehmann  with predictable input (can --random-input or how its called be seeded?)
18:10 erlehmann  undefined behaviour does not mean “the compiler will emit code to format your disk”, but rather “some clown will make your program behave weirdly and use an exploit to format your disk”
18:11 erlehmann  for the record, in noise.cpp there is a noise3d function which has pretty much the same issue
18:11 erlehmann  i.e. x / y / z / seed go in, stuff is multiplied, result is probably not going to fit
18:13 erlehmann  oh my this is funny
18:14 erlehmann  commit d198e420ec54be47fe2285b3205953282ec06742 claims that this kind of UB already led clang >= 3.7 to optimize the thing away
18:14 erlehmann  >    Solved by changing n to an unsigned type, making behavior well-defined.
18:15 erlehmann  points for effort i guess?
18:39 Krock      sfan5: hmm that's weird. in master the error message appears as expected, but with 11594, Minetest aborts
18:39 sfan5      maybe make clean && make will fix it
18:39 sfan5      though I wouldn't know why that is
18:40 sfan5      in theory LuaError could refer to a different type in both places but I don't see how that could happen
18:42 Krock      hmm
18:43 Krock      rebuilding with master + PR
18:45 Krock      __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
18:46 Krock      still the same issue after "make clean"
18:47 sfan5      compiler?
18:47 sfan5      gcc 11.1 here
18:47 Krock      g++ (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0
18:48 Krock      register_on_mods_loaded for example does work
18:48 Krock      i.e. when adding an error there
18:49 sfan5      i'll push something that tests it on CI
18:51 Krock      hmm. register_playerevent is apparently not documented. I wonder why...
18:52 sfan5      % grep playerevent games/minetest_game -r | wc -l
18:52 sfan5      0
18:52 sfan5      :thinking.
18:52 sfan5      ?
18:53 Krock      it's a race condition
18:53 Krock      delaying this erro() line by 5 seconds in globalstep works just fine
18:53 Krock      as in - no abort.
18:54 sfan5      ah
18:54 sfan5      I did test with core.after
18:54 Krock      PS: playerevent is only used in builtin / statbars
18:57 sfan5      fixed.
19:02 Krock      thank you
19:42 pgimeno    for me, this produces the same code with -O3 as the original: http://www.formauri.es/personal/pgimeno/pastes/blockseed.c
19:43 pgimeno    I think it should fix the UB
19:44 pgimeno    well, it doesn't fix the negative to unsigned type conversion
19:48 erlehmann  pgimeno, just because it is not optimization-unstable, does not mean it is ok
19:49 erlehmann  pgimeno, but the other way is true, if it is optimization-unstable, it is prob not ok
19:51 pgimeno    wait, that should have been 1013LL
19:52 pgimeno    with that, it's not the same code but the number of instructions doesn't change
19:53 pgimeno    erlehmann: I don't understand what you mean. That change is meant to remove the UB of signed integer overflow.
19:55 erlehmann  pgimeno maybe i misunderstood what “with -O3 it produces the same code” means for you?
19:57 pgimeno    erlehmann: it means that the code formalizes the behaviour that most compilers will probably take when compiling the original code. Anyway I made a mistake in the implementation.
19:58 pgimeno    It also means that there's no performance penalty in doing that.
19:58 erlehmann  i see
20:12 erlehmann  pgimeno what does ubsan say about it?
20:15 erlehmann  pgimeno it is not obvious though
20:15 pgimeno    what's not obvious?
20:15 erlehmann  how the types are promoted
20:15 pgimeno    depends on how used you are to it, I guess
20:16 pgimeno    it's also not obvious that there was UB there
20:18 erlehmann  that adding 3 s16 + 1 s32, with all of them multiplied by some numbers may exceed an u32 is not obvious, but kinda to be expected
20:19 pgimeno    the s16 are multiplied by ints, which are 32 bits in all platforms supported, so it's not adding 3 s16, it's adding 4 ints
20:19 pgimeno    at least* 32 bits
20:19 erlehmann  especially because that error was caught earlier
20:19 erlehmann  with a smaller data type
20:19 erlehmann  but tbh i have no idea about cpp
20:19 erlehmann  i am just the person pointing out the holes, not the fixer
20:19 Fixer      .
20:20 erlehmann  lol
20:21 pgimeno    guarding against UB in that function while preserving what most compilers do is going to be tricky
20:21 pgimeno    (and keeping the speed)
20:22 erlehmann  it's not like multiplications and additions can be really slow or can they?
20:23 pgimeno    no but conversion to long long needs to be done in a way that the compiler can detect and optimize
20:27 celeron55  that function is called relatively rarely within relatively massive tasks so at the moment it could be written in BASIC for all that matters, but we all know how that ends up
20:27 celeron55  some day it will be called for a new task where it then has to be the fastest possible
20:28 pgimeno    oh I thought it was speed critical
20:29 erlehmann  it is called when blocks are generated i think
20:29 erlehmann  but only once or so
20:29 erlehmann  for each block
20:29 erlehmann  the UB will i practice happen every few minutes or so
20:30 erlehmann  i was able to get it by making a map with the seed “ubsan” for mapgen v7 and walking straight north for around 100 nodes from spawn
20:30 erlehmann  ok i also dug everything in front of me to stay level
20:31 erlehmann  pgimeno, if you fix this pls fix the almost identical noise function too, using the same fix.
20:33 pgimeno    I think this version gets rid of all UB: http://www.formauri.es/personal/pgimeno/pastes/blockseed2.c
20:33 pgimeno    erlehmann: I'm not a core dev
20:33 pgimeno    and I'm not on GitHub, which means I can't submit PRs
20:35 pgimeno    the generated code is practically identical
20:35 erlehmann  you can build it into minetest, commit it on a branch, then either make a pull request using git request-pull and post it here or – way easier – use git format-patch and post it here
20:36 erlehmann  pgimeno, but then again, there probably should be a unit test and so on …. thank you for your efforts though!
20:38 erlehmann  pgimeno, could you explain WRAP_SIGNED though?
20:38 pgimeno    I have submitted patches, but unless someone adopts it as a PR, it's hopeless, and so far only one of my 8 or 10 PRs has been adopted
20:38 erlehmann  oh damn
20:40 pgimeno    erlehmann: when passed a negative number, adding 0x100000000LL to it gives the unsigned equivalent of the usual signed representation
20:41 pgimeno    the positive number fits in signed 64 bits
20:41 sfan5      surely we can assume two's-complement and just cast it directly?
20:42 pgimeno    isn't that UB though? I aimed for removing those
20:43 pgimeno    but there's probably hundreds of uses of type casts like that in the code anyway
20:43 sfan5      I hope casting numbers is not USB
20:43 sfan5      UB*
20:43 sfan5      yeah
20:47 pgimeno    you're right: "Otherwise, if the new type is unsigned, the value is converted by repeatedly adding or subtracting one more than the maximum value that can be represented in the new type until the value is in the range of the new type."
20:48 sfan5      neat
20:49 pgimeno    we can always precede it with: assert(!~INT_MAX); i.e. assert that INT_MAX is all 1's
20:50 erlehmann  if the code is littered with hundreds of instances of questionable behaviours, they should ofc be patched and not preserved?
20:50 erlehmann  pgimeno, thank you for the explanation, i like it!
20:52 specing    erlehmann: rewrite mt in Ada <3
20:53 specing    no, not Ada. SPARKAda!
20:54 erlehmann  so far i have not written anything but just pointed out bugs
20:54 erlehmann  i mean, recently
20:54 erlehmann  i'll be happy to stay in that role if it means that more capable people do the bugfixing work
20:54 erlehmann  obv i am not familiar with anything
21:00 pgimeno    oops, ~INT_MAX for signed changes the sign bit too, that's not a good check either
21:03 erlehmann  what is wrong with the code above?
21:03 Wuzzy      sfan5: unfortunately, I don't think there is a clean way for old clients to emulate move_resistance behavior
21:03 erlehmann  you only need to make it as clever as you can, not more clever
21:04 erlehmann  Wuzzy what happened when you tested the fluid thing?
21:04 sfan5      merging #10324, #11594, #11597 in 10m
21:04 Wuzzy      what i tried is to pretend the node is a liquid_source and sent move_resistance as liquidtype to the 5.4.0 client
21:04 ShadowBot  https://github.com/minetest/minetest/issues/10324 -- Split vector.new into 3 constructors by Desour
21:04 ShadowBot  https://github.com/minetest/minetest/issues/11594 -- Clean up/improve some scriptapi error handling code by sfan5
21:04 ShadowBot  https://github.com/minetest/minetest/issues/11597 -- Send to clients node metadata that changed to become empty by TurkeyMcMac
21:05 Wuzzy      that is already hacky but still not enough
21:05 Wuzzy      5.4.0 is simply imcapable of fully emulating the move_resistance behavior, best i can do is to pretend its a liquid but it will allow "swimming" ...
21:06 erlehmann  Wuzzy for the liquid thing that is enough?
21:07 Wuzzy      not for a pure move_resistance node
21:07 sfan5      Wuzzy: okay, then it won't be done
21:07 erlehmann  the liquidlike move resistance thing
21:07 erlehmann  that's the only one ppl would use as a lift
21:07 erlehmann  but i *only* complained about the one for which it *can* be done
21:08 erlehmann  Wuzzy are there any negative effects for move_resistance on liquidlike nodes?
21:08 erlehmann  if the sfan5 hack is applied?
21:08 erlehmann  (i mean you can already swim in them)
21:09 Wuzzy      hmmm i think those should work
21:09 Wuzzy      but i dont like the hack
21:09 erlehmann  well i didn't like dying while testing it
21:09 Wuzzy      either do it right or dont do it at all, no half-solution please
21:09 erlehmann  player fun is worth more than theoretical purity
21:10 erlehmann  but no other nodes make sense as a slow moving lift OR they already work
21:10 erlehmann  the climbable thing works
21:10 Wuzzy      radical solution is to mark 5.5.0 as incompatible with 5.4.0 clients
21:10 pgimeno    sfan5: anyway the last term can't be done with just a cast, you need a dance similar to my WRAP_SIGNED so that the last product doesn't overflow
21:10 erlehmann  that's possible, but not necessary
21:11 Wuzzy      to be clear: if your server doesn't use move_resistance, you're fine
21:11 erlehmann  working around user issues will always be ugly. users falling to their death is an issue.
21:12 erlehmann  making it incompatible bc the hack is not using cases where they do not fall to their deaths … is at least a bit strange.
21:12 Wuzzy      why are you only concerned with this one use case of "falling to their death"?
21:12 Wuzzy      theres a billion ways to use move_resistance
21:13 sfan5      pgimeno: but isn't the overflow of the last product intended (and defined behaviour)?
21:13 Wuzzy      also, i REALLY want to avoid overbloating fallback code in the serializer with special cases for old prot versions ?
21:13 erlehmann  because i only ever saw in mods and games the use of liquidlike (i.e. swimming is possible) and climbable lift shafts and the climbable ones work already.
21:13 Wuzzy      so the most obvious usecase for move_resistance is cobwebs
21:14 pgimeno    sfan5: wait, I mean the product 1013 * seed
21:14 pgimeno    the last line is fine
21:14 Wuzzy      5.5.0 clients will be slowed down as expected but 5.4.0 will currently move through those nodes like air
21:14 erlehmann  Wuzzy, ppl do indeed use cobwebs to stop falling
21:14 Wuzzy      so it's not only "falling to their death" but also an advantage (cobwebs no longer slow you)
21:14 sfan5      pgimeno: 1013 * (u32)seed?
21:15 Wuzzy      no matter what you do, you will always end up with weird gameplay inconsistencies depending on version you connect with. that is very very bad
21:15 pgimeno    sfan5: well, if it's not a big deal to not return the same block seed between versions then yes
21:15 Wuzzy      only good solution would be declaring incompability ?
21:15 erlehmann  Wuzzy would cobwebs be liquidlike?
21:15 pgimeno    sfan5: in that case it's a one letter fix: 1013U * seed
21:16 pgimeno    (and let automatic integer promotions do the rest)
21:16 sfan5      well it should return the same values as before
21:16 erlehmann  Wuzzy there are a bunch of examples where old clients get a degraded experience but i have never seen deadly results.
21:16 pgimeno    sfan5: then no, unlike sum, unsigned product does not return the same value as signed product
21:17 Wuzzy      well the closest way to emulate move_resistance is by lying to clients. send liquidtype="source" and liquid_viscosity=move_resistance
21:17 Wuzzy      then the difference is that old clients can "swim" in node (allowing up movement) while new clients can anly walk/move horizontally
21:17 erlehmann  Wuzzy, i see it not as “does this slow you down”, but “does the liquidlike block act like a liquid” and falling into a liquid and then not dying bc it slows the fall is an important gameplay mechanic.
21:17 Wuzzy      oddly, this gives old clients and advantage on one hand
21:18 erlehmann  as i said, i am only caring about the liquidlike block, bc that's the only one that ppl are likely to use as intended and then get bad results for others.
21:18 Wuzzy      i feel like the difference in behavior is large enough to justify incompability. .
21:19 Wuzzy      if you want to be REALLY strict about it, postpone it to 6.0.0 ?
21:19 Wuzzy      alternatively, declare incompability w/ 5.4.0
21:19 Wuzzy      btw, what's the oldest version that can connect to 5.4.0?
21:19 pgimeno    sfan5: hmm, maybe it does for negative * positive, that needs to be checked
21:19 erlehmann  no idea, but 5.3 is widely used because raspberry pi
21:19 sfan5      Wuzzy: 5.0
21:20 Wuzzy      oh no...
21:20 erlehmann  making stuff incompatibile in a minor release is hopefully not going to happen
21:20 Wuzzy      so we have to wait all the way to 6.0.0 before we can have this. so sad
21:21 Wuzzy      hmmmm, is there a way for servers to restrict versions that are allowed to connect?
21:21 erlehmann  yes, if you want perfect compatibility. as i said before, as a gamedev, i'd accept “does not slow down everyone”, but not “players die if their client is too old”.
21:22 Wuzzy      i understand
21:22 Wuzzy      but it feels really hacky and ugly
21:22 erlehmann  any logic “if a block with this property but not that one is defined, old clients are not allowed to connect” will feel uglier and hackier
21:22 Wuzzy      if u use this node for parcours, you'd basically have to ban all 5.4.0 and previous players anyway (if you can)
21:22 Wuzzy      because for parcours, consistency is key
21:23 erlehmann  true, but tbh, if you are cheating in parkour, you are cheating yourself
21:23 erlehmann  it is like flying on inside the box
21:23 erlehmann  you don't win anything with it
21:23 erlehmann  except loss of fun
21:24 Wuzzy      its wore for PvP because a 5.4.0 cobweb is like air, i.e. you can chase a player thrhough cobweb much easier
21:24 Wuzzy      worse*
21:24 erlehmann  PvP is dominated by killaura, forcefield and some nastier things that i don't want to go into
21:24 erlehmann  though i recently trolled a player killer by just connecting with --random-input
21:25 Wuzzy      yeah its not like its very vulnerable to cheats...
21:25 erlehmann  he got bored
21:25 Wuzzy      i can almost live with the liquid_viscosity = move_resistance hack
21:25 erlehmann  Wuzzy, i think if you are concerned about parkour and stuff, maybe ask sofar and other parkour server devs
21:26 Wuzzy      but what I think is very problematic is faking liquidtype to be "source"
21:26 erlehmann  why that
21:26 Wuzzy      idk, it seems to be a very important value, it should not be faked
21:26 erlehmann  it's not like someone will put the block in a bucket?
21:26 erlehmann  when browsers connect somewhere, they claim to be Mozilla/5.0
21:27 Wuzzy      i'm afraid faking liquidtype leads to many bad side-effects
21:27 sfan5      Wuzzy: I don't think clients use those for anything other than rendering
21:27 sfan5      they don't compute liquids
21:27 erlehmann  “Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/74.0.3729.169 Safari/537.36”
21:27 sfan5      now since the liquidtype vs paramtype change was also recent idk what <5.3 would do
21:27 erlehmann  Wuzzy i will try to break that solution, as i have tried to break the previous code
21:27 Wuzzy      how for rendering?
21:27 erlehmann  my approach to testing is “try to break it and hope i fail”
21:28 sfan5      dunno, that's a guess
21:28 Wuzzy      i recently decoupled liquidtype from drawtypes, so that should no longer be the case
21:28 erlehmann  try it and we'll figure it out?
21:28 Wuzzy      it could affect liquids_pointable ...
21:33 Wuzzy      oh. also, the no-swim liquid will be swimmable in 5.4.0 ...
21:34 Wuzzy      which means adding more ugly special checks and hacks for hacky "backwards-compability" ...
21:34 Wuzzy      yeah, i think its best to not even bother
21:45 pgimeno    sfan5: from what it looks like, you're right that in this case, the signed multiplication can be made unsigned without changing the result, so adding a 'U' to the last constant should suffice as a fix
21:46 Wuzzy      to be clear: functionally, the liquid PR is complete if you only care about 5.5.0. the only thing we've been talking about is how MT should behave WRT old clients
21:47 erlehmann  the most difficult about everything is backwards compatibility
21:48 erlehmann  it's easy to make something that works compared to something that works reliably 5 or 10 years later still, across a range of versions.
21:49 sfan5      note that how new features work on old clients is not part of our compatibility guarantee
21:50 Wuzzy      good point
21:50 Wuzzy      but this particular feature has a risk of high frustation of players in old clients connecting to new server
21:51 Wuzzy      imagine a cobweb trap. new clients will correctly sink through gravefully. but old clients will fall right through, potentially dealing deadly fall damage
21:51 Wuzzy      this is not an issue as long the server doesn't use any of the new features
21:52 Wuzzy      maybe its best to leave this up to the server owner to figure out?
21:52 Wuzzy      allow only new clients to connect, decided by server?
21:52 sfan5      strict_protocol_version_checking is a thing
21:57 erlehmann  do you already do strict checking if a feature is enabled?
21:57 erlehmann  like if a node with feature X is defined, clients in some range can not connect?
21:59 Wuzzy      ahhhh, that setting is what i was looking for
22:01 Wuzzy      I'm starting to think that "doing nothing" is probably the best solution =)
22:01 Wuzzy      just tell server operators to use setring_protocol-version_checking if correctness is important
22:03 Wuzzy      well, so i guess my work on the PR is done then ... erlehmann has helped a LOT with testing (thx) and i also tested a lot. functionally it seems to work great!
22:03 Wuzzy      unless erlehmann has found a bug?
22:04 erlehmann  i have not yet tested the one thing with the different fluid thingy things
22:04 erlehmann  i forgot what
22:04 erlehmann  but i have not tested 100%
22:04 Wuzzy      no-swim?
22:04 erlehmann  this one setting that should be changed
22:04 Wuzzy      movement_liquid_fluidity
22:05 erlehmann  > Also, you might want to change this setting: movement_liquid_fluidity. Test once with default value and one with changed value. The movement speed should always be the same for "Move-resistant node" regardless of setting, but it should change with the setting value for "Move-resistant node, liquidlike".
22:05 erlehmann  yes i have not tested it
22:05 Wuzzy      also, this should change movement speeds in "normal" liquids like water of course
22:06 erlehmann  Wuzzy may i suggest to document in lua_api.txt what does not work in old clients? mod devs will thank you
22:06 erlehmann  everyone who reads the documentation *should* know that
22:06 Wuzzy      idk where to put it... :/
22:06 Wuzzy      do we have a section for that?
22:07 Wuzzy      i am running slowly out of energy ...
22:08 Wuzzy      if you have more things to say about my PR, just post in the PR thanks. I'll be back ... tomorrow!
22:08 Wuzzy      bye