Time  Nick             Message
00:00 Shara            I don't really see a reason to change obsidian door texture either way
00:01 paramat          hey, user Unarelith (Quent42430 on github) would like to chat here about #7899 and is using Hexchat, but somehow can't talk here, anyone know why?
00:01 ShadowBot        https://github.com/minetest/minetest/issues/7899 -- Architectural improvement needed
00:01 paramat          hm, i think the current handles are awful, but nevermind :)
00:02 Shara            Well, simple straight vertical ones would be nicer. The shape of the suggested change just doesn't map across to so few pixels very well in my opinion
00:03 paramat          ah
00:04 paramat          i might be able to do vertical ones
00:09 Unarelith        .
00:09 Unarelith        ok looks like I'm able to talk here after all
00:10 Unarelith        the most important thing I want to discuss is file splits, for example for content_sao it would look like this: https://github.com/Quent42340/minetest/commit/da9e0c3dc89d857091ebbd89999f69b2429e7e13
00:12 Unarelith        currently, I can't make a PR because this stuff can break current open PRs since they'll be impossible to merge after mine
00:14 Unarelith        but the goal would be to make new PRs easier to merge (more files = less conflicts)
00:22 paramat          ah good
00:24 Unarelith        I think the main problem is that the code is packed into giant files, so splitting them, sorting them and organizing them should make everything more clear
00:26 Unarelith        but with the side-effect of invalidating old PRs, and this side-effect will always be present since PRs will continue to grow
00:26 paramat          the best devs to talk to about code architecture are nerzhul, sfan5 and rubenwardy, best time is EU daytime, especially evening. i'm not so good with complex code architecture stuff
00:28 Unarelith        ok thanks paramat
07:05 nerzhul          Unarelith i agree with you on that point but it's not easy, we started the refactor. This also permits to test the code easily
08:35 nerzhul          i agree about SAO split
08:35 nerzhul          i can provide a such PR
09:43 p_gimeno         I don't follow the logic behind "more files = less conflicts". If two changes touch the same area, they are going to conflict no matter the split, and if they touch different areas, they are not going to conflict, and I don't see how separation into files changes that.
11:41 nerzhul          it's not more files, but more objects, somes are too big i think, i don't know how to split all but this can permit to have also more tests
11:41 nerzhul          the shown separation is logical for me but has no real effect
13:46 Unarelith        nerzhul, I made the SAO split so I can PR my changes if you want
13:46 Unarelith        actually, I can do this for all the files if needed
13:46 nerzhul          you can, but due to the change i tend to prefer to do it myself to ensure we don't miss any line
13:46 nerzhul          and not it's not java we will never do it on all files :p
13:47 Unarelith        you really should tho :/
13:48 Unarelith        because "where the fuck is defined PlayerHPChangeReason? at the end of content_sao.h after 400 unrelated lines" is a problem because the answer could be `PlayerHPChangeReason.hpp`
13:50 Unarelith        <nerzhul> you can, but due to the change i tend to prefer to do it myself to ensure we don't miss any line <= and it's mainly copy pasting, I don't how it could be possible to miss lines
13:54 nerzhul          i'm not with you, code review principes
13:56 Unarelith        small commits shouldn't be hard to review actually
14:10 Unarelith        nerzhul, and btw I work in C++ since 8 years so your "no I'll do it because you'll probably miss lines" is really hard to take actually
14:10 nerzhul          Unarelith, it's not against you, error can happen everytime, and don't forget nobody knows you since yesterday
14:13 nerzhul          Unarelith, provide the SAO PR i will use git diff to ensure it's proper
14:13 nerzhul          i'm okay to split this big file
14:13 nerzhul          i think you neverlook server, serverenv or game :p
14:13 nerzhul          i miss time on MT to perform more refactor but we need to refactor many things
14:16 Unarelith        nerzhul, unfortunately my fork had two commits prior to the SAO split: moving every src/ file (which is not already in a folder) into src/engine and src/engine/client.
14:16 Unarelith        and I don't think this kind of change can be easily merged because of existing PRs
14:17 nerzhul          why did you do this, and why did you fork ?
14:17 nerzhul          i agree we need to move our files to more proper folders to have a better project view, it's a long time issue we do
14:17 nerzhul          see 0.4.16 tree to see before :p
14:18 Unarelith        so you have your answer :)
14:18 Unarelith        my fork was made to give examples for #7899
14:18 ShadowBot        https://github.com/minetest/minetest/issues/7899 -- Architectural improvement needed
14:20 Unarelith        so I can still make a PR with what I did, but then we'll have to change some older PRs
14:21 nerzhul          not a problem
14:21 nerzhul          open the split the sao PR
14:24 Unarelith        nerzhul, I opened #7900 with what I did on my fork
14:24 ShadowBot        https://github.com/minetest/minetest/issues/7900 -- Global code architecture improvement + SAO split by Quent42340
15:07 Unarelith        nerzhul, I had to do a full cmake clean to see missing/invalid headers in current build. CMake is supposed to handle that, why isn't that the case here?
15:40 nerzhul          because you miss something about cmake :D
15:52 Unarelith        nerzhul, what do you mean?
16:01 Unarelith        anyone knowns what's the purpose of this line? `LuaEntitySAO proto_LuaEntitySAO(NULL, v3f(0,0,0), "_prototype", "");`
16:02 sfan5            context?
16:04 Unarelith        sfan5, https://github.com/minetest/minetest/blob/master/src/content_sao.cpp#L297
16:04 sfan5            looks unused to me
16:05 rubenwardy       it looks like it was originally intended to be an object that you copy and then deserialise into
16:05 rubenwardy       bad coding style, though
16:07 Unarelith        so it's unused? because it causes crash in my fork due to the usage of ServerActiveObject::registerType in LuaEntitySAO constructor
16:07 Unarelith        that's why this: `std::map<u16, ServerActiveObject::Factory> ServerActiveObject::m_types;` is in content_sao.cpp and not in serverobject.cpp
16:08 Unarelith        I'll remove it to fix the crash then
16:10 Unarelith        rubenwardy, about #7900, you said you didn't liked `engine` folder name, any better idea?
16:10 ShadowBot        https://github.com/minetest/minetest/issues/7900 -- Global code architecture improvement + SAO split by Quent42340
16:10 T4im             if it registers itself form the constructor, you should probably assume it not being dead code
16:10 rubenwardy       src/client exists because things are starting to be partially moved into subfolders
16:11 rubenwardy       so I'm not sure why you can't move the rest of the client stuff into there
16:14 Unarelith        rubenwardy, ok then I'll merge `src/engine/client` into `src/client`
16:15 rubenwardy       sounds good
16:39 Unarelith        T4im, I understand your concern but everything seems to work after removing it
16:40 Unarelith        so it looks like legacy code which hasn't been touched since ages
16:40 T4im             :D
16:41 Unarelith        and if anything ever goes wrong I'll remember to try adding it back into the code first :p
16:41 Unarelith        rubenwardy, I don't think 80 column debate is relevant for #7899
16:41 ShadowBot        https://github.com/minetest/minetest/issues/7899 -- Architectural improvement needed
16:42 rubenwardy       you did mention it in the OP
16:42 rubenwardy       maybe make a separate issue?
16:44 Unarelith        sure, but it was only a suggestion, if any debate on the topic is _really_ needed then I'll make another issue, but I think it could be better to discuss it here
16:44 rubenwardy       oh lol, found some random compat code
16:44 rubenwardy       nice
16:46 Unarelith        btw, should I let my PR in its current state or can I keep working on stuff?
16:49 rubenwardy       looks like a good first PR
16:49 rubenwardy       I'm currently trying to work out what proto_LuaEntitySAO is, and how the system works
16:50 Unarelith        I tried a lot of things, like creating a world, joining a server, and everything seems to work fine
16:50 rubenwardy       was added in this commit: https://github.com/minetest/minetest/commit/bfc68d31510bbd40732c19ada51d4683cb050de2
16:50 rubenwardy       not very helpful
16:51 rubenwardy       there's also TestSAO proto_TestSAO(NULL, v3f(0,0,0));
16:52 Unarelith        I removed it too
16:55 Unarelith        but maybe we should just ask this to celeron55?
16:58 Unarelith        hmm my travis build is failing but I don't know why
16:59 rubenwardy       you probably need to update the travis whitelist
16:59 rubenwardy       some old files are whitelisted by the code style litner
16:59 rubenwardy       ah, also builds
17:00 rubenwardy       looks like IRrlicht isn't included correctly
17:00 Unarelith        weird, I didn't make any change about Irrlicht
17:01 Unarelith        oh
17:01 Unarelith        I actually failed an unit test
17:02 Unarelith        testStreamRead failed
17:03 rubenwardy       it looks like the proto is used, weirdly, to register a factory to ServerActiveObject
17:03 rubenwardy       looks like it's called when a mapblock is loaded
17:04 Unarelith        so it's still needed somehow?
17:04 rubenwardy       so when testing, try spawning entities (ie: dropping an item), shutting down and restarting
17:04 rubenwardy       this is disgusting
17:04 rubenwardy       well, I guess it's one way of having code register itself without having to be called by somefunction
17:04 rubenwardy       as the code runs on init due to the constructor
17:04 rubenwardy       still a hack
17:05 Unarelith        well, I'll add it back then
17:05 Unarelith        will need another PR to fix that
17:05 rubenwardy       with the proto removed, you should get: "ServerEnvironment::activateObjects(): failed to create active object from static object"
17:05 rubenwardy       heh
17:05 rubenwardy       couldn't you just ammend it on?
17:06 Unarelith        I meant another PR to fix the need for this "thing"
17:06 rubenwardy       ah right, yeah
17:07 Unarelith        btw, is TestSAO still needed?
17:08 Unarelith        because it will be impossible to split TestSAO and LuaEntitySAO since they both need `m_types` declaration first: std::map<u16, ServerActiveObject::Factory> ServerActiveObject::m_types;
17:08 Unarelith        since this variable is used in `registerType`
17:09 rubenwardy       isn't that just a forward declaration?
17:09 rubenwardy       wait, no
17:09 Unarelith        nope, it's the definition, the declaration is in serverobject.h, hmm or I could just add the protos to serverobject.cpp
17:13 Unarelith        so I added protos back to serverobject.cpp with a FIXME note indicating that their presence here is temporary and they need to move
17:13 Unarelith        [FAIL] testStreamRead - 0ms
17:13 Unarelith        [FAIL] testBufReader - 0ms
17:13 Unarelith        Unit Test Results: FAILED
17:14 Unarelith        though somehow I fail some unit tests
17:17 Unarelith        ok so actually minetest master branch fails the same tests
17:18 rubenwardy       lol
17:18 rubenwardy       unit tests are ran in the CI, though
17:18 rubenwardy       m
17:18 rubenwardy       oops, wrong window
17:18 rubenwardy       (   alias m="make -j9"   )    ;)
17:19 rubenwardy       works for me
17:19 rubenwardy       [PASS] testStreamRead - 0ms
17:21 Unarelith        hmmm weird, then why my build fails? :/
17:21 rubenwardy       maybe make clean is needed?
17:21 rubenwardy       or it could be a platform issue
17:21 rubenwardy       doubgt it
17:21 rubenwardy       s/platform/undefined behaviour/g
17:25 Unarelith        I tried updating my system, I cleaned and restarted the build, we'll see
17:27 Unarelith        I still fail the same tests
17:28 Unarelith        Test assertion failed: readF1000(is) == 53.534f    at test_serialization.cpp:305
17:28 Unarelith        Test assertion failed: buf.getF1000() == 53.534f    at test_serialization.cpp:472
17:30 rubenwardy       well, probably fine then
17:31 rubenwardy       well, no
17:32 rubenwardy       #3943
17:32 rubenwardy       lol
17:32 ShadowBot        https://github.com/minetest/minetest/issues/3943 -- testStreamRead and testBufReader failures
17:33 Unarelith        so it's a known thing for two years, ok x)
17:36 rubenwardy       nerzhul: is it possible to restart this job? https://gitlab.com/minetest/minetest/-/jobs/125127331
17:36 rubenwardy       and also answer your PMs ;)
17:40 p_gimeno         I'm interested in analyzing 3943. Where's the test that fails?
17:41 p_gimeno         ah, test_serialization.cpp:305, sorry
17:47 p_gimeno         Unarelith: if you change 53.534f to 53.534000396728515625f does it pass?
17:48 p_gimeno         (that particular test)
17:49 Unarelith        still doesn't pass
17:49 p_gimeno         hm, the problem is probably the opposite, 53.534f is correct but the incoming value is a double and is being compared as double
17:49 p_gimeno         thanks
17:52 p_gimeno         could you please try temporarily replacing line 305 with this and see if it still fails? {volatile float f = readF1000(is); UASSERT(f == 53.534f);}
17:53 Unarelith        still fails :/
17:53 p_gimeno         wow
17:55 p_gimeno         could you please add this before the UASSERT and check if it's displayed? printf("readF1000=%a expected=%a\n", f, 53.534f);
17:55 rubenwardy       well, the issue is probably right in that it should be a range check
17:55 Unarelith        damn wait p_gimeno I may be have been stupid
17:56 p_gimeno         it's not easy to find a platform where it fails
17:58 Unarelith        p_gimeno, so I tried again you two suggestions to check if the error line actually changed, but without success
17:59 p_gimeno         could you try to get the output from printf, please?
17:59 Unarelith        p_gimeno, that's what I was doing :p
18:00 Unarelith        readF1000=0x1.ac45a4p+5 expected=0x1.ac45a2p+5
18:00 p_gimeno         wow
18:00 p_gimeno         holy rounding error, Batman
18:00 p_gimeno         thank you :)
18:02 p_gimeno         the double result is 0x1.ac45a1cac0831p+5, so actually under 0x1.ac45a2p+5 yet the result is 1 ulp above
18:02 * p_gimeno       looks into readF1000 again
18:10 p_gimeno         ok, so the best explanation I have is that there's some optimization that is changing the division into a multiplication by the reciprocal
18:11 p_gimeno         multiplying 53534.f by (float)(1.f/1000.f) can cause exactly that discrepancy
18:15 p_gimeno         Unarelith: any idea of the exact compiler flags used to compile src/util/serialize.cpp?
18:15 p_gimeno         (and what compiler?)
18:16 Unarelith        my compiler is gcc version 8.2.1 20180831 (GCC)
18:16 Unarelith        and I don't know for the compiler flags
18:17 p_gimeno         what processor/bits?
18:18 Unarelith        i7 7th gen/64 bits
18:22 p_gimeno         it's a very surprising result... I wonder if -ffast-math or some other unsafe math optimization flag is in effect
18:22 rubenwardy       I use an i7 8550U (ie: 8th gen, 64bit)
18:22 celeron55        oh you figured out the AO factory thing on your own, good 8)
18:23 nerzhul          rubenwardy: why not
18:23 celeron55        would have created some nasty bugs if it was removed without moving elsewhere
18:23 rubenwardy       from what I can see, it would've broken map loading from disk
18:24 rubenwardy       I really need a better IDE, it's really painful trying to navigate to symbols
18:24 nerzhul          rubenwardy clion
18:24 rubenwardy       maybe I should try that again :)
18:24 nerzhul          yeah don't do a massive refactor without understanding all
18:24 nerzhul          it's not as simple :p
18:27 rubenwardy       also, having classes only in .cpp files is perfectly valid
18:27 nerzhul          if they are intended to be private yes
18:27 rubenwardy       it's a classic example of implementation / interface
18:27 rubenwardy       which is heh
18:29 rubenwardy       the classes in game.cpp are disgusting though, and a bad example
18:30 rubenwardy       inputhandler should be in inputhandler.h (which actually exists)
18:30 rubenwardy       +already
18:32 Krock            to be fair, game.cpp probably never had a major code movement, resulting in partially messy code from "the old days"
18:33 rubenwardy       yeah
18:33 rubenwardy       I really want to work on an input rework at some point, but I always feel a bit sick after trying
18:33 rubenwardy       partially Irrlicht's fault, tbh
18:35 rubenwardy       tbh, I do too much talking and not enough actual coding
18:37 rubenwardy       merging #7863 in 10
18:37 ShadowBot        https://github.com/minetest/minetest/issues/7863 -- Don't display page-nav buttons when #store.packages == 0 by ClobberXD
18:37 Krock            if I were just 30s earlier...
18:37 rubenwardy       looool
18:38 rubenwardy       you can if you like
18:38 Krock            nah, go on ;)
18:38 rubenwardy       I'll probably forget anyway, I usually do
18:43 rubenwardy       #7901
18:43 ShadowBot        https://github.com/minetest/minetest/issues/7901 -- Accepting PRs/patches from non-Github users
18:43 rubenwardy       p_gimeno ^
18:45 p_gimeno         sweet :)
18:45 rubenwardy       updated
18:46 rubenwardy       mergin...
18:47 rubenwardy       done
18:49 p_gimeno         I like the bot approach, if it's able to fetch any remote branch on any repo no matter where it's hosted
18:50 rubenwardy       that's part of the issue
18:50 ANAND            Thanks :)
18:50 rubenwardy       the bot would need to have it's own fork and update the branches, in that case
18:50 rubenwardy       it's probably not the easiest solution
18:51 p_gimeno         sounds like quite some work, yeah
18:53 p_gimeno         I'm willing to do it on GitLab
18:54 rubenwardy       that's probably the easiest method out of the ones given
18:54 rubenwardy       well, idk
18:58 Krock            Unarelith: thanks for #7902. Makes things easier. What's "testbm.txt" btw?
18:58 ShadowBot        https://github.com/minetest/minetest/issues/7902 -- Client-specific files moved to 'src/client' by Quent42340
18:59 Unarelith        Krock, np, and "testbm.txt" is a file sometimes generated by MT build
19:00 Krock            weird. will check where this comes from
19:00 Krock            ah. perhaps just ignore the entire test world
19:01 Krock            src/unittest/test_world/world.mt -> src/unittest/test_world/
19:02 Unarelith        the file is generated in the repo root
19:02 p_gimeno         I can only reproduce a reduced version of #3943 <https://paste.scratchbook.ch/view/69368d2b>, if I activate either -ffast-math or -funsafe-math-optimizations, trying more FP-related flags...
19:02 ShadowBot        https://github.com/minetest/minetest/issues/3943 -- testStreamRead and testBufReader failures
19:02 Krock            oh, that's not related to the test world, more as a side-product of the unittests overall
19:02 Unarelith        I guess so
19:03 Krock            test_ban.cpp apparently does not delete that file after finishing the tests
19:05 Unarelith        Krock, for file naming, is my suggestion open to discussion?
19:05 Krock            yes sure. was just my personal opinion on keeping it consistent with the existing code base
19:06 Krock            also personal preference
19:06 Unarelith        it's mine too, purely subjective, but I have arguments to defend it :p
19:07 rubenwardy       I'd prefer names which match class names
19:08 rubenwardy       + where relevant
19:08 Krock            there used to be meetings on Saturday around 9 PM UTC where most of the devs were online. Too bad such discussions are now much more luck-based and spontaneous
19:08 rubenwardy       this could be left until later, though
19:09 Krock            btw, the Android source list is in build/android/jni/Android.mk
19:31 rubenwardy       oh nice, you made your own MC-like
19:37 Unarelith        Krock, rubenwardy, why you don't use automatic .cpp file gathering instead of having them manually entered into Android.mk AND CMakeLists.txt?
19:37 Unarelith        it's so inconvenient
19:38 rubenwardy       because the Android build system is completely shit
19:38 sfan5            android.mk is a hack, it should not exist
19:38 rubenwardy       ^
19:38 sfan5            android should use cmake like any other platform
19:39 rubenwardy       sfan5 was working on a nice cmake build system because this hacky shit by sapier was merged
19:39 rubenwardy       there's a PR for this by nerzhul
19:39 sfan5            nrz you mean
19:39 Unarelith        it's still possible to do this inside Android.mk
19:39 sfan5            my cmake build system predates sapier's android port
19:40 sfan5            Unarelith: either way I wouldn't worry too much about android right now when developing
19:40 rubenwardy       that's what I was referring to. Did you stop working on it before sapier started porting?
19:40 rubenwardy       and yeah, you could still use find
19:40 sfan5            i stopped working on it eventually when sapier's port was nearing completion
19:40 sfan5            (because his stuff worked and mine didn't at that point)
19:41 rubenwardy       thought so
19:41 rubenwardy       #7123
19:41 ShadowBot        https://github.com/minetest/minetest/issues/7123 -- Android: switch to cmake based standard builds by nerzhul
19:43 Unarelith        sfan5, the only problem is keeping Android.mk valid, which is a pain due to all the files I moved
19:46 Unarelith        rubenwardy, Android.mk updated in both PRs
19:47 rubenwardy       nice, thanks
19:49 Unarelith        rubenwardy, what was the issue with mainmenu?
19:50 rubenwardy       well, gui contains stuff which is also used by the main menu
19:50 rubenwardy       so it depends whether you mean client as the state of being, or client as the build target
19:50 rubenwardy       like, should main menu stuff be in client?
19:50 Krock            I'd assume "client" is the in-game client
19:51 rubenwardy       in which case src/gui should stay separate
19:51 rubenwardy       but it shouldn't have references to src/client in it, really
19:51 Krock            well, both mainmenu and in-game client need the gui
19:52 Unarelith        in my opinion, the code should be separated between 'client', 'server' and 'common'
19:54 Krock            so GUI would be common..?
19:54 Unarelith        that would be weird indeed
19:54 rubenwardy       GUI isn't used by server, though
19:55 Unarelith        then it should go into client
19:55 rubenwardy       client would be the client target in this case, rather than the client in the server-client relationship
19:55 Unarelith        it would be, and maybe a new folder `client/game/` could be the in-game client
19:56 rubenwardy       maybe
19:56 rubenwardy       that could work
19:56 rubenwardy       but game is also a bad term
19:56 Unarelith        sure, I don't have a better name yet
20:01 nerzhul          Unarelith i had a such opinion 2 years ago
20:01 nerzhul          GUI is client only
20:01 nerzhul          i think we need common & client only
20:02 nerzhul          let me push some little reorganization PR
20:02 rubenwardy       err, please don't conflict with each other XD
20:02 nerzhul          i will split this in multiple parts to ensure we can do an easy bissect if needed
20:03 rubenwardy       have you seen Unarelith's PRs, nerzhul?
20:04 nerzhul          i will look
20:04 Unarelith        <rubenwardy> oh nice, you made your own MC-like <= I missed it :O
20:05 rubenwardy       sorry, I stalked you briefly
20:05 nerzhul          hmm they seems to be very big
20:05 rubenwardy       well yeah, there's a lot of files
20:05 rubenwardy       luckly the client one won't break much because git can handle files changing location
20:05 nerzhul          i don't agree with .hpp
20:05 Unarelith        rubenwardy, np haha, this is a pretty old project I tried to revive a few months ago
20:05 rubenwardy       :(
20:05 nerzhul          .h is our convention
20:05 Unarelith        nerzhul, makes no sense
20:05 rubenwardy       I think we can change to .hpp though
20:06 Unarelith        .hpp is for C++, .h is for C
20:06 rubenwardy       just because .h is currently what we use
20:06 nerzhul          we are not using this
20:06 rubenwardy       and yeah, what Unarelith
20:06 nerzhul          and we will not use
20:06 rubenwardy       +Said
20:06 nerzhul          .hpp is not very common in C++
20:06 rubenwardy       really?
20:06 nerzhul          look at stlibc++
20:06 nerzhul          haha
20:06 nerzhul          .h everytime
20:06 nerzhul          it's used, but not everywhere
20:06 Unarelith        "look at something really old, see, I'm right"
20:07 rubenwardy       we currently have a .gitattributes file to tell Github that .h is for C++
20:07 Unarelith        .hpp is for modern C++, to make the distinction between .h and .hpp
20:07 Krock            well, in a certain way he's right
20:07 nerzhul          we will not change this currently
20:07 rubenwardy       we also use #pragma once, which is probably not as widely used and is *non-standard*
20:08 rubenwardy       but anyway, I suggest leaving it out of that PR
20:08 nerzhul          i see this in the gnome C code
20:08 nerzhul          yeah don't change convention right now
20:08 Unarelith        nerzhul, https://stackoverflow.com/questions/152555/h-or-hpp-for-your-class-definitions
20:08 p_gimeno         I think I've seen more .h than .hpp for C++ code
20:09 rubenwardy       I'm actually surprised at this, would've thought that the separate extension for C++ would've been more common
20:09 Unarelith        same for me, but seems like .h is still the default in IDE these days so people don't bother change it
20:10 nerzhul          yep
20:11 nerzhul          it's okay except the file names for the SAO thing
20:13 nerzhul          for the client move, ty git
20:13 nerzhul          i commented what i wanted for inclusion
20:14 nerzhul          i don't think we should add -I flag on server to include client files
20:14 rubenwardy       I agree with that
20:14 rubenwardy       I imagine it's a stepping stone
20:14 nerzhul          yes but error prone in IDE
20:14 nerzhul          i prefer explicit fix
20:14 rubenwardy       I  agree with not adding the -I flag *
20:14 rubenwardy       is what I meant
20:15 nerzhul          it's added with the include_directories(.../client)
20:15 nerzhul          fix the headers
20:15 nerzhul          it will permit to track client code in common files if we find it
20:15 nerzhul          the include_directories should be in the client/CMakeLists.txt part
20:16 nerzhul          but later
20:16 nerzhul          be explicit at now
20:16 Unarelith        nerzhul, ok then I'll do it, but it will take a LOT of time since there's some files I don't even compile on my system, which are only checked by a 15 min long travis build
20:16 nerzhul          wtf
20:16 rubenwardy       oh really
20:16 nerzhul          why don't you compile ?
20:17 rubenwardy       separate target?
20:17 Unarelith        I'm building on ArchLinux, without postgresql, without windows-specific files, etc...
20:17 rubenwardy       ah ofc
20:17 nerzhul          i'm building on archlinux too
20:17 nerzhul          with postgresql but without windows
20:17 Unarelith        Moreover some include directives are not needed on my PC but needed for travis
20:17 nerzhul          i'm using clion too
20:18 Unarelith        I'm using vim tho
20:18 nerzhul          omg
20:18 nerzhul          use qtcreator at least if you don't have a licensed thing
20:18 nerzhul          or gnome builder
20:18 Unarelith        ._.
20:18 nerzhul          C++ without good IDE is dangerous :p
20:18 Unarelith        I'll never switch from vim dude, why would I ever use anything else?
20:19 nerzhul          use emacs *jokeù
20:19 jacky            lol
20:19 nerzhul          (i don't use emacs but vim)
20:19 nerzhul          cmake integration in IDE is good
20:19 jacky            yeah vim + cmake + good tests and you're good
20:19 nerzhul          and it's easy on archlinux to compile
20:19 nerzhul          pacman -S cmake base-devel clang
20:19 nerzhul          mkdir cmakebuild && cmakebuild && cmake .. -DRUN_IN_PLACE=1 && make
20:19 rubenwardy       vim's symbol jumping with ctags is pretty aweosme
20:20 nerzhul          i don't know about that
20:20 Unarelith        rubenwardy, I'm also using YouCompleteMe for code completion
20:20 rubenwardy       the issue is that it's vim, however
20:20 nerzhul          then i will go look for marvel agents of shield, see you in 1 or 2 hours
20:20 rubenwardy       ooh nice, I like that show
20:20 nerzhul          please remove the include_directories and fix headers :p
20:20 rubenwardy       lol
21:04 Unarelith        nerzhul, I made the changes you requested, though #7902 may still not build on travis due to missing fixes
21:04 ShadowBot        https://github.com/minetest/minetest/issues/7902 -- Client-specific files moved to 'src/client' by Quent42340
21:31 paramat          nerzhul is #7820 mergeable? it seems to be running ok for those who tested. is that decremented loop the only issue? the indents have been changed back to tabs
21:31 ShadowBot        https://github.com/minetest/minetest/issues/7820 -- Update Android java-part by MoNTE48
21:32 paramat          also everyone, #7850 just needs 1 more approval
21:33 ShadowBot        https://github.com/minetest/minetest/issues/7850 -- Make non-formspec modal menus respect gui scale by stujones11
21:37 paramat          as does #7395
21:37 ShadowBot        https://github.com/minetest/minetest/issues/7395 -- Add Lua methods 'set_rotation()' and 'get_rotation()' by CoderForTheBetter
21:55 nerzhul          merging #7850
21:55 ShadowBot        https://github.com/minetest/minetest/issues/7850 -- Make non-formspec modal menus respect gui scale by stujones11
22:01 paramat          :D
22:07 paramat          down to 5 blockers
22:09 paramat          actually 6
22:20 Unarelith        paramat, #7395 seems to be the only one of these 4 PRs which could create conflicts with my PRs: https://github.com/minetest/minetest/pulls?q=is%3Aopen+is%3Apr+label%3ABlocker
22:20 ShadowBot        https://github.com/minetest/minetest/issues/7395 -- Add Lua methods 'set_rotation()' and 'get_rotation()' by CoderForTheBetter
22:21 Unarelith        #7891 and #7820 could too but on a really small scale (.gitignore, l_mainmenu.cpp)
22:21 ShadowBot        https://github.com/minetest/minetest/issues/7891 -- Fix ContentDB packages timing out by using download_file instead by rubenwardy
22:21 ShadowBot        https://github.com/minetest/minetest/issues/7820 -- Update Android java-part by MoNTE48
22:21 Unarelith        But you mentioned 6 blockers so I don't know for the other two
22:24 Unarelith        *sorry I meant #7768 not 7820
22:24 ShadowBot        https://github.com/minetest/minetest/issues/7768 -- Network: Send IEEE floats by SmallJoker
22:31 paramat          ok. sorry by '6' i mean including 2 issues
22:32 Unarelith        oh ok
22:32 paramat          i'll push to get those blockers merged soon, especially 7395
22:59 CoderForTheBette Yeah, we should get #7395 merged so I don't have to rebase it (again). I have used my PR in my own branch and have not noticed anything weird or any extra lag with it. If there are any questions just post it on the Github PR, I'm still around!
22:59 ShadowBot        https://github.com/minetest/minetest/issues/7395 -- Add Lua methods 'set_rotation()' and 'get_rotation()' by CoderForTheBetter
23:05 p_gimeno         hm, I'm checking 7395 and got some problems with it
23:06 CoderForTheBette What seems to be the problems?
23:08 p_gimeno         I am looking closer, but I'm not sure the player is correctly marked dirty
23:09 CoderForTheBette Ah, that could be a problem! Good catch, I'll take a look at the code right now to help verify that is something that might happen.
23:09 p_gimeno         also, it would be best if accompanied by #7768, to add precision
23:09 ShadowBot        https://github.com/minetest/minetest/issues/7768 -- Network: Send IEEE floats by SmallJoker
23:10 CoderForTheBette This could also be a problem that was occurring before my PR.
23:11 p_gimeno         yes, it's independent
23:12 CoderForTheBette For sure, let's see who gets their PR merged first. However, if it comes to it then I do not think rebasing mine would be too hard. I think it also would not be too hard to rebase that one as well. Which ever comes first.
23:13 p_gimeno         who does the heavy lifting? I don't see .X used in the PR, for instance
23:14 p_gimeno         I mean, I'm trying to understand how these angles are used, but I don't see anything in the PR, is it because that was pre-existing code?
23:16 CoderForTheBette This was semi existing code that I had to modify to use the vector3df type instead of just a float.
23:16 CoderForTheBette Let me see if I can get a path of what happens when the added lua methods are called to where the rotation takes place.
23:17 p_gimeno         also, are the internal values in degrees???
23:17 p_gimeno         because otherwise this doesn't look right to me: +	float yaw = co->getRotation().Y * core::DEGTORAD;
23:17 CoderForTheBette Yes, it has always been like that since Irrlicht uses that. You can look at the old code and it does the same thing.
23:17 p_gimeno         eek
23:18 CoderForTheBette I know, gross.
23:19 CoderForTheBette In fact, part of my PR preserves the old setYaw() methods so here is exactly where this "old" code is: https://github.com/minetest/minetest/pull/7395/files#diff-9444313da8be4290e9304a4b1faed804R935
23:21 CoderForTheBette Remember, the units go in as radians, get converted to degrees, and then if getYaw() or getRotation() is called they are converted back to radians from degrees.
23:21 p_gimeno         thanks
23:23 CoderForTheBette Anyway, back to this set the player dirty stuff. I don't see where anything changed when it comes to this. If you have a scenario/proof that something is wrong then please tell us!
23:25 p_gimeno         is this where it's passed to the engine?		node->setRotation(rot);
23:26 CoderForTheBette yes, right here: https://github.com/minetest/minetest/pull/7395/files#diff-722710b5ecd9db42e7dd785dd517e186R774
23:26 CoderForTheBette oh, one line lower.
23:26 p_gimeno         yes, thanks
23:28 p_gimeno         right there, why obtain rot two lines above?
23:28 p_gimeno         it made sense when only Y was changed, but now it's completely replaced
23:29 CoderForTheBette hm
23:29 CoderForTheBette Yeah, no sense in doing that.
23:29 CoderForTheBette Line 773, right?
23:29 p_gimeno         right
23:30 p_gimeno         I'm also a bit puzzled by the minus sign
23:30 CoderForTheBette Doesn't seem necessary.
23:30 CoderForTheBette not the minus sign, sorry.
23:31 p_gimeno         yeah, sorry, I'm firing too many comments in a row :)
23:32 CoderForTheBette I honestly forget why the minus sign is needed. I do remember that if I got rid of it then something did not work. This was months ago however. I'll do some more investigating real quick.
23:34 p_gimeno         with MT's weird axis convention and left-handed reference frame, I wouldn't be surprised if only one of these needs a minus sign
23:34 paramat          i'll mark the PR WIP for now then
23:35 Unarelith        I'm working on using `file(GLOB *.cpp)` in CMake instead of manually writing each filename, but I have an issue with `src/settings_translation_file.cpp` since it's not a real C++ file
23:36 Unarelith        can I move this file? will it cause hundreds of changes in .po files?
23:37 Unarelith        I don't know anything about gettext
23:37 CoderForTheBette Weird, Minetest was already obtaining 'rot' before but it was only a single axis: https://github.com/minetest/minetest/blob/master/src/content_cao.cpp#L738
23:38 CoderForTheBette wait, not single axis
23:38 CoderForTheBette it was all three, but from the engine
23:38 p_gimeno         yes, it got all three, changed Y, set all three (IIUC)
23:39 CoderForTheBette But now that should not be a problem since all three will be stored and can just be set each time without getting the engine's. That is, along as the rotation is not modified anywhere else in MInetest.
23:41 CoderForTheBette Ehhh, forget that last bit. I'm pretty sure that it is just completely unnecessary.
23:41 p_gimeno         yep
23:42 CoderForTheBette Ok, Ill get rid of that line by just defining a v3f and let it use its default constructor to set it to 0,0,0
23:53 p_gimeno         interpolating in Euler is also yucky, but I think that given the use cases for interpolation, a proper slerp is not necessary
23:57 CoderForTheBette Yeah, I don't like the way I'm doing it in this PR, but I think someone requested it. It does look good. I extended the code (again) that was in place for interpolating a single axis.