ArkoSammy12 / creeper-healing

A server and client side, customizable Fabric mod to automatically and naturally heal Creeper explosions (and other kinds of explosions!).
GNU Lesser General Public License v2.1
2 stars 1 forks source link

Enhancements & Bugs <3 #6

Closed BioTechproject27 closed 1 year ago

BioTechproject27 commented 1 year ago
ArkoSammy12 commented 1 year ago

I am testing with version 0.1.7 + 1.20.1+ and drop_items_on_explosions seems to work for all explosion sources. Perhaps you can tell me your settings to see if I can reproduce the bug?

BioTechproject27 commented 1 year ago

Ah, my bad, that was very poorly formulated by me. I tried to formulate both bugs into one, that didn't quite work out and forgot to delete that part, sorry. You can disregard the "and only works on creeper explosions." part

SirFireNewt commented 1 year ago

Hey so I am using creeper-healing-0.1.7-1.20.1+.jar and it seems the drop_items_on_creeper_explosions toggle disables all explosions dropping blocks, do you have plans to separate out creepers and TNT etc I want to disable the blocks dropping from creepers to prevent dupes but want TNT to still drop blocks as normal

ArkoSammy12 commented 1 year ago

I have thought about separating the setting into each of the explosion sources, but I can't think of a way to implement these extra settings without polluting the preferences config category, both in the commands and in the config file itself. I would have a extra "drop_items_on_x_explosions" which would all be contained in the preferences category.

Any suggestions ares welcome.

SirFireNewt commented 1 year ago

Perhaps something similar to your explosion_source would work eg /creeper-healing explosion_drops drop_on_X_explosions true and do the same for the config file

[drop_from_explosions]
    drop_on_X_explosions = true

The preference setting could be changed to a global toggle perhaps

BioTechproject27 commented 1 year ago

Hey so I am using creeper-healing-0.1.7-1.20.1+.jar and it seems the drop_items_on_creeper_explosions toggle disables all explosions dropping blocks, do you have plans to separate out creepers and TNT etc I want to disable the blocks dropping from creepers to prevent dupes but want TNT to still drop blocks as normal

You can kind of do it in vanilla using the mobExplosionDropDecay gamerule, which will disable drops from mob explosions but not tnt afaik.

I have thought about separating the setting into each of the explosion sources, but I can't think of a way to implement these extra settings without polluting the preferences config category, both in the commands and in the config file itself. I would have a extra "drop_items_on_x_explosions" which would all be contained in the preferences category.

Any suggestions ares welcome.

How about putting that into explosion_sources? so you have heal_creeper_explosions heal_ghast_explosions heal_wither_explosions heal_tnt_explosions heal_tnt_minecart_explosions drop_creeper_explosions drop_ghast_explosions drop_wither_explosions drop_tnt_explosions drop_tnt_minecart_explosions

And perhaps adding other explosions like end_crystal, respawnBlocks (beds, respawnanchors).
ArkoSammy12 commented 1 year ago

I will probably go with @SirFireNewt's suggestion. As for end crystals, beds and respawn anchors, from what I can tell right now, whenever explosions are created from any of those blocks, the explosion doesn't really differentiate from them, it just treats them all as being caused by a block. On the worst case scenario, there would be a combined toggle for the 3 of them.

BioTechproject27 commented 1 year ago

On the worst case scenario, there would be a combined toggle for the 3 of them.

I'd say that's good enough for now and if a way is found to seperate them it can be added in a future update?

ArkoSammy12 commented 1 year ago

Yep. I'll look into it. Thanks for the suggestions by the way, really appreciate it xd. Hardest thing here is gonna be this

Blocks that need support (e.g. rails, lanterns, carpets, etc) get destroyed (and sometimes drop as items) and get replaced even with drop_items_on_creeper_explosions set to false, causing them to be basically duped.

BioTechproject27 commented 1 year ago

Yeah I figured. I guess it has something to do with how when the supporting block breaks and the mod saves the blocks it also saves the to be supported block, eventhough it was not destroyed by the explosion itself?

(I first noticed that with the wither shooting skulls, causing a flower to dupe, which is where my wrong assumption about the drop_items_on_creeper_explosions rule came from) (Also there are temporary workarounds for now I think, like replacing them with air in the replace settings [I'm pretty sure that would work]. Although it's not really that gamebreaking anyway.)

Anyway, it is a great mod and we really appreciate it xD <3

ArkoSammy12 commented 1 year ago

The problem has to do with the fact that the reason the torch drops and item is because its supporting block was destroyed. Even if I prevent the internal explosion mechanism from dropping items which were directly affected by the explosion, I still can't prevent the neighbor update that the torch receives which causes it to be broken and its item dropped.

This is also related to why sometimes the mod will not heal stuff like doors or torches. It depends on whether the particular block which needs support is broken by the explosion itself, or by its supporting block being destroyed.

I'll have to find a way to detect this neighbor update and prevent whatever the game does from dropping its item when dropped.

ArkoSammy12 commented 1 year ago

Do you guys think blocks should be healed where there is currently fire always, or should it be a toggle as well? Also, should explosions heal fire at all, or should that be left to be configurable in the replace map like how it currently is?

ArkoSammy12 commented 1 year ago

I ended up creating a combined toggle for allowing the healing and the dropping of items for beds and respawn anchors, and another one for end crystals :). I will now have to see if I can do something about bug 2.

ArkoSammy12 commented 1 year ago

@BioTechproject27 @SirFireNewt Alright, I've finished addressing this issue. If you would like, you can now download the current repo and build the mod to help me test out and find any overlooked bugs 👍.

BioTechproject27 commented 1 year ago

Do you guys think blocks should be healed where there is currently fire always, or should it be a toggle as well? Also, should explosions heal fire at all, or should that be left to be configurable in the replace map like how it currently is?

Honestly would probably just let it be configurable in the replace map, perhaps add it to the replace map as default (like, the same way the diamond block is replaced by stone by default)?

Do you guys think blocks should be healed where there is currently fire always

I'm not quite sure I understand, do you mean you wanna add a config that heals blocks that burned/got replaced with fire (like from fireball or respawnBlock explosions)? I'd think that's a good idea.

If you would like, you can now download the current repo and build the mod to help me test out and find any overlooked bugs

how do I do that xd

ArkoSammy12 commented 1 year ago

Honestly would probably just let it be configurable in the replace map, perhaps add it to the replace map as default (like, the same way the diamond block is replaced by stone by default)?

I ended up hardcoding it. I really doubt there is a situation in which a player would want to have a fire block healed.

I'm not quite sure I understand, do you mean you wanna add a config that heals blocks that burned/got replaced with fire (like from fireball or respawnBlock explosions)? I'd think that's a good idea.

What I meant was whether a block should be healed in a position that contains fire or soul fire. Right now, the mod will not heal a block if that position happens to contain a fire block. I hardcoded it so that it will always does. It is similar to the "heal_on_flowing_water" setting except for fire blocks, and always true.

how do I do that xd

If you are not familiar, that's fine. I will take care of testing properly over here xd. Still, thank you for your suggestions.

BioTechproject27 commented 12 months ago

I ended up hardcoding it. I really doubt there is a situation in which a player would want to have a fire block healed.

I'd put it in the replace map, just in case anyone ever wants to heal fire. Unless ofc that introduces some other funky bugs.

What I meant was whether a block should be healed in a position that contains fire or soul fire. Right now, the mod will not heal a block if that position happens to contain a fire block. I hardcoded it so that it will always does. It is similar to the "heal_on_flowing_water" setting except for fire blocks, and always true.

Ah I see. Yeah, maybe add a setting where it heals on blocks that can be replaced by rightclicking (e.g. grass, fern, fires, single snowlayers, etc) I think especially the snowlayer stuff would mess you up when it snows after an explosion now that I think about it

BioTechproject27 commented 5 months ago

Hey, here to bother you again xd so this is regarding your v1.0.0 release [writing it here in case those aren't bugs]:

[From changelogs:]

[...] because shulker boxes always drop their item with the inventory contained within it, making the restoration of Shulker boxes unnecessary.

[My last msg here:]

Yeah, maybe add a setting where it heals on blocks that can be replaced by rightclicking (e.g. grass, fern, fires, single snowlayers, etc) [...]

ArkoSammy12 commented 5 months ago

is force_blocks_to_heal [accessible through /creeper-healing preferences force_blocks_to_heal] the same as force_blocks_with_nbt_to_always_heal in the config file? Functionally it seems that force_blocks_to_heal set to true does seem to do what force_blocks_with_nbt_to_always_heal promises, there just seems to be some naming inconsistency?

Yeah it was an unfortunate oversight when I coded the command to change the setting via commands. It's supposed to be named as it is in the config file. It's already fixed in the latest commit, but for some reason I don't feel good about releasing an update with only one fix xd.

Regarding shulkers, honestly they are just pretty finicky to deal with given that they are the only block that retain their inventory when mined or exploded, as opposed to all other container blocks. So I chose to instead not deal with them xd.

Also, regarding this, imo there should be a seperate whitelist for this (with vanilla blocks added), so mods that add their own blocks or change the behavior of existing ones can also easily be included [which will hopefully also reduce any future bug reports with incompatabilities regarding this]

I don't quite understand what you mean by this xd.

BioTechproject27 commented 5 months ago

but for some reason I don't feel good about releasing an update with only one fix xd.

No, absolutely fair xd Just wanted to make sure tho.

So I chose to instead not deal with them xd.

Yeah, I understand. Minecraft does make things unnecessarily hard.. But I imagine restore_block_nbt = true and drop_items_on_*_explosions = false should be easy enough since no items are supposed to be spawned?

I don't quite understand what you mean by this xd.

Well, blocks like minecraft:fire, minecraft:snow [which has layers], minecraft:grass are all treated like air. In the sense that they simply always get replaced with a regenerating block.

It should have its own whitelist of blocks that are replaced with the block that attempts to regenerate

[The full list of replaceable blocks is minecraft:crimson_roots, minecraft:dead_bush, minecraft:fern, minecraft:fire, minecraft:glow_lichen, minecraft:grass, minecraft:hanging_roots, minecraft:large_fern, minecraft:light, minecraft:nether_sprouts, minecraft:soul_fire, minecraft:tall_grass, minecraft:vine, minecraft:warped_roots

excluding fluids and variants, as well as air variants and the strcuture void]