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 meter and dynamic remainder #1

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

implement meter and dynamic remainder #1

wants to merge 11 commits into from

Conversation

repulica
Copy link
Collaborator

  • implement custom damage meter
  • implement dynamic recipe remainder
    • right now returns an item due to redirect limitations, mixins could be changed to allow full stack return if desired
    • could have knowledge of recipe id except for in brewing
  • currently untested, focusing on structure currently
  • currently undocumented, focusing on structure currently

@repulica
Copy link
Collaborator Author

now with license headers

Copy link
Member

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

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

I am a bit unsure about what some of this does like MeterComponent so it's a bit hard to review right now.

JavaDoc would greatly improve this and help us understand what all of this does.

Now my current concerns:

  • WovenSettingsHolder is a bit vague, should be fine with the package.
  • WovenItemSettings should also implements the equipmentSlot from Fabric API as currently it limits the choice for modders, either they use Woven's ItemSettings and they loose that method, or they use Fabric's ItemSettings and they loose access to MeterComponent, etc.
    This, however, can be solved in another PR if it's too much for this PR.

Else this PR seems overall good. Will review again when I'm sure what that MeterComponent does.

Copy link
Member

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

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

This is cool, we need to do some testing to see if it works as intended, thank you!

@repulica
Copy link
Collaborator Author

tested and confirmed working for everything except brewing remainders (due to not being able to add new brewing ingredients without extensive mixin)

@repulica
Copy link
Collaborator Author

now throws if a user attempts to set multiple recipe remainder styles at once, like how vanilla item settings throws if you try to set both damage and max count

Copy link
Member

@LambdAurora LambdAurora left a comment

Choose a reason for hiding this comment

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

Seems good for me.

@emilyploszaj
Copy link

I'm wondering how recipe remainders work with stacks that have more than 1 item, does vanilla typically ignore it or is that in scope of this? Otherwise seems good.

@LemmaEOF
Copy link

The only items that have recipe remainders in vanilla are non-stackable - water buckets, lava buckets, milk buckets, dragon's breath, and honey bottles. Because of that, it just creates a full new stack. Dynamic recipe remainders provide a way to do arbitrary behavior for remainders, including stackable ones.

@LambdAurora
Copy link
Member

Also note: the compileJava task fails

@repulica
Copy link
Collaborator Author

i have no idea why

@repulica
Copy link
Collaborator Author

oh idea just didnt catch anything when i swapped from functions to functional interfaces just a sec

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

Successfully merging this pull request may close these issues.

4 participants