Fix damage desyncing entity from chest node, making it invisible #3385

emptyshore · 2023-02-01T06:24:08+01:00

emptyshore commented

2023-02-01 06:24:08 +01:00

Each chest node has an associated entity, which presumably is used for
animation. This makes chests distinctly different from e.g. barrels.

This special behaviour means chest entity (which is NOT the chest node)
can desync from node. This occurs when entity is damaged and destroyed
for example by lightning or anvil, which makes the chest disappear
visually (with the node still persisting).

This patch makes chest entity ignore the damage.

Testing

Method 1, lightning:

install a lightning rod less than 5 blocks from a standard chest
/weather thunder
wait for two strikes (first reduces chest entity HP to 5, the second one destroys it)
chest disappears (but you can still interact with the chest - when you open it, it reappears).

Method 2, anvil:

drop an anvil on a chest from high up
once anvil hits the chest, chest disappears.

Each chest node has an associated entity, which presumably is used for animation. This makes chests distinctly different from e.g. barrels. This special behaviour means chest entity (which is NOT the chest node) can desync from node. This occurs when entity is damaged and destroyed for example by lightning or anvil, which makes the chest disappear visually (with the node still persisting). This patch makes chest entity ignore the damage. ### Testing Method 1, lightning: * install a lightning rod less than 5 blocks from a standard chest * /weather thunder * wait for two strikes (first reduces chest entity HP to 5, the second one destroys it) * chest disappears (but you can still interact with the chest - when you open it, it reappears). Method 2, anvil: * drop an anvil on a chest from high up * once anvil hits the chest, chest disappears.

invisible_chest.gif

2.0 MiB

anvil.gif

3.7 MiB

emptyshore force-pushed fix-lightning-chests from 6ef34f1c7d to cb549473d0

2023-02-01 06:29:55 +01:00

Compare

emptyshore force-pushed fix-lightning-chests from cb549473d0 to 765b4b8fbc

2023-02-01 06:31:15 +01:00

Compare

emptyshore commented

2023-02-01 06:32:57 +01:00

Checked TNT still explodes the chest and makes it drop items.

Michieal commented

2023-02-01 06:55:07 +01:00

chest texture disappears (but you can still interact with the chest - when you open it, texture reappears).

it destroys the entity, and that causes it to disappear. it's not textures. And, when you open it, it recreates the entity. At least, that's how I understand it. @MrRar would know more on it.

> * chest texture disappears (but you can still interact with the chest - when you open it, texture reappears). it destroys the entity, and that causes it to disappear. it's not textures. And, when you open it, it recreates the entity. At least, that's how I understand it. @MrRar would know more on it.

MrRar commented

2023-02-01 16:12:01 +01:00

chest texture disappears (but you can still interact with the chest - when you open it, texture reappears).

it destroys the entity, and that causes it to disappear. it's not textures. And, when you open it, it recreates the entity. At least, that's how I understand it. @MrRar would know more on it.

What you said sounds right to me.

I wonder why the damage groups are being ignored. That does not seem right. I bet this affects more then chests.

> > * chest texture disappears (but you can still interact with the chest - when you open it, texture reappears). > > it destroys the entity, and that causes it to disappear. it's not textures. And, when you open it, it recreates the entity. At least, that's how I understand it. @MrRar would know more on it. What you said sounds right to me. I wonder why the damage groups are being ignored. That does not seem right. I bet this affects more then chests.

❤️ 1

emptyshore commented

2023-02-02 00:29:39 +01:00

@MrRar @Michieal thanks for looking, will have a look at the damage groups!

Edit: as far as I can see, damage groups don't apply here, they are a higher-level abstraction. They apply to tools, players and mobs and Chest entity isn't one of these (neither is lightning). Kinda similar example is the berry bush which deals dmg directly, just as the lightning:

mcl_util.deal_damage(obj, 0.5, {type = "sweet_berry"})

mcl_util.deal_damage is really rudimentary and low level - it seems to largely just decrease HP, but makes a special case for mobs (n/a here). It also checks for handler on entity, but that isn't used anywhere in the codebase (unless I failed search).

My possibly flawed reasoning behind the change is that the chest entity does not represent the chest (node does), so we really shouldn't let it take damage like this at all. Therefore using a NOP hook seems "correct", but I'm happy to redo the PR :)

@MrRar @Michieal thanks for looking, will have a look at the damage groups! Edit: as far as I can see, damage groups don't apply here, they are a higher-level abstraction. They apply to tools, players and mobs and Chest entity isn't one of these (neither is lightning). Kinda similar example is the berry bush which [deals dmg directly](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/ITEMS/mcl_farming/sweet_berry.lua#L112), just as the lightning: ``` mcl_util.deal_damage(obj, 0.5, {type = "sweet_berry"}) ``` [mcl_util.deal_damage](https://git.minetest.land/MineClone2/MineClone2/src/branch/master/mods/CORE/mcl_util/init.lua#L542) is really rudimentary and low level - it seems to largely just decrease HP, but makes a special case for mobs (n/a here). It also checks for handler on entity, but that isn't used anywhere in the codebase (unless I failed search). My possibly flawed reasoning behind the change is that the chest entity does not represent the chest (node does), so we really shouldn't let it take damage like this at all. Therefore using a NOP hook seems "correct", but I'm happy to redo the PR :)

👍 1

emptyshore force-pushed fix-lightning-chests from 765b4b8fbc to 6eaa2ac031

2023-02-02 00:52:19 +01:00

Compare

emptyshore commented

2023-02-02 00:58:03 +01:00

Oh also, anvil does the same (anything doing mcl_util.deal_damage on a non-player, non-mob entity will cause this). Also bypasses damage groups.

Added animated gif to MineClone2/MineClone2#3389

Oh also, anvil does the same (anything doing `mcl_util.deal_damage` on a non-player, non-mob entity will cause this). Also bypasses damage groups. Added animated gif to https://git.minetest.land/MineClone2/MineClone2/issues/3389

emptyshore referenced this pull request

2023-02-02 01:02:39 +01:00

Chests become invisible after taking non-explosive damage #3389

Michieal commented

2023-02-02 01:13:13 +01:00

This PR fixes #3389

Michieal commented

2023-02-02 01:16:59 +01:00

Added animated gif to MineClone2/MineClone2#3389

You didn't need to make an Issue for the Pull Request. In fact, if you don't know how to set it so that the PR closes the issue, it's really an annoying thing, because then we have to find the orphaned issue.
You can discuss things in the PR, including other "errors" or "bugs found". And, it's best to keep all of the information together in one spot, so that it's at hand for reference.

in fact, you should probably go close the issue, so that everything is here...

> Added animated gif to https://git.minetest.land/MineClone2/MineClone2/issues/3389 You didn't need to make an Issue for the Pull Request. In fact, if you don't know how to set it so that the PR closes the issue, it's really an annoying thing, because then we have to find the orphaned issue. You can discuss things in the PR, including other "errors" or "bugs found". And, it's best to keep all of the information together in one spot, so that it's at hand for reference. in fact, you should probably go close the issue, so that everything is here...

👍 1

emptyshore changed title from ~~Fix lightning desynchronising entity from chest node~~ to Fix damage desynchronising entity from chest node

2023-02-02 01:34:43 +01:00

emptyshore commented

2023-02-02 01:35:50 +01:00

And, it's best to keep all of the information together in one spot, so that it's at hand for reference.

PR as good as an issue, point taken, will close the issue.

> And, it's best to keep all of the information together in one spot, so that it's at hand for reference. PR as good as an issue, point taken, will close the issue.

❤️ 1

emptyshore force-pushed fix-lightning-chests from 6eaa2ac031 to 805e6bf674

2023-02-02 01:43:27 +01:00

Compare

emptyshore changed title from ~~Fix damage desynchronising entity from chest node~~ to Fix damage desyncing entity from chest node, making it invisible

2023-02-02 01:44:41 +01:00

Michieal commented

2023-02-02 05:02:24 +01:00

You're probably mostly done with this, but...

In the code for Item Frames and Signs (both) I created a "clearobjects fix" (the link is here) that might help.

Both are nodes with entities attached... just like chests are. Most... modules that create a node with an entity attached have some way of refreshing the entity (as it is required on load / start) in order for it to display properly.

It looks like the chests was opening them. For signs, it was punching them. Sorry that I did not remember this sooner. If you look in the on_rightclick() you should see a function called "update"-something. That would be the thing to put into the chest entity's on_damage callback. The idea being that every time it takes damage, it resets itself via the update function.

You're probably mostly done with this, but... In the code for Item Frames and Signs (both) I created a "clearobjects fix" (the link is [here](https://git.minetest.land/MineClone2/MineClone2/pulls/2919)) that might help. Both are nodes with entities attached... just like chests are. Most... modules that create a node with an entity attached have some way of refreshing the entity (as it is required on load / start) in order for it to display properly. It looks like the chests was opening them. For signs, it was punching them. Sorry that I did not remember this sooner. If you look in the on_rightclick() you should see a function called "update"-something. That would be the thing to put into the chest entity's on_damage callback. The idea being that every time it takes damage, it resets itself via the update function.

emptyshore commented

2023-02-02 06:24:14 +01:00

The plot thickens :) Turns out signs are also affected - you can wipe out the text with two lightnings (same technique, just plop a rod next to it, two because lightning has 5 dmg, and I think default entity HP is 10). Writing comes back after punching the sign. Anvil destroys the sign though.

Just tested the following in MC:

in MC, lightning strikes do not damage nor destroy chests nor signs
in MC, anvils dropping on chests and signs break down into item form, without affecting chests and signs

Some other notes from the chat:

ender chests, shulker boxes are also affected
/clearobjects causes the same "invisible chest" and "invisible writing" effect
logging out and in fixes the issue
double chests are affected by lightning, but unaffected by anvils (however they may be affected by double anvils falling at exactly the same time!)

There is actually two problems bundled together, I think:

even more edge-cases in node-entity sync in signs, frames and chests
even with these edge cases fixed, node-entities should NOT take damage like normal entities (if we are following MC)

The plot thickens :) Turns out signs are also affected - you can wipe out the text with two lightnings (same technique, just plop a rod next to it, two because lightning has 5 dmg, and I think default entity HP is 10). Writing comes back after punching the sign. Anvil destroys the sign though. Just tested the following in MC: - in MC, lightning strikes do not damage nor destroy chests nor signs - in MC, anvils dropping on chests and signs break down into item form, without affecting chests and signs Some other notes from the chat: - ender chests, shulker boxes are also affected - /clearobjects causes the same "invisible chest" and "invisible writing" effect - logging out and in fixes the issue - double chests are affected by lightning, but unaffected by anvils (however they may be affected by double anvils falling at exactly the same time!) There is actually two problems bundled together, I think: - even more edge-cases in node-entity sync in signs, frames and chests - even with these edge cases fixed, node-entities should NOT take damage like normal entities (if we are following MC)

👍 2

Michieal commented

2023-02-02 07:36:39 +01:00

This:

logging out and in fixes the issue.

Is because on load (the LBM calls this) the entities are recreated but their respective Update functions. (I know this, because I rewrote the sign and the item frames.)

All chests will be affected by this, as well as all other node/entitiy combinations. (Chest, signs, item frames are an example of what I mean by this.)

The group immortal completely disables normal damage.

^ This could be applied to the entity...

on_death(self, killer)

This is the callback when the entity "dies"... placing the update function in here would cause it to immediately be "resurrected"... may or may not be called when /clearobjects is called.

This: > logging out and in fixes the issue. Is because on load (the LBM calls this) the entities are recreated but their respective Update functions. (I know this, because I rewrote the sign and the item frames.) All chests will be affected by this, as well as all other node/entitiy combinations. (Chest, signs, item frames are an example of what I mean by this.) > The group `immortal` completely disables normal damage. ^ This could be applied to the entity... > `on_death(self, killer)` This is the callback when the entity "dies"... placing the update function in here would cause it to immediately be "resurrected"... may or may not be called when `/clearobjects` is called.

kneekoo commented

2023-02-02 14:24:20 +01:00

The group immortal completely disables normal damage.

^ This could be applied to the entity...

We could, but would explosions still be able to affect chests (as they should) if they're "immortal"?

> > The group `immortal` completely disables normal damage. > > ^ This could be applied to the entity... We could, but would explosions still be able to affect chests (as they should) if they're "immortal"?

MrRar commented

2023-02-02 15:08:55 +01:00

The group immortal completely disables normal damage.

^ This could be applied to the entity...

We could, but would explosions still be able to affect chests (as they should) if they're "immortal"?

Yes. Explosions interact with the chest node. When the chest node is removed so will the chest entity.

> > > The group `immortal` completely disables normal damage. > > > > ^ This could be applied to the entity... > > We could, but would explosions still be able to affect chests (as they should) if they're "immortal"? Yes. Explosions interact with the chest node. When the chest node is removed so will the chest entity.

👍 2

MrRar commented

2023-02-02 15:13:45 +01:00

@emptyshore

Please make mcl_util.deal_damage() do no damage to ObjectRefs that have the immortal damage group. All Minetest mods assume that ObjectRefs with the immortal damage group can't be removed.

As a bonus you could remove the no-op deal_damage callbacks from node entites.

Thanks for working on this!

@emptyshore Please make mcl_util.deal_damage() do no damage to ObjectRefs that have the immortal damage group. All Minetest mods assume that ObjectRefs with the immortal damage group can't be removed. As a bonus you could remove the no-op deal_damage callbacks from node entites. Thanks for working on this!

👍 2

emptyshore force-pushed fix-lightning-chests from 805e6bf674 to f0d0880e68

2023-02-03 00:16:23 +01:00

Compare

emptyshore force-pushed fix-lightning-chests from f0d0880e68 to 0b77396106

2023-02-03 00:16:44 +01:00

Compare

emptyshore commented

2023-02-03 00:28:52 +01:00

Added, but one thing to decide is if we should still call the deal_damage handler even if the entity is set to immortal?

It's a handler that was added in ce0148d9a8 and I don't think it has ever been used inside the MCL2 codebase, but it could be used by mods.

I've put the "simple" case in, where "immortal" prevents the handler from firing, but I think this could perhaps break some downstream mods that rely on deal_damage? Happy to rewrite so that it checks for the handler's existence before checking for immortality.

Other things: chests, signs and frames already configure the entities to be immortal, so no need for changes there! TNT has it's own separate routine, so still explodes the node-entities fine. Anything else to check for?

Added, but one thing to decide is if we should still call the `deal_damage` handler even if the entity is set to immortal? It's a handler that was added in ce0148d9a86463b2a34bce0a145f7fc324015f88 and I don't think it has ever been used inside the MCL2 codebase, but it could be used by mods. I've put the "simple" case in, where "immortal" prevents the handler from firing, but I think this could perhaps break some downstream mods that rely on deal_damage? Happy to rewrite so that it checks for the handler's existence before checking for immortality. Other things: chests, signs and frames already configure the entities to be immortal, so no need for changes there! TNT has it's own separate routine, so still explodes the node-entities fine. Anything else to check for?

❤️ 1

emptyshore force-pushed fix-lightning-chests from 0b77396106 to badc247678

2023-02-03 00:31:01 +01:00

Compare

emptyshore commented

2023-02-03 02:09:44 +01:00

Turns out all mobs are immortal :-) In mcl_mobs/api.txt:

Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually.

So as a result of this change, fire, anvils, etc no longer damages mobs... uh.

Turns out all mobs are immortal :-) In mcl_mobs/api.txt: > Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually. So as a result of this change, fire, anvils, etc no longer damages mobs... uh.

👀 1

Michieal commented

2023-02-03 05:38:18 +01:00

Turns out all mobs are immortal :-) In mcl_mobs/api.txt:

Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually.

So as a result of this change, fire, anvils, etc no longer damages mobs... uh.

Whoops! I was going to say, I thought that I remembered Iron Golems having "Immortal=1" on them, too. I guess that they do.

Maybe make another armor group? like non_mob_entity?

> Turns out all mobs are immortal :-) In mcl_mobs/api.txt: > > > Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually. > > So as a result of this change, fire, anvils, etc no longer damages mobs... uh. Whoops! I was going to say, I thought that I remembered Iron Golems having "Immortal=1" on them, too. I guess that they do. Maybe make another armor group? like `non_mob_entity`?

MrRar commented

2023-02-03 16:38:34 +01:00

Turns out all mobs are immortal :-) In mcl_mobs/api.txt:

Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually.

So as a result of this change, fire, anvils, etc no longer damages mobs... uh.

I think you should add the immortal armor group check before:
target:set_hp(hp - damage, {_mcl_reason = mcl_reason})

I tested it myself and it seems to work.

> Turns out all mobs are immortal :-) In mcl_mobs/api.txt: > > > Note: The 'immortal=1' armor group will automatically be added since this mod handles health and damage manually. > > So as a result of this change, fire, anvils, etc no longer damages mobs... uh. I think you should add the immortal armor group check before: target:set_hp(hp - damage, {_mcl_reason = mcl_reason}) I tested it myself and it seems to work.

emptyshore commented

2023-02-03 21:49:41 +01:00

Moving it will work, but it feels like a leaky abstraction. If we take a step back, deal_damage and get_hp seem to be intended as simple wrappers to support redirection for the custom mob damage handling (using health instead of core hp). Hijacking it for immortality check might surprise someone down the line :)

What is odd to me is that the entity is destroyed even though its armor group is set to immortal. Shouldn't minetest engine not kill it in that case?

I couldn't find the spot where the entity is killed when HP reaches 0 - do you know where to find it? Is it in MT core cpp files somewhere?

Moving it will work, but it feels like a leaky abstraction. If we take a step back, `deal_damage` and `get_hp` seem to be intended as simple wrappers to support redirection for the custom mob damage handling (using health instead of core hp). Hijacking it for immortality check might surprise someone down the line :) What is odd to me is that the entity is destroyed even though its armor group is set to immortal. Shouldn't minetest engine not kill it in that case? I couldn't find the spot where the entity is killed when HP reaches 0 - do you know where to find it? Is it in MT core cpp files somewhere?

MrRar commented

2023-02-03 22:23:23 +01:00

Moving it will work, but it feels like a leaky abstraction. If we take a step back, deal_damage and get_hp seem to be intended as simple wrappers to support redirection for the custom mob damage handling (using health instead of core hp). Hijacking it for immortality check might surprise someone down the line :)

I'm not sure what you mean. get_hp gets the Minetest entity HP.

What is odd to me is that the entity is destroyed even though its armor group is set to immortal. Shouldn't minetest engine not kill it in that case?

The damage groups are only respected by the default punch handeling and mods that want to respect damage groups.

I couldn't find the spot where the entity is killed when HP reaches 0 - do you know where to find it? Is it in MT core cpp files somewhere?

Minetest automatically removes any entities with HP of 0

I don't know what the purpose of the if hp > 0 then check is.

> Moving it will work, but it feels like a leaky abstraction. If we take a step back, deal_damage and get_hp seem to be intended as simple wrappers to support redirection for the custom mob damage handling (using health instead of core hp). Hijacking it for immortality check might surprise someone down the line :) I'm not sure what you mean. get_hp gets the Minetest entity HP. > What is odd to me is that the entity is destroyed even though its armor group is set to immortal. Shouldn't minetest engine not kill it in that case? The damage groups are only respected by the default punch handeling and mods that want to respect damage groups. >I couldn't find the spot where the entity is killed when HP reaches 0 - do you know where to find it? Is it in MT core cpp files somewhere? Minetest automatically removes any entities with HP of 0 I don't know what the purpose of the `if hp > 0 then` check is.

MrRar commented

2023-02-03 22:47:54 +01:00

The problem is that third party mobs will not be affected by deal_damage() because they are using a custom HP system like MCL2 mobs.

One thing I haven't tested but I bet it would be better than target:set_hp() is target:punch(). I am talking about the code that is commented out in the mob damage area.

The default punch handling respects damage groups including immortal. It can also deal damage to mobs that have a custom HP system.

I don't know why the MCL2 mobs aren't using the punching mechanism.

The problem is that third party mobs will not be affected by deal_damage() because they are using a custom HP system like MCL2 mobs. One thing I haven't tested but I bet it would be better than target:set_hp() is target:punch(). I am talking about the code that is commented out in the mob damage area. The default punch handling respects damage groups including immortal. It can also deal damage to mobs that have a custom HP system. I don't know why the MCL2 mobs aren't using the punching mechanism.

emptyshore force-pushed fix-lightning-chests from badc247678 to 17a91fc381

2023-02-04 20:34:05 +01:00

Compare

emptyshore force-pushed fix-lightning-chests from 17a91fc381 to 8d1a926dc9

2023-02-04 20:37:25 +01:00

Compare

emptyshore commented

2023-02-04 20:50:39 +01:00

Can't figure out either why target:punch code is commented out. Maybe it was causing infinite loops if on_punch was calling deal_damage? After cursory search, I don't see any obvious places where this would happen though.

The damage groups are only respected by the default punch handeling and mods that want to respect damage groups.

Immortal is kinda special, because it's checked for in minetest I think (but not for entities). I think philosophically (lol) minetest should not wipe out immortal entities - if we agree this is the case, then patching it over in deal_damage would be the right thing to do (updated the PR).

I'll try commenting out the punch code and see what happens - but anyway, I don't think that part is relevant to this PR.

Can't figure out either why target:punch code is commented out. Maybe it was causing infinite loops if on_punch was calling deal_damage? After cursory search, I don't see any obvious places where this would happen though. > The damage groups are only respected by the default punch handeling and mods that want to respect damage groups. Immortal is kinda special, because it's checked for in minetest I think (but not for entities). I think philosophically (lol) minetest *should* not wipe out immortal entities - if we agree this is the case, then patching it over in deal_damage would be the right thing to do (updated the PR). I'll try commenting out the punch code and see what happens - but anyway, I don't think that part is relevant to this PR.

emptyshore force-pushed fix-lightning-chests from 8d1a926dc9 to bdc82b76b5

2023-02-08 04:03:59 +01:00

Compare

emptyshore commented

2023-02-08 04:08:15 +01:00

