-
Notifications
You must be signed in to change notification settings - Fork 546
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
api/Open up range field #4231
api/Open up range field #4231
Conversation
Your Pull Request was automatically labelled as: "🎈 Feature" |
Slimefun preview buildA Slimefun preview build is available for testing! https://preview-builds.walshy.dev/download/Slimefun/4231/f731ad24
|
is there any real application to this? other than wolves killing mobs, and zombified piglins, both farms of which are in tight narrow kill areas, what would be the purpose to go out 15 blocks? |
It's worth noting a few things:
Overall I like the idea of having range within the object so at least addons can extend the class to do something similar here? Would like to hear further opinions before I do anything more. |
The addon one was one of the main reason for me too, all the points you made are valid, reason why in my PR description towards the end I said that I am waiting for an adjustement for energy and a recipe, i don't want to register something with null recipe, if the idea is accepted and the recipe made and energy cost made I will of course add them |
Yeah the 15x15x15 seems incredibly unnecessary and taxing in performance to boot imo |
So you did! Sorry for that :). |
There are many machines with only tier 2, we can just remove the tier 3 if deemed unnecessary, this way we add a new feature and allow addon makers to make their own xp collector easily if needed, still better than the way everything was hardcoded in that class |
I’d go so far as to say perhaps to leave the new tiers out completely and just have the api opened up like you have done. That’s would be a quick accept as far as I’m concerned. |
Updated now it only opens up the api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...b/thebusybiscuit/slimefun4/implementation/items/electric/machines/entities/ExpCollector.java
Show resolved
Hide resolved
...b/thebusybiscuit/slimefun4/implementation/items/electric/machines/entities/ExpCollector.java
Outdated
Show resolved
Hide resolved
...b/thebusybiscuit/slimefun4/implementation/items/electric/machines/entities/ExpCollector.java
Show resolved
Hide resolved
...b/thebusybiscuit/slimefun4/implementation/items/electric/machines/entities/ExpCollector.java
Outdated
Show resolved
Hide resolved
…n/items/electric/machines/entities/ExpCollector.java
Description
Add new EXP collectors
EXP Collector II: Collects XP orbs in a 7x7x7 area.
EXP Collector III: Collects XP orbs in a 15x15x15 area.
Proposed changes
Make EXP collector's constructor have a range and remove hardcoded capacity and consumption
Approved suggestion
Discord approved suggestion number #514
What is left to do
I need someone to tell me the recipes and or changes to the consumption and capacity of these new machines, so that I can register them into
SlimefunItemSetup
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values