-
Notifications
You must be signed in to change notification settings - Fork 181
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 property for specially registered tools #2693
base: master
Are you sure you want to change the base?
Added property for specially registered tools #2693
Conversation
Crafttweaker and GroovyScript integrations are not implemented yet. |
This fix will eliminate the NullPointerException caused from repairing tools without toolProperty properly set for the material. Therefore, the current soft-mallet/plumber duplications will be fixed since they are separately registered currently.
72a5c14
to
2cfcfdb
Compare
The previous one (until 2cfcfdb) is faulty as many properties were not applied properly.
|
durability = (int) (durability * toolStats.getDurabilityMultiplier(stack)); | ||
} else { | ||
durability += toolStats.getBaseDurability(stack) * toolStats.getDurabilityMultiplier(stack); | ||
durability += (int) (toolStats.getBaseDurability(stack) * toolStats.getDurabilityMultiplier(stack)); |
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.
Is there any reason for the explicit casting and separating the *=
apart? The +=
and *=
operators implicitly perform the casting for integer types, iirc.
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.
No, but the operands are floating point numbers, so I just wanted to be clear here.
import it.unimi.dsi.fastutil.objects.Object2ObjectArrayMap; | ||
import it.unimi.dsi.fastutil.objects.Object2ObjectMap; | ||
|
||
public class SimpleToolProperty { |
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.
Since ToolProperty
now extends this, and this class now contains most of the functionality of ToolProperty
, I think it would be better to call this ToolProperty
and the current ToolProperty
MaterialToolProperty
instead.
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.
Ok, these classes are renamed.
} | ||
|
||
public boolean hasOverrideProperty(String toolId) { | ||
return getOverrideProperty(toolId) != null; |
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.
It's an edge case, but it might be better to do this.overideMap.hasKey(toolID)
instead here.
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.
Changed to Map.containsKey
method.
What
This PR adds another
IMaterialProperty
calledExtraToolProperty
. This new property willImplementation Details
Now
ExtraToolProperty
andToolProperty
are two different classes inherited fromSimpleToolProperty
.ToolProperty
keeps all its original functionality, whileExtraToolProperty
holds a modification mask for every other tools.These tool masks are distinguished by the strings defined in
ToolClasses
.The
get
method now takesIGTTool
into consideration of overriding the specified properties. Hence, all the tools generated will now adapt the overridden stats.The soft mallet and plungers are also re-defined using the new method. Now the tool stats are registered in
SoftToolAddition
and the recipe still registered fromToolRecipeHandler
.Outcome
By introducing this new method, #2692 is fixed since the tools are now correctly generated.
Also, modpacks can now have finer controls over the built-in tools. They are now able to change specific stats of some tools instead of modifying the whole set of tools from a material.
Potential Compatibility Issues
Most modpacks that tweaks the tools by the NBT will likely broken. However, I believe the bug similar #2692 will be fixed.