Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thorns enchantment #553

Merged
merged 23 commits into from
Jul 16, 2022
Merged

Thorns enchantment #553

merged 23 commits into from
Jul 16, 2022

Conversation

HashimTheArab
Copy link
Contributor

No description provided.

@HashimTheArab
Copy link
Contributor Author

ill test this in like 15 hours, cant rn

@JustTalDevelops JustTalDevelops added the feature New feature or request label Jul 14, 2022
@DaPigGuy DaPigGuy mentioned this pull request Jul 14, 2022
89 tasks
@Sandertv
Copy link
Member

Would like to have someone with a more thorough understanding of thorns to have a look at the logic.

server/player/player.go Outdated Show resolved Hide resolved
server/player/player.go Outdated Show resolved Hide resolved
HashimTheArab and others added 2 commits July 15, 2022 04:14
Co-authored-by: DaPigGuy <mcpepig123@gmail.com>
Co-authored-by: DaPigGuy <mcpepig123@gmail.com>
Copy link
Member

@DaPigGuy DaPigGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple worn armor items with the Thorns enchantment do stack. Each piece confers an independent chance to deal damage to the attacker as described above. The total amount of damage that can be dealt this way is capped at 4

Copy link
Member

@DaPigGuy DaPigGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation damages every armor piece that procs Thorns. The current implementation does not account for the Unbreaking enchantment

If multiple armor pieces are enchanted with Thorns, the durability penalty is applied to one piece chosen at random, regardless of what level of Thorns that piece has. If the item is also enchanted with Unbreaking, the damage penalty has a chance of being ignored.

Copy link
Member

@DaPigGuy DaPigGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thornsDamage variable also seems redundant

server/player/player.go Outdated Show resolved Hide resolved
@HashimTheArab
Copy link
Contributor Author

The current implementation damages every armor piece that procs Thorns. The current implementation does not account for the Unbreaking enchantment

If multiple armor pieces are enchanted with Thorns, the durability penalty is applied to one piece chosen at random, regardless of what level of Thorns that piece has. If the item is also enchanted with Unbreaking, the damage penalty has a chance of being ignored.

Looking at the java source code, I'm not seeing any extra logic for unbreaking

ThornsEnchantment.java

public void doPostHurt(LivingEntity wearer, Entity attacker, int p_45217_) {
      RandomSource randomsource = wearer.getRandom();
      Map.Entry<EquipmentSlot, ItemStack> entry = EnchantmentHelper.getRandomItemWith(Enchantments.THORNS, wearer);
      if (shouldHit(p_45217_, randomsource)) {
         if (attacker != null) {
            attacker.hurt(DamageSource.thorns(wearer), (float)getDamage(p_45217_, randomsource));
         }

         if (entry != null) {
            entry.getValue().hurtAndBreak(2, wearer, (p_45208_) -> {
               p_45208_.broadcastBreakEvent(entry.getKey());
            });
         }
      }
}

ItemStack.java

public <T extends LivingEntity> void hurtAndBreak(int damage, T wearer, Consumer<T> p_41625_) {
      if (!wearer.level.isClientSide && (!(wearer instanceof Player) || !((Player)wearer).getAbilities().instabuild)) {
         if (this.isDamageableItem()) {
            if (this.hurt(damage, wearer.getRandom(), wearer instanceof ServerPlayer ? (ServerPlayer)wearer : null)) {
               p_41625_.accept(wearer);
               Item item = this.getItem();
               this.shrink(1);
               if (wearer instanceof Player) {
                  ((Player)wearer).awardStat(Stats.ITEM_BROKEN.get(item));
               }

               this.setDamageValue(0);
            }

         }
      }
}

public boolean isDamageableItem() {
      if (!this.emptyCacheFlag && this.getItem().getMaxDamage() > 0) {
         CompoundTag compoundtag = this.getTag();
         return compoundtag == null || !compoundtag.getBoolean("Unbreakable");
      } else {
         return false;
      }
}

public boolean hurt(int damage, RandomSource wearer, @Nullable ServerPlayer p_220160_) {
      if (!this.isDamageableItem()) {
         return false;
      } else {
         if (damage > 0) {
            int i = EnchantmentHelper.getItemEnchantmentLevel(Enchantments.UNBREAKING, this);
            int j = 0;

            for(int k = 0; i > 0 && k < damage; ++k) {
               if (DigDurabilityEnchantment.shouldIgnoreDurabilityDrop(this, i, wearer)) {
                  ++j;
               }
            }

            damage -= j;
            if (damage <= 0) {
               return false;
            }
         }

         if (p_220160_ != null && damage != 0) {
            CriteriaTriggers.ITEM_DURABILITY_CHANGED.trigger(p_220160_, this, this.getDamageValue() + damage);
         }

         int l = this.getDamageValue() + damage;
         this.setDamageValue(l);
         return l >= this.getMaxDamage();
      }
}

The stuff in hurt just looks like the general unbreaking code which in dragonfly is in player.damageItem

server/player/player.go Outdated Show resolved Hide resolved
Copy link
Member

@Sandertv Sandertv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me too.

@Sandertv Sandertv merged commit 8e87a7f into df-mc:master Jul 16, 2022
@HashimTheArab HashimTheArab deleted the enchantment/thorns branch August 19, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants