Bawnorton / BetterTrims

Add new armour trims and give each trim a unique feature.
MIT License
12 stars 3 forks source link

[Bug Report] <Iron armor trim effect not working> #48

Open Vago76 opened 5 months ago

Vago76 commented 5 months ago

Expected Behaviour Iron trim effect should increase mining speed

Actual Behaviour The effect from the iron armor trims do nothing. I have tested multiple other effects and they all work fine. It is just the iron trim in particular that has no effect.

Steps to Reproduce

What version of the mod are you using 2.2.3

Video of other effect working

https://github.com/Bawnorton/BetterTrims/assets/165547645/72e2108a-bb70-46b5-bd44-03daabb28c7a

Video of iron effect not working

https://github.com/Bawnorton/BetterTrims/assets/165547645/ecac60e0-0a3c-41a1-91d9-1e9a0b5d09ed

Helpful Files My mod lists image image

image image

Bawnorton commented 4 months ago

I can't replicate this behaviour

NolanHewitt commented 4 months ago

It works for me, but I did have to significantly increase the bonus for it to allow instant mining deepslate with haste 2 on an efficiency 5 netherite pick axe. I had to increase it by a factor for 4x infact. I think something (another mod maybe?) messes with this mods values because it seems lots of people have different experiences with trim effects.

Bawnorton commented 4 months ago

It works for me, but I did have to significantly increase the bonus for it to allow instant mining deepslate with haste 2 on an efficiency 5 netherite pick axe. I had to increase it by a factor for 4x infact. I think something (another mod maybe?) messes with this mods values because it seems lots of people have different experiences with trim effects.

This is likely the case for most trim effects that do not behave as expected, the approach I have to applying these effects is done as compatibly as possible. For example I accept the mining speed value of a tool right before it mines a block, check the wearers armour and apply a % increase to that value depending on the config and the number of pieces. I do this in a way which allows other mods to also modify this without crashing.

To put it simply, internally it looks something like this

mine block with speed myModifier(miningSpeed)

function myModifier(originalValue) => 2 * originalValue

but other mods may come around and either completely remove the original target, ie miningSpeed is gone and replaced with something else, or stick their own modifiers on it which make the behaviour unpredictable:

mine block with speed anotherModsModifier(myModifier(anotherModsModifier(miningSpeed)))

Knowing which mod does what and where is an impossible task for me to undertake as there are too many combinations and I don't have days to give towards this. My only recommendation is to tweak the config until you get something that works. If you find that some behaviour is not working as expected and you are only using better trims, then that's something I can look into but otherwise it's just a game of whack-a-thousand-moles

Vago76 commented 4 months ago

It works for me, but I did have to significantly increase the bonus for it to allow instant mining deepslate with haste 2 on an efficiency 5 netherite pick axe. I had to increase it by a factor for 4x infact. I think something (another mod maybe?) messes with this mods values because it seems lots of people have different experiences with trim effects.

This is likely the case for most trim effects that do not behave as expected, the approach I have to applying these effects is done as compatibly as possible. For example I accept the mining speed value of a tool right before it mines a block, check the wearers armour and apply a % increase to that value depending on the config and the number of pieces. I do this in a way which allows other mods to also modify this without crashing.

To put it simply, internally it looks something like this

mine block with speed myModifier(miningSpeed)

function myModifier(originalValue) => 2 * originalValue

but other mods may come around and either completely remove the original target, ie miningSpeed is gone and replaced with something else, or stick their own modifiers on it which make the behaviour unpredictable:

mine block with speed anotherModsModifier(myModifier(anotherModsModifier(miningSpeed)))

Knowing which mod does what and where is an impossible task for me to undertake as there are too many combinations and I don't have days to give towards this. My only recommendation is to tweak the config until you get something that works. If you find that some behaviour is not working as expected and you are only using better trims, then that's something I can look into but otherwise it's just a game of whack-a-thousand-moles

I just did a test with only 4 mods, better trims, fabric api, mod menu, yet another config mod, and I had the same results. For the test I had haste 2, fully trimmed armor set with iron for the speed boost with eff 5 netherite pick. When the setting was on .50 it did not insta mine, but putting the value to anything >= 85 let me insta mine the deepslate. Again this is with just the dependent mods needed using fabric and better trims, I am playing on 1.20.1, I'm going to do further testing to see if any other mods I was using conflicts with the value you were talking about.

Vago76 commented 4 months ago

After a little bit of testing it seems the Inventorio mod messes up the value somehow, regardless of how much I change the value of increased mining speed it does not affect my mining speed at all. Just going to have to play without inventorio.

TheBlackWisp commented 4 months ago

It would be very helpful and we would be very grateful if you could try to solve this issue with inventorio, because it's a mod that a lo of people use and many need that mod to play because of minecraft's inventory problem. I understand that it's impossible to make this compatible with all the mods, so this is just a suggestion and I would appreciate if you could respond to this message saying if you are planning in fixing it or no. Thanks for your time

Bawnorton commented 4 months ago

I might look into it over the summer when I have more time. The mod is closed source, the only public source is for their api, so I'd have to decompile it and look for what they mess with and make a compat for that.

TheBlackWisp commented 4 months ago

Okay, thank you very much

peow commented 4 months ago

It would be very helpful and we would be very grateful if you could try to solve this issue with inventorio, because it's a mod that a lo of people use and many need that mod to play because of minecraft's inventory problem. I understand that it's impossible to make this compatible with all the mods, so this is just a suggestion and I would appreciate if you could respond to this message saying if you are planning in fixing it or no. Thanks for your time

try to open a issue on Inventorio github https://github.com/RubixDev/Inventorio/issues saying about the incompatibility and if they have any idea what the BetterTrims dev could do to fix this

limemango3 commented 2 months ago

I, an insane person, checked every mod to see wich one causes the problem, cuz I have similar mods as you, it is inventorio that is causing the problem

Feuerball500 commented 1 month ago

Hi, I found a configuration of versions where both mods work for me: BetterTrims Version 1.0.2 Inventorio Version 1.9.2 Minecraft Version 1.20.1 I do not have a clue whats different maybe this can help with the fixing of the problem.