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

Small fix, some new functionality and localization update #72

Merged
merged 10 commits into from
Sep 20, 2020
Merged

Small fix, some new functionality and localization update #72

merged 10 commits into from
Sep 20, 2020

Conversation

Aizistral
Copy link
Contributor

@Aizistral Aizistral commented Sep 18, 2020

So, list of changes:

Fixes:

  • A bug (I believe) where recipe book, if toggled visible while in survival, is still visible in Curios inventory when after switching to creative mode. The recipe book button is removed so it's not even possible to toggle it off without switching back to survival mode. Current fix does simply disable visibility of recipe book when opening Curios inventory in creative menu, which may not be perfect, but I couldn't come up with something better that would not have any potential conflicts with other mod's inventories that iherit same recipe book. Perhaps I am just dumb and there's much simpler solution. Current one is better than it used to be anyways.

Features:

  • ICurio#showAttributesTooltip call that kindly asks each curio whether or not tooltip listing their attributes when worn should be added, for each identifier. Sometimes mod creators might want to outline those attributes as part of their own stylized tooltip, similar to how I often do. I noticed "HideFlags" nbt check in ItemTooltipEvent listener which in theory may allow to achieve this, but it seems to be an inherited vanilla behaviour that isn't very obvious and I guess affects more than just Curios attribute tooltips.

  • Added curios:equip_curio criterion trigger, which is fired in same places where ICurio#onEquip calls are dispatched. Current implementation allows to perform item and/or location tests if neccessary. Might prove useful for some custom advancements thingies.

  • Added compatibility with Fortune and Looting enchantments. Default implementation considers enchantment levels of these enchantments applied to each curio ItemStack, if there are any, then applying total Fortune bonus through special GlobalLootModifierSerializer and Looting through LootingLevelEvent, respectively. Of course wearables cannot legitimately receive those enchantments by default, but mod creators can make it possible for their individual trinkets I suppose. Furthermore, default implementations reside in ICurio, as a way to alter or completely override amount of provided enchantment levels for both Looting and Fortune bonuses.
    This functionality can surely be achieved by each mod on their own, but I assume many mod creators may want to add curios that simply increase Fortune and/or Looting traits when worn, so why not spare us a burden of having to reinvent and reimplement this wheel.

Localization:

  • Updated Russian localization.

If you aim for parity between Forge and Fabric versions, it might be fair to somehow implement this stuff on Fabric port either, but I unfortunately can't help with that as so far I still have no idea what Fabric is even about.

@TheIllusiveC4 TheIllusiveC4 self-assigned this Sep 18, 2020
Copy link
Owner

@TheIllusiveC4 TheIllusiveC4 left a comment

Choose a reason for hiding this comment

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

Some really good stuff here, just a couple concerns up for discussion.

@TheIllusiveC4 TheIllusiveC4 added the type: enhancement New feature or request label Sep 18, 2020
@Aizistral
Copy link
Contributor Author

Fair point. Will push changes as soon as I get back to the computer.

I wonder if it makes sense to collect and store data on fortune/looting bonuses when entity's curios are cycled through in tick handler, rather than needlessly repeat that code and cycle through all stuff all over again for events when these values are needed.

@Aizistral
Copy link
Contributor Author

Aizistral commented Sep 18, 2020

I wonder if it makes sense to collect and store data on fortune/looting bonuses when entity's curios are cycled through in tick handler, rather than needlessly repeat that code and cycle through all stuff all over again for events when these values are needed.

Anyhow, I altered the implementation so that Fortune and Looting bonuses are stored in inventory wrapper, being recalculated in each LivingUpdateEvent along with other stuff that gets handled when cycling through curios map. This also allows to externally check (and even alter, if for whatever reason needed) values of those bonuses through an API, which I hope is good thing.

@TheIllusiveC4 TheIllusiveC4 merged commit 54a1cb9 into TheIllusiveC4:1.16.x-forge Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants