Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

Enchantment Name Support - EnchantCommand #2009

Merged
merged 4 commits into from
Oct 1, 2016
Merged

Enchantment Name Support - EnchantCommand #2009

merged 4 commits into from
Oct 1, 2016

Conversation

0x15f
Copy link
Contributor

@0x15f 0x15f commented Oct 1, 2016

Description

This PR adds support for Enchantment names when using the enchant command.
Previously when using the enchant command /enchant ImagicalGamer smite 1 the command returned with a Unknown Enchantment message. Now when running the same command checks for enchantment names as well as enchantment ID's.

Reason to modify

Yes this is a good way to fix things.
There is no real issue, this is to help users who don't know every enchantment ID.
I modified an existing function to make it work correctly.

Tests & Reviews

I have tested this code and it works.

  • Enchantment Name Support 1/2
  • Enchantment Name Support 2/2

* Enchantment Name Support 1/2

* Enchantment Name Support 2/2
@dktapps
Copy link
Contributor

dktapps commented Oct 1, 2016

Looks good, but some formatting issues. Please correct these. spaces -> tabs

Copy link
Contributor

@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.

Looks good but has some formatting issues.

@0x15f
Copy link
Contributor Author

0x15f commented Oct 1, 2016

Okay I will fix those.

return self::getEnchantment(constant(Enchantment::class . "::TYPE_FISHING_" . strtoupper($name)));
}
else{
return new Enchantment(self::TYPE_INVALID, "unknown", 0, 0, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

^^

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I was just about to commit that, I'm squashing commits right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@dktapps
Copy link
Contributor

dktapps commented Oct 1, 2016

Please refrain from using the web editor in future, even if you're uploading files. I strongly recommend you learn to use git.

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@0x15f
Copy link
Contributor Author

0x15f commented Oct 1, 2016

Sorry I was eating, I'm learning Git now.

@0x15f
Copy link
Contributor Author

0x15f commented Oct 1, 2016

@dktapps Is it OK if I close this pr and restart? I learned the git basics 😸

@dktapps
Copy link
Contributor

dktapps commented Oct 1, 2016

If you can't squash or had difficulties, I can squash-merge this time, but in future please learn to do it properly.

@dktapps dktapps merged commit 6ccefae into iTXTech:master Oct 1, 2016
@0x15f
Copy link
Contributor Author

0x15f commented Oct 1, 2016

Thank you, I'm learning how to squash via command line right now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants