| 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 |
|
behalebabo joined #minetest-dev |
| 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 |
|
ANAND joined #minetest-dev |
| 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 |
| 02:22 |
|
oil_boi joined #minetest-dev |
| 02:27 |
|
Miner_48er joined #minetest-dev |
| 03:42 |
|
gorbachev_pizza_ joined #minetest-dev |
| 05:00 |
|
calcul0n joined #minetest-dev |
| 06:07 |
|
ANAND joined #minetest-dev |
| 07:38 |
|
NetherEran joined #minetest-dev |
| 08:00 |
|
ShadowNinja joined #minetest-dev |
| 08:52 |
|
erlehmann joined #minetest-dev |
| 09:00 |
|
Beton joined #minetest-dev |
| 09:46 |
|
NetherEran joined #minetest-dev |
| 10:07 |
|
ANAND joined #minetest-dev |
| 10:14 |
|
proller joined #minetest-dev |
| 10:37 |
|
Fixer joined #minetest-dev |
| 11:07 |
|
NetherEran joined #minetest-dev |
| 11:22 |
|
oiaohm joined #minetest-dev |
| 11:39 |
|
NetherEran joined #minetest-dev |
| 12:23 |
|
NetherEran joined #minetest-dev |
| 12:45 |
|
Zughy joined #minetest-dev |
| 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 |
|
appguru joined #minetest-dev |
| 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:41 |
|
gorbachev_pizza joined #minetest-dev |
| 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 |
| 15:43 |
|
oil_boi joined #minetest-dev |
| 16:11 |
|
gorbachev_pizza joined #minetest-dev |
| 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:16 |
|
hecks joined #minetest-dev |
| 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:10 |
|
Fixer_ joined #minetest-dev |
| 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:02 |
|
NetherEran joined #minetest-dev |
| 18:40 |
|
Wuzzy joined #minetest-dev |
| 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:39 |
|
olliy joined #minetest-dev |
| 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:29 |
|
clavii joined #minetest-dev |
| 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 |
| 23:54 |
|
erlehmann joined #minetest-dev |