I've tried out the punch code that's commented out in mcl_util.deal_damage, and it seemed to work ok (but unrelated so shouldn't be bundled here). Is anything else I need to do here to make the PR acceptable for fixing disappearing node entities?

I've tried out the punch code that's commented out in `mcl_util.deal_damage`, and it seemed to work ok (but unrelated so shouldn't be bundled here). Is anything else I need to do here to make the PR acceptable for fixing disappearing node entities?

Michieal commented

2023-02-08 04:13:05 +01:00

Well, all that I can say is... things are in there for a reason (or removed for one). That said, the reason could be a work-around for a previous minetest version (that is no longer relevant), because the author didn't know any better, because doing so fixes another problem, or something like "tech-debt"... All of these things exist in the code base. It's our job to find them and fix them so that they work correctly.

In other nodes with attached entities, on_punch() was used to refresh them. Not that any player knows about this. I mean, I didn't until I tracked down what on_punch in signs did, when I made it into an API.

Well, all that I can say is... things are in there for a reason (or removed for one). That said, the reason could be a work-around for a previous minetest version (that is no longer relevant), because the author didn't know any better, because doing so fixes another problem, or something like "tech-debt"... All of these things exist in the code base. It's our job to find them and fix them so that they work correctly. In other nodes with attached entities, on_punch() was used to refresh them. Not that any player knows about this. I mean, I didn't until I tracked down what on_punch in signs did, when I made it into an API.

MrRar commented

2023-02-08 23:30:21 +01:00

I've tried out the punch code that's commented out in mcl_util.deal_damage, and it seemed to work ok (but unrelated so shouldn't be bundled here). Is anything else I need to do here to make the PR acceptable for fixing disappearing node entities?

Sometimes it's better to cure the ultimate problem instead of curing the symptom.

Do what you want to do. I am not the ultimate authority.

> I've tried out the punch code that's commented out in `mcl_util.deal_damage`, and it seemed to work ok (but unrelated so shouldn't be bundled here). Is anything else I need to do here to make the PR acceptable for fixing disappearing node entities? Sometimes it's better to cure the ultimate problem instead of curing the symptom. Do what you want to do. I am not the ultimate authority.

Michieal commented

2023-02-09 01:08:02 +01:00

Sometimes it's better to cure the ultimate problem instead of curing the symptom.

I think that they are confused / lost as to what the root problem is.

Do what you want to do. I am not the ultimate authority.

You are the closest to an authority on chests that we have, as the rest of us haven't touched the code. Well, maybe epCode too; but I don't know how busy they are. If you're asking me about it, I can look and say, "Yep! It's a chest alright!" lmao.

> Sometimes it's better to cure the ultimate problem instead of curing the symptom. I think that they are confused / lost as to what the root problem is. > Do what you want to do. I am not the ultimate authority. You are the closest to an authority on chests that we have, as the rest of us haven't touched the code. Well, maybe epCode too; but I don't know how busy they are. If you're asking *me* about it, I can look and say, "Yep! It's a chest alright!" lmao.

😆 1

emptyshore commented

2023-02-09 23:23:13 +01:00

I think that they are confused / lost as to what the root problem is.

I'd say I understand the problem, but I don't know how you want to progress. I've proposed two solutions so far. Am I looking for a third solution? Do you want me to clean something up? Should I be looking for why minetest core does not respect immortal in this case (I'm not sure if I am able to fix up upstream though)? Should I be looking for why the on_punch is commented out (which is unrelated and won't fix this?)

What's the next step I need to do?

> I think that they are confused / lost as to what the root problem is. I'd say I understand the problem, but I don't know how you want to progress. I've proposed two solutions so far. Am I looking for a third solution? Do you want me to clean something up? Should I be looking for why minetest core does not respect `immortal` in this case (I'm not sure if I am able to fix up upstream though)? Should I be looking for why the on_punch is commented out (which is unrelated and won't fix this?) What's the next step I need to do?

ancientmarinerdev approved these changes 2023-02-10 01:38:14 +01:00

ancientmarinerdev left a comment

I think there is a wider and more complex problem here, and mobs do need plenty of work. I will be focussing on them in future, and will refer back to this stuff so it isn't forgotten, but I don't know the best way yet. I think it's probably a bit tricky for a first issue though.

I appreciate the context and the investigation into this very strange bug. I have tested and it does work. Thanks for picking it up, emptyshore!

I think there is a wider and more complex problem here, and mobs do need plenty of work. I will be focussing on them in future, and will refer back to this stuff so it isn't forgotten, but I don't know the best way yet. I think it's probably a bit tricky for a first issue though. I appreciate the context and the investigation into this very strange bug. I have tested and it does work. Thanks for picking it up, emptyshore!

❤️ 1

ancientmarinerdev added this to the 0.83.0 - Safe and Sound milestone 2023-02-10 01:38:49 +01:00

ancientmarinerdev referenced this issue from a commit

2023-02-10 01:38:56 +01:00

Merge pull request 'Fix damage desyncing entity from chest node, making it invisible' (#3385) from emptyshore/MineClone2:fix-lightning-chests into master

ancientmarinerdev merged commit 620a0af483 into master

2023-02-10 01:38:57 +01:00

emptyshore deleted branch fix-lightning-chests

2023-02-10 08:55:13 +01:00

Sign in to join this conversation.

No reviewers