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

Implement enchanting system #5953

Merged
merged 97 commits into from
Aug 15, 2023
Merged

Implement enchanting system #5953

merged 97 commits into from
Aug 15, 2023

Conversation

S3v3Nice
Copy link
Contributor

@S3v3Nice S3v3Nice commented Jul 31, 2023

Introduction

This adds the long-awaited functionality of enchanting items via an enchanting table.

Changes

API changes

  • Add to Human: getEnchantmentSeed(), setEnchantmentSeed(int), generateEnchantmentSeed()
  • ItemFlags is now deprecated
  • getPrimaryItemFlags, getSecondaryItemFlags, hasPrimaryItemType, hasSecondaryItemType in Enchantment are now deprecated
  • Add ItemEnchantmentTags
  • Add ItemEnchantmentTagRegistry
  • Add to Item: getEnchantmentTags(), getEnchantability()
  • Add to BlockTypeInfo: getEnchantmentTags() (some block items, such as carved pumpkin and mob heads, can be enchanted (with enchanted books in the anvil when the appropriate functionality is added))
  • Add to Block: getEnchantmentTags()
  • Add to EnchantInventory: getInput(), getLapis(), getOutput(int), getOption(int)
  • Add EnchantmentHelper
  • Add to Enchantment: isCompatibleWith(Enchantment), getMinEnchantingPower(int), getMaxEnchantingPower(int)
  • Add IncompatibleEnchantmentGroups
  • Add IncompatibleEnchantmentRegistry
  • Add AvailableEnchantmentRegistry
  • Add EnchantTransaction
  • Add EnchantedBook item
  • Add ArmorMaterial (to hold the value of enchantability for each armor material)
  • Add VanillaArmorMaterials registry
  • Add to ArmorTypeInfo: getMaterial()
  • Add to Armor: getMaterial()
  • Add to ToolTier: getEnchantability()
  • Add EnchantOption
  • Add to InventoryManager: syncEnchantingTableOptions, getEnchantingTableOptionIndex
  • Add PlayerEnchantOptionsRequestEvent
  • Add PlayerItemEnchantEvent

Behavioural changes

Add enchanting functionality for an enchanting table.

Follow-up

  • Maybe do something with CraftRecipeAutoStackRequestAction and CraftingConsumeInputStackRequestAction (these actions are now used for both crafting and enchanting, and in the future for anvil repair by idea).
  • It would be nice to add the ability to change the enchantment option cost (the amount of XP levels and lapis lazuli spent). The difficulty is that the client is responsible for the amount of lapis lazuli spent. We can control this, but would have to add additional checks when creating the SlotChangeAction and change the amount of output lapis lazuli. Although, on the client side without additional resource packs, the cost of XP levels and lapis lazuli will still remain the same (from 1 to 3), so maybe it's not such a good idea.
  • Since we now have ArmorMaterial, we can add some armor material related values there, such as equipSound, toughness, isDyeable.

Tests

Untitled.1.mp4

src/entity/Human.php Outdated Show resolved Hide resolved
src/item/enchantment/EnchantmentHelper.php Outdated Show resolved Hide resolved
src/item/enchantment/EnchantmentHelper.php Outdated Show resolved Hide resolved
src/item/enchantment/ItemEnchantmentFlags.php Outdated Show resolved Hide resolved
src/player/Player.php Outdated Show resolved Hide resolved
src/inventory/transaction/EnchantTransaction.php Outdated Show resolved Hide resolved
src/entity/Human.php Outdated Show resolved Hide resolved
src/item/enchantment/Enchantment.php Outdated Show resolved Hide resolved
src/item/enchantment/Enchantment.php Outdated Show resolved Hide resolved
src/item/enchantment/Enchantment.php Outdated Show resolved Hide resolved
src/block/inventory/EnchantInventory.php Outdated Show resolved Hide resolved
Copy link
Member

@dktapps dktapps left a comment

Choose a reason for hiding this comment

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

I've only reviewed this at the surface level so far.

src/block/inventory/EnchantInventory.php Show resolved Hide resolved
src/block/inventory/EnchantInventory.php Outdated Show resolved Hide resolved
src/entity/Human.php Outdated Show resolved Hide resolved
src/inventory/transaction/EnchantTransaction.php Outdated Show resolved Hide resolved
src/inventory/transaction/EnchantTransaction.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Outdated Show resolved Hide resolved
src/network/mcpe/handler/ItemStackRequestExecutor.php Outdated Show resolved Hide resolved
src/network/mcpe/handler/ItemStackRequestExecutor.php Outdated Show resolved Hide resolved
@S3v3Nice S3v3Nice requested a review from dktapps August 1, 2023 12:23
@S3v3Nice S3v3Nice requested a review from ipad54 August 1, 2023 12:56
Copy link
Member

@ShockedPlot7560 ShockedPlot7560 left a comment

Choose a reason for hiding this comment

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

I've only had a quick read. The logic seems OK, to be seen more precisely.

src/block/WaterCauldron.php Outdated Show resolved Hide resolved
src/block/Block.php Outdated Show resolved Hide resolved
src/entity/Human.php Outdated Show resolved Hide resolved
src/item/ArmorMaterial.php Outdated Show resolved Hide resolved
src/item/VanillaItems.php Outdated Show resolved Hide resolved
src/network/mcpe/NetworkSession.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Outdated Show resolved Hide resolved
src/item/enchantment/VanillaEnchantments.php Show resolved Hide resolved
src/block/Block.php Outdated Show resolved Hide resolved
src/block/BlockTypeInfo.php Outdated Show resolved Hide resolved
@S3v3Nice S3v3Nice requested a review from dktapps August 15, 2023 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Gameplay Related to Minecraft gameplay experience Type: Enhancement Contributes features or other improvements to PocketMine-MP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants