Maciej916 / Ma-Enchants

Adds various new enchant and curses to the game
GNU Lesser General Public License v3.0
3 stars 8 forks source link

Faster Attacks Causes Negative Attack Speed on ALL Tools #46

Open sheeshryan opened 1 year ago

sheeshryan commented 1 year ago

As the title states. the Faster Attack sword enchantment causes negative attack speed on all tools, even if the enchantment is only on one item.

Seems to attach itself to player data, because all tools are effected, even after removing the enchantment and even the mod.

Causes game to be almost unplayable without the enchantment.

Examples of affected items: [even items from other mods aren't safe] 2022-12-16_01 55 25 2022-12-16_01 55 31 2022-12-16_01 55 33 2022-12-16_01 55 48

![2022-12-16_01 55 41](https://user-images.github 2022-12-16_01 55 45 usercontent.com/120717698/208072968-a7008c96-c63c-436f-8d04-ce70d303cbc2.png)

Averso commented 1 year ago

I'm pretty sure to problem lies in using wrong methods to set/get attack speed value.

When equipping enchanted weapon, attack speed is calculated from defaultValue, and then baseValue is set. When deselecting weapon, calculation is based on current value (getValue()), which apparently does not equal baseValue. I've added logs and there's difference between "baseAttack lvl" which is set as BaseValue and attack speed value get with getValue() method (e.g. baseAttack lvl = 8.0, but ATTACK_SPEED.getValue() gives 5.5999999).

Changing to getting/setting only BaseValue seems to fix the problem.

I'm no Minecraft modder, just stumbled on this problem on my friend's server. So I do not know what is difference between getValue() and getBaseValue() in connection to attack speed attribute.

Other problem which might rise and needs consideration are other mods that might tinker with attack speed.

Below is fixed code (HandlerFasterAttack.java):

        if (lvl > 0) {
            if (enchantsCap.getFasterAttack() == 0) {
                double baseAttack = Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).getBaseValue(); //In my opinion baseAttack should be get from getBaseValue(), not getDefaultValue(), as other mods can tinker with attack speed
                Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).setBaseValue(baseAttack * lvl);
                enchantsCap.setFasterAttack(lvl);
            }
        } else {
            if (enchantsCap.getFasterAttack() != 0) {
                double attackSpeed = Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).getBaseValue(); //Here was getValue(), should be getBaseValue() 
                Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).setBaseValue(attackSpeed / enchantsCap.getFasterAttack());
                player.removeEffect(ModMobEffects.FASTER_ATTACK.get());
                enchantsCap.setFasterAttack(0);
            }
        }
alphalee65 commented 1 year ago

I'm pretty sure to problem lies in using wrong methods to set/get attack speed value.

When equipping enchanted weapon, attack speed is calculated from defaultValue, and then baseValue is set. When deselecting weapon, calculation is based on current value (getValue()), which apparently does not equal baseValue. I've added logs and there's difference between "baseAttack lvl" which is set as BaseValue and attack speed value get with getValue() method (e.g. baseAttack lvl = 8.0, but ATTACK_SPEED.getValue() gives 5.5999999).

Changing to getting/setting only BaseValue seems to fix the problem.

I'm no Minecraft modder, just stumbled on this problem on my friend's server. So I do not know what is difference between getValue() and getBaseValue() in connection to attack speed attribute.

Other problem which might rise and needs consideration are other mods that might tinker with attack speed.

Below is fixed code (HandlerFasterAttack.java):

        if (lvl > 0) {
            if (enchantsCap.getFasterAttack() == 0) {
                double baseAttack = Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).getBaseValue(); //In my opinion baseAttack should be get from getBaseValue(), not getDefaultValue(), as other mods can tinker with attack speed
                Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).setBaseValue(baseAttack * lvl);
                enchantsCap.setFasterAttack(lvl);
            }
        } else {
            if (enchantsCap.getFasterAttack() != 0) {
                double attackSpeed = Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).getBaseValue(); //Here was getValue(), should be getBaseValue() 
                Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED)).setBaseValue(attackSpeed / enchantsCap.getFasterAttack());
                player.removeEffect(ModMobEffects.FASTER_ATTACK.get());
                enchantsCap.setFasterAttack(0);
            }
        }

Where can i add/change this? If it is HandlerFasterAttack.java i dont seem to be able to find it.

Kon1Kobryn commented 10 months ago

The patch below seems to be working, I'm sure there is a better way to do this, unfortunately, I'm not a modder and I don't know forge API. I added attackSpeedPerLevel multiplier because weapon with no enchant gave the same attack speed as level 1 enchant which was weird and higher levels seemed too overpowered.

public class HandlerFasterAttack {
    public static final double attackSpeedPerLevel = 0.375D;

    private static double calcAttackSpeedMod(double enchant_lvl){
        return 1.0D + (enchant_lvl * attackSpeedPerLevel);
    }

    public static void handlerPlayerTick(Player player) {
        IPlayerCapability enchantsCap = PlayerUtil.getAliveEnchantsCapability(player);
        if (enchantsCap == null) return;

        ItemStack usedItem = player.getItemInHand(player.getUsedItemHand());
        int enchant_lvl = usedItem.getEnchantmentLevel(ModEnchantments.FASTER_ATTACK.get());
        int currentFasterAttackBuff_lvl = enchantsCap.getFasterAttack();
        double currentAttackSpeedMod = HandlerFasterAttack.calcAttackSpeedMod(currentFasterAttackBuff_lvl);
        var playerAttackSpeedObj = Objects.requireNonNull(player.getAttribute(Attributes.ATTACK_SPEED));

        if (enchant_lvl > 0) {
            if (currentFasterAttackBuff_lvl != enchant_lvl) {
                double newAttackSpeedMod = HandlerFasterAttack.calcAttackSpeedMod(enchant_lvl);

                if(enchantsCap.getFasterAttack() != 0){
                    playerAttackSpeedObj.setBaseValue(playerAttackSpeedObj.getBaseValue() / currentAttackSpeedMod);
                }

                double playerAttackSpeedBaseValue = playerAttackSpeedObj.getBaseValue();

                playerAttackSpeedObj.setBaseValue(playerAttackSpeedBaseValue * newAttackSpeedMod);
                enchantsCap.setFasterAttack(enchant_lvl);
            }
        } else {
            if (enchantsCap.getFasterAttack() != 0) {
                double playerAttackSpeedBaseValue = playerAttackSpeedObj.getBaseValue();

                playerAttackSpeedObj.setBaseValue(playerAttackSpeedBaseValue / currentAttackSpeedMod);
                player.removeEffect(ModMobEffects.FASTER_ATTACK.get());
                enchantsCap.setFasterAttack(0);
            }
        }
    }
}

I'm posting a link to the compiled version of the mod for those who don't want to build the entire project themselves and trust random people on the internet enough: Google Drive - maenchants-1.19.2-6.0.0_1.jar