-
Notifications
You must be signed in to change notification settings - Fork 41
minor changes to blocks #63
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
base: master
Are you sure you want to change the base?
Conversation
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.
These are my thoughts on some of the high level stuff, it's up to you @GoldenStack if you want to address my comments or ask @Garrett-EMS to
blocks/src/main/java/net/minestom/vanilla/fluids/MinestomFluids.java
Outdated
Show resolved
Hide resolved
public class FluidUtils { | ||
|
||
public static Point raycastForFluid(Player player, Point startPosition, Vec direction, double maxDistance) { | ||
Point currentPosition = startPosition; |
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.
You should be able to do a sweep cast here as opposed to an approximation.
IIRC it's something like boundingBox.sweep()
.
BlockIterators might also be useful.
@@ -0,0 +1,7 @@ | |||
{ |
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.
Why are there these random resource json files?
You should be able to extract these from the datapack 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.
fyi: Not all of them are in the vanila tags (i.e. vri:glass_panes don't exist as a block tag)
* Interface for tag providers that check if elements are tagged and can return sets of elements with specific tags | ||
* @param <T> The type of element being tagged | ||
*/ | ||
public interface TagProvider<T> { |
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.
We should remove this in favor of our current tags system, or merge/replace it into our current tag system.
Ideally all these tags should be fetched from the datapack.
* Ported from Kotlin to Java and adapted for use in this project with modifications. | ||
*/ | ||
public class CandlePlacementRule extends BlockPlacementRule { | ||
private static final Map<Block, Block> CANDLE_CAKE = new HashMap<>(); |
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.
Use a Map#of or Map#copyOf at the end and assign it to this.
CANDLE_CAKE.put(Block.BLACK_CANDLE, Block.BLACK_CANDLE_CAKE); | ||
} | ||
|
||
public static Map<Block, Block> getCANDLE_CAKE() { |
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.
Seems like a weird name
*/ | ||
public class CandleCakeRule implements BlockHandler { | ||
private final Block block; | ||
private static final Map<Block, Block> CAKE_CANDLE = new HashMap<>(); |
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.
Map#of
*/ | ||
public class CopperOxidationRule implements BlockHandler { | ||
private final Block block; | ||
public static final Map<Block, Block> oxidationStages = new HashMap<>(); |
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.
Map#copyOf
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.
Please add the author
8b0b667
to
d200260
Compare
Btw you've included the client jar in this pr, it should be removed lol |
Will do that. @GoldenStack any updates on reviewing this? |
port blocks n stuff to java from kotlin