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

added curios support for divination/seer sigil #1961

Open
wants to merge 3 commits into
base: 1.18.2
Choose a base branch
from

Conversation

stellanera98
Copy link

Changed ElementDivinedInformation.shouldRender() to consider Curios slots if the mod is loaded (BloodMagic.curiosLoaded) as well as the main and off hand

Changed the check for the sigils in above function a bit to avoid basically duplicating what's inside the if/else clause

Added the divination and seer sigil to the charm and living armour socket curios tags

@VT-14
Copy link
Collaborator

VT-14 commented Aug 5, 2023

When I initially added Curios compatibility I reworked BM's inventory handler. It's set up to allow add-ons to be able to add their custom inventories via the API too.

I would also like to keep as much Curios related code as possible consolidate within https://github.com/WayofTime/BloodMagic/blob/1.18.2/src/main/java/wayoftime/bloodmagic/compat/CuriosCompat.java. The current PR ends up with a bit of duplicated code already present in that class.

Right now the Inventory Helper class (https://github.com/WayofTime/BloodMagic/blob/1.18.2/src/main/java/wayoftime/bloodmagic/util/helper/InventoryHelper.java) only has a single 'getAllInventories' method as when I wrote it that was the only check I needed a the time. Some kind of 'getActiveSlots' method would need to be added, preferably with a way in the API for add-ons to register which of their inventory providers should be considered 'active' as well (ex. "curiosInventory" from BloodMagicAPI.INSTANCE.registerInventoryProvider("curiosInventory", player -> getCuriosInventory(player));).

@stellanera98
Copy link
Author

Added registerActiveInventoryProvider(String) to IBloodMagicAPI and BloodMagicAPI (since its just a list of strings maybe it shouldnt have provider in the name?)
Added getActiveInventoryProvider() to BloodMagicAPI
Added getActiveInventories(Player) to InventoryHelper
Changed CuriosCompat.registerInventory() to also register the inventory as active
Changed ElementDivinedInformation.shouldRender() to use InventoryHelper.getActiveInventories(player) instead of the code I copied from CuriosCompat before

@VT-14 VT-14 added the 1.18.2 label Oct 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants