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

Feature/bauble capability #208

Merged
merged 3 commits into from Aug 1, 2017

Conversation

@Xalcon
Copy link
Contributor

commented Jun 4, 2017

Hi Azanor,
This feature was requested by @Darkhax in #183 a while ago.

This PR replaces the whole IBauble item logic with a capability variant. I've added some backwards compatibility code that allows using IBauble as an interface for an Item as well as a capability.

  • Registered IBauble as a capability CAPABILITY_ITEM_BAUBLE
  • Replaced all (IBauble)itemStack.getItem() calls with itemStack.getCapability()
  • Added compatibility event handler that adds the IBauble capability to all items, that implement the IBauble interface

Note: I did see a lot of null checks for Itemstacks. These checks are always false, due to the Itemstack changes in 1.11.2. I did not refactor that code to reduce the amount of changes done by this PR.

This code has been successfuly tested with Botania 1.11.2-r1.9-343. I've also tested some custom capability that adds IBauble to the minecraft potions, an example can be found here.

I did not do any ports to 1.10.2 or below yet since I'd like to get some feedback on this code first :)

Xalcon added some commits Jun 4, 2017

- Registered IBauble as a capability
- Replaced all IBauble item checks with has/getCapability
- Added backwards compatibility event handler which adds the capability to all items that implement IBauble but dont have the capability yet
@gr8pefish

This comment has been minimized.

Copy link

commented Jun 28, 2017

I'd like to see this PR accepted. Azanor, any thoughts?

@Azanor

This comment has been minimized.

Copy link
Owner

commented Jun 28, 2017

I'll check it out once I get a bit of time this week.

@Azanor Azanor changed the base branch from master to 1.11 Aug 1, 2017

@Azanor Azanor merged commit 8f99a00 into Azanor:1.11 Aug 1, 2017

@Azanor

This comment has been minimized.

Copy link
Owner

commented Aug 1, 2017

I've merged this with the 1.11 branch for now. Mostly my own fault for taking so long - I wouldn't have been able to easily merge it into the new master which is for MC 1.12

@Xalcon

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2017

All good :) I'll make a PR for the 1.12 branch as well.

@Xalcon Xalcon referenced this pull request Aug 1, 2017

Merged

IBauble capability for 1.12 #217

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.