UpdateAnvilEvent#1898
Merged
Merged
Conversation
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
ccda965 to
ca16fc4
Compare
gabizou
requested changes
Aug 18, 2018
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
ca16fc4 to
5e850f1
Compare
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
5e850f1 to
c22b6af
Compare
gabizou
requested changes
Aug 30, 2018
gabizou
added
system: event
status: needs updating
and removed
status: input wanted
labels
gabizou
requested changes
Sep 5, 2018
gabizou
left a comment
gabizou
left a comment
Member
There was a problem hiding this comment.
Pretty much done, just move the other two groups of methods into a transaction and you're pretty much set.
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
3708d00 to
1eb983e
Compare
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
1eb983e to
5247372
Compare
gabizou
approved these changes
Nov 17, 2018
Cybermaxke
reviewed
Nov 17, 2018
| /** | ||
| * The level and material costs for an anvil usage. | ||
| */ | ||
| interface Cost extends DataSerializable { |
Contributor
There was a problem hiding this comment.
I would prefer this class to be immutable with a builder. That way is the transaction also completely immutable, so you cannot modify the original costs.
Contributor
Author
There was a problem hiding this comment.
How about immutable without a builder like it is now?
Contributor
There was a problem hiding this comment.
Oh, I didn't notice that it returns a new object. Javadoc is missing @return
Ant it's a common practice to use builders in sponge, not sure if it's worth adding one for this.
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
5247372 to
7d7bc93
Compare
Cybermaxke
reviewed
Nov 18, 2018
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
7d7bc93 to
3a082eb
Compare
Cybermaxke
reviewed
Nov 18, 2018
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
2 times, most recently
from
df469f3 to
23fa671
Compare
Faithcaio
force-pushed
the
feature/anvilupdateevent
branch
from
23fa671 to
bd923a6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SpongeAPI | SpongeCommon
Adds an event to handle anvil results and costs.
The event fires before vanilla is doing its computation but after forge mods had a chance to set the values.
As such there is no initial level/material cost and the result is empty by default.
Only when the result is set by the plugin costs will be applied.
No result set => Vanilla Behaviour
Result set => Override Costs and Result
Cancel => Empty Result