| Time |
Nick |
Message |
| 02:51 |
|
YuGiOhJCJ joined #minetest-dev |
| 04:00 |
|
MTDiscord joined #minetest-dev |
| 05:09 |
|
calcul0n joined #minetest-dev |
| 05:54 |
|
tekakutli joined #minetest-dev |
| 08:22 |
|
tekakutli joined #minetest-dev |
| 09:55 |
|
appguru joined #minetest-dev |
| 11:17 |
|
fluxionary joined #minetest-dev |
| 12:11 |
|
YuGiOhJCJ joined #minetest-dev |
| 12:24 |
|
appguru joined #minetest-dev |
| 12:33 |
|
proller joined #minetest-dev |
| 12:34 |
|
afcm joined #minetest-dev |
| 13:16 |
|
Desour joined #minetest-dev |
| 13:44 |
|
Desour_ joined #minetest-dev |
| 14:06 |
|
izzyb joined #minetest-dev |
| 14:06 |
|
freshreplicant[m joined #minetest-dev |
| 14:50 |
|
appguru joined #minetest-dev |
| 16:01 |
|
proller joined #minetest-dev |
| 17:22 |
|
appguru joined #minetest-dev |
| 17:40 |
|
Desour joined #minetest-dev |
| 18:11 |
|
lhofhansl joined #minetest-dev |
| 18:12 |
lhofhansl |
Hi all... Planing to merge #13468 soon. |
| 18:12 |
ShadowBot |
https://github.com/minetest/minetest/issues/13468 -- Guarantee m_active_objects is not modified while iterating by lhofhansl |
| 18:17 |
lhofhansl |
And... Done. |
| 18:19 |
Krock |
!tell Desour what do you mean by https://github.com/minetest/minetest/issues/13465#issuecomment-1527886581 ? Only callbacks can change the size, and that's currently protected. Hence, the references cannot become invalid |
| 18:19 |
ShadowBot |
Krock: OK. |
| 18:19 |
Krock |
ShadowBot <3 |
| 18:19 |
ShadowBot |
Krock: An error has occurred and has been logged. Please contact this bot's administrator for more information. |
| 18:20 |
Krock |
perfect |
| 18:25 |
lhofhansl |
:) |
| 18:37 |
Pexin |
ShadowBot less than three |
| 18:56 |
|
Desour joined #minetest-dev |
| 19:02 |
Desour |
Krock: the list_to_lock is released before calling the put/move/take callbacks. the problem is that the list (and its size) is used in the calling function, after the recursive call |
| 19:03 |
Desour |
(idk what I've done to the bots, but they seem to dislike me. the shady one at least never sends me tell-messages );) |
| 19:06 |
Krock |
ah right. that sucks |
| 19:10 |
Krock |
so a check for the entire inventorylist is needed inside the loop (after executing the first apply()) |
| 19:10 |
Krock |
yet that cannot capture either when a node is removed within the callback |
| 19:27 |
Desour |
Krock: #13470 |
| 19:27 |
ShadowBot |
https://github.com/minetest/minetest/issues/13470 -- Release invlist resizelock while doing the recursive callback in move_somewhere mode by Desour |
| 19:28 |
Desour |
what should we do if an invlist suddenly disappears? |
| 19:28 |
Desour |
just returning feels wrong, because invaction seems to hold some weird state |
| 19:31 |
Krock |
the nested calls are protected by the name lookup already |
| 19:32 |
Krock |
problem are only the move_somewhere loop from what I can see |
| 19:32 |
Desour |
yes |
| 19:32 |
Desour |
the outer thing has to protect its stuff from the inner thing |
| 19:33 |
Krock |
then why did you move the "list_from" check below the loop? |
| 19:34 |
Krock |
I think it would be more straight-forward to either throw and catch an exception or to have return values to indicate failure |
| 19:35 |
Krock |
that would basically replace the need for checking the validity of the list within the loop since that's already done by the nested apply() call |
| 19:36 |
Krock |
> //TODO: how to handle? |
| 19:37 |
Krock |
no special code at all. there's already an infostream log above |
| 19:39 |
Desour |
I've moved list_from down because we don't need to lock it up there |
| 19:40 |
Desour |
>that would basically replace the need for checking the validity of the list within the loop since that's already done by the nested apply() call |
| 19:41 |
Desour |
apply() only checks the lists when it's called, not after its call. we need the list after calling it |
| 19:43 |
Desour |
updated the PR. it now effectively just breaks the loops when the to-list is removed. so, shift-clicking will just do partial work |
| 19:43 |
Desour |
this should be fine I think |
| 19:52 |
Krock |
> not after its call |
| 19:52 |
Krock |
hence the suggestion to use return values (or exceptions) |
| 19:53 |
Desour |
but the return value would then say if the invlist was removed, which is not a failure of the apply function |
| 19:55 |
Krock |
no but indicating whether apply() handled the item stack would be an indicator whether or not to continue the loop. anyway. the current state seems to do the trick as well |
| 19:57 |
Desour |
apply() can handle the item partly (i.e. an allow_put can return a smaller item count), and the list can be removed regardless |
| 19:57 |
Desour |
maybe I'm misunderstanding you x] |
| 20:00 |
Krock |
no, you understood me correct. I just appear to be tired to have missed that |
| 20:02 |
Desour |
ah ok, good then. anyway, thanks for the suggestions! |
| 20:05 |
Krock |
I'll have a PR later on. thanks for digging into that |
| 20:05 |
Krock |
*look into the PR |
| 20:12 |
|
proller joined #minetest-dev |
| 20:12 |
|
appguru joined #minetest-dev |
| 21:10 |
|
Guest38 joined #minetest-dev |
| 21:49 |
|
diceLibrarian joined #minetest-dev |
| 22:33 |
|
panwolfram joined #minetest-dev |
| 22:34 |
|
jonadab joined #minetest-dev |
| 22:35 |
|
tekakutli joined #minetest-dev |
| 23:28 |
|
tekakutli joined #minetest-dev |
