Time  Nick      Message
00:24 PGimeno   hecks: does it really need two updateAbsolutePosition?
00:25 hecks     when attaching? it actually does
00:25 hecks     it doesn't seem like irrlicht evaluates the whole chain
00:25 PGimeno   I think if you update the parent, the child is automatically updated
00:26 hecks     let me check
00:27 hecks     alright, that works too
00:29 PGimeno   wait, that doesn't make sense, and in fact the Irrlicht source doesn't do that
00:29 PGimeno   leave it like that just in case
00:29 hecks     but I tested it and that works
00:29 hecks     but wait
00:29 hecks     my attach position is 0,0,0 so maybe it doesn't
00:31 PGimeno   I think if you're going to update the parents, you better update the whole chain up to the root
00:32 PGimeno   unless there is good reason to think that it won't fail in case of nested attachments
00:39 hecks     I'm not seeing any adverse side effects from deleting the child position update
00:40 hecks     but I'll try a transform chain now
00:43 hecks     a transform chain is causing a small camera glitch, but no catastrophic effects
00:45 PGimeno   huh, do you mean when you run updateAbsolutePosition() on all parents? that should be basically unnoticeable
00:47 PGimeno   you need to do it from root to child, so in postorder, kind of
00:48 PGimeno   every child's position depends on its parent's
00:48 hecks     no I'm getting the camera glitch when attaching the player to a lot of transforms and then only updating the parent
00:48 hecks     I'll update the whole hierarchy now and see what happens
00:48 PGimeno   ah got it
00:51 PGimeno   /** Note: This does not recursively update the parents absolute positions, so if you have a deeper
00:51 PGimeno   hierarchy you might want to update the parents first.*/
00:51 PGimeno   virtual void updateAbsolutePosition()
00:51 PGimeno   (from Irrlicht source code)
00:52 hecks     I didn't gain or lose anything from updating the whole hierarchy
00:52 hecks     I guess it's safer to just do it anyway?
00:52 PGimeno   yeah, it will only matter for nested attachments (e.g. the attachments on a player that gets attached to a boat, for example)
00:52 hecks     be aware that this PR only handles one nasty edge case
00:52 hecks     when the parent has been spawned in the same frame
00:53 hecks     that's the only bug that was here in the first place
00:53 PGimeno   yeah, that makes it unlikely to matter for the nested attachments case
00:53 hecks     this camera thing I guess is a separate thing that just came up
00:54 hecks     so do you think I should commit it with the full chain eval?
00:54 hecks     any of the three versions still fix my bug
00:54 PGimeno   it's unlikely to matter, but it sounds more correct with it
00:55 hecks     also I'm doing this recursive update by getting ClientActiveObject parents rather than traversing the irrlicht hierarchy
00:55 PGimeno   hmm... not sure if that's good
00:56 hecks     I mean they should be equivalent
00:56 hecks     but let me recurse the scene node hierarchy instead and see what that does
00:57 PGimeno   the parent update is pretty much an Irrlicht thing
01:00 hecks     oh hey, camera gremlins are gone
01:00 PGimeno   heh
01:00 hecks     ...not always though
01:00 hecks     another race condition
01:01 PGimeno   gotta love these ^.^
01:01 hecks     the camera thing is related to interpolation; when attaching a CAO to another, the last pos should probably be set to current pos
01:02 hecks     oh crap
01:03 hecks     so this visual glitch does actually affect the position sent to the server
01:05 hecks     this is a part of the same bug, but solving it for large hierarchies would be a lot harder
01:09 hecks     and it is affected by the order in which things are parented to one another
01:16 hecks     so this kinda works but there's bizarre gremlins when longer transform chains are involved
01:17 hecks     the only time this would happen is if you spawn a very complex vehicle made out of nested objects and make the player board it immediately
01:29 hecks     I think the hierarchy gremlins are related to the fact that steps are unsorted and not hierarchical, this must be fixed separately
01:41 PGimeno   I'll leave it in your hands then
12:55 sfan5     just noticed github actions does not let you view build logs or download artifacts without an account
12:55 sfan5     this is bad
12:56 sfan5     one of the selling points of msvc build were so that users could download the build, but in fact most people can't
12:58 Zughy     sorry guys, if I want to convert a string inside a .txt file into a table, what format/function I should use exactly in Lua after I retrieved it? I'm stuck at obtaining "{x=5,y=5,z=5}" (as a string)
12:59 sfan5     that's a modding question and should go to #minetest
13:31 ANAND     I think I'm regaining my footing again: https://github.com/minetest/minetest/pull/7924/files#r436585959
13:31 ANAND     One PR review per day is still just a daydream for me, sadly. I'm yet to find time for that...
13:32 ANAND     Hopefully being more organised ought to give me that extra bit of time I need.
13:33 ANAND     After implementing the middle click feature, I'm planning to work on #9377
13:33 ShadowBot https://github.com/minetest/minetest/issues/9377 -- Change the wield item with no animation
13:33 ANAND     Should be easy from what I've seen
13:45 ANAND     Ideally the "pressed" and "released" key states should be reset from within InputHandler itself.
13:46 ANAND     Instead, this happens in Game::processPlayerInteraction
13:47 ANAND     I'm guessing this was overlooked when refactoring Input-related code from game.cpp to InputHandler.*
13:48 ANAND     Both Game and InputHandler should "step" in sync, for things to continue to work properly though
14:57 ANAND     There was also some weirdness with placement wherein right-clicking while pointing at air is "preserved" until you point at a node, and then the wielded node is placed
14:57 ANAND     Turned out to be a rather simple fix
15:00 ANAND     I've fixed everything (TM) in #7924 now :)
15:00 ShadowBot https://github.com/minetest/minetest/issues/7924 -- Allow binding dig, place actions to keys; remove LMB/RMB hardcoding by ClobberXD
15:10 ANAND     PR is test-worthy again, rubenwardy and Krock
15:10 ANAND     I'll post a detailed comment of the stuff I've tested
15:22 sfan5     it appears yawsprite entities are broken (again?)
15:24 sfan5     or maybe not
15:29 sfan5     yeah it never worked in devtest but nobody noticed
16:13 erlehmann FYI: https://gitlab.freedesktop.org/mesa/mesa/-/issues/3090
16:13 erlehmann > This bug causes accelerated rendering in KiCAD to fail and makes some games like Minetest (a sophisticated open source Minecraft clone) unplayable. I attached some screenshot of the problems below.
16:13 erlehmann > MNT Research GmbH will pay a bounty of EUR 1000.00 to the individual who provides the first correct fix for this bug
16:40 hecks     #10008 now handles whole transform chains, no more gremlins; please review
16:40 ShadowBot https://github.com/minetest/minetest/issues/10008 -- Fix player-to-object attachment teleport bug by hecktest
16:58 sfan5     https://github.com/minetest/minetest/blob/cf961cab78c1a811ff7dcf455c0aaa08186d677a/src/client/content_cao.cpp#L406
16:58 sfan5     hecks: ^ considering getPosition() *adds* the camera offset, wouldn't it be easier to just bypass that and call m_matrixnode->getAbsolutePosition()?
17:00 hecks     well, m_matrixnode->getAbsolutePosition() returns a position in client offset space, and getPosition wants to return world space
17:01 hecks     who calls getPosition and why is not my concern here, I just made sure it returns a valid position all the time
17:02 hecks     this class does a lot of ugly juggling with the camera offset, adding and subtracting it back and forth; all I did was make sure those are balanced
17:03 sfan5     in line 1447 the code calls getPosition (which adds the camera offset), in line 1450 it subtract the camera offset again
17:04 sfan5     my suggestion is not to change getPosition, but to change line 1447 to bypass the added offset to avoid having to subtract it again
17:05 hecks     we don't know if we're actually attached or not in this branch
17:06 hecks     I mean, I can try this, it's a trivial difference
17:07 sfan5     the code is inside a if (!parent) which means it's not attached, doesn't it?
17:08 hecks     nope, this actually breaks it again
17:09 hecks     you see, this branch is taken in an edge case when we've just attached but the client doesn't know the parent yet
17:09 hecks     so, no, the suggested change is not safe
17:11 hecks     getPosition correctly accounts for the actual attachment state, I won't pretend that the offset juggling isn't a mess but let's at least make it a consistent mess for now
17:11 sfan5     to be clear this is what I'm suggesting: http://sprunge.us/If6hou?diff
17:11 hecks     yeah and I just applied this and it brought the bug back
17:11 hecks     I suggest to leave the fix as it is and mark this offset system for a later refactor
17:15 hecks     tell me about the whitespace linting, is it okay to commit that?
17:16 sfan5     large-scale whitespace changes are not okay, but it's just three lines here and I'd say that's okay
17:17 hecks     alright =] a robot should probably lint the whole repo once to avoid things like this
17:29 sfan5     I still don't see what's different about the diff but I can't manage to reproduce the bug so I can't test
17:29 hecks     do you want a more complicated test mod?
17:30 sfan5     nah
17:30 hecks     I mean this isn't easy to reproduce; you have to bring your framerate down somehow, go some distance from the map's center
17:30 hecks     and then spam directional keys hoping to trigger it
17:31 hecks     optionally watching the position in debug because that is also affected
17:31 hecks     the debug pos blinking is a sure tell, the node selector also blinks
17:54 oil_boi   fps limiter in settings and /tp
18:54 hecks     cool, I didn't know there was a limiter
19:08 sfan5     merging #10012
19:08 ShadowBot https://github.com/minetest/minetest/issues/10012 -- devtest: Improve tool and formspec usability by sfan5
19:15 PGimeno   hecks: nice job!
19:19 hecks     PGimeno: thanks for making me test this with whole chains
20:11 hecks     So what do we do when a legit irrlicht bug is encountered? http://irrlicht.sourceforge.net/forum/viewtopic.php?f=7&t=47041
20:11 hecks     This is possibly the cause of #2813
20:11 ShadowBot https://github.com/minetest/minetest/issues/2813 -- Attachments experience movement delays, triggered randomly
20:12 sfan5     patch it in our fork (to be used soon) and hope it gets fixed in 1.9
20:14 hecks     I can try to hack around this; the correct code path exists in the engine, it's just not an exported function
20:52 hecks     oh wow, I think I nailed it
20:52 hecks     but it's a race condition so it might just be hiding
20:58 hecks     and I got another related, more subtle bug while at it
21:35 hecks     okay, hat lag is officially fixed, making PR in a few
21:46 PGimeno   hecks: you're on fire ^.^
21:49 PGimeno   I found another rendering bug that affects attachments, I initially thought it was related to my patch but it wasn't, are you interested in taking a look now that you're all warmed up?
21:49 PGimeno   it's described here: https://notabug.org/pgimeno/minetest/pulls/5#issuecomment-13501
21:50 PGimeno   disregard the reference to the patch, it actually happens also without the patch
22:55 hecks     #10015 is this the oldest bug ever fixed or what, please review
22:55 ShadowBot https://github.com/minetest/minetest/issues/10015 -- Fix bone-attached entities by hecktest
22:56 hecks     PGimeno: you might want to retest this with my branch just in case, it caught some other bugs too
22:57 PGimeno   yeah I will, eventually
22:59 PGimeno   I see that updatePositionRecursive is back
23:00 hecks     yes, it was actually needed this time
23:02 PGimeno   I wonder, isn't root->updateAbsolutePositionOfAllChildren() a viable alternative?
23:02 PGimeno   where root is the root of the scene graph
23:04 hecks     maybe, but that would fall under a general CAO refactor
23:04 hecks     I'm just killing bugs
23:04 PGimeno   the idea being to avoid updating certain stuff twice
23:04 PGimeno   for example, the parent of multiple attachments
23:04 hecks     but curiously, sorting the scene by hierarchy did not fix anything
23:04 hecks     so maybe in some cases re-evaluations are actually needed
23:05 hecks     I mean, this is a very clumsy way of handling transforms in the first place, normally you would mark them as dirty and evaluate just-in-time when reading them
23:06 hecks     but keeping the bug fixes short and simple increases the chances of them getting reviewed and merged in time
23:06 hecks     I'll leave architectural decisions to the big guys
23:09 hecks     actually, updateAbsolutePositionOfAllChildren() is a bone only method
23:09 hecks     there is no equivalent for ISceneNode
23:16 PGimeno   ah, the Irrlicht source mentions doing it once per frame
23:16 PGimeno   I thought it was done through that method