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.
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
Add AutoPlaceHack #1051
base: master
Are you sure you want to change the base?
Add AutoPlaceHack #1051
Changes from all commits
1ebb8d9
76a5b03
d4b624e
21ce72b
32f85be
0998d1d
e27ce53
0b0ec5f
90e45c0
f3584a6
74c4def
5559d0f
a1e7c2e
bb0215f
02e19a2
334e507
ccb1292
d944be9
ad61d02
c7d8d4e
31ea40e
6d9f820
f1e9d11
1aed3af
1f7d318
41ab6d2
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Avoid using assertions for null checks in production code.
Using
assert
statements for null checks is not recommended in production code because assertions may be disabled at runtime. Instead, consider implementing explicit null checks to preventNullPointerException
.Apply this change:
📝 Committable suggestion
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.
🛠️ Refactor suggestion
Consider allowing placement on the top and bottom faces.
Currently, the hack does not allow placing blocks when targeting the top or bottom face of a block. This might limit the usefulness of the hack in certain building scenarios.
Consider removing or adjusting this condition to allow more flexible block placement.
📝 Committable suggestion
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.
This should be made configurable in the settings. The PR author said they only want it to place blocks horizontally, but I agree this might limit its usefulness e.g. if you're trying to tower up.
Some different ideas for how the settings might work:
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.
@Alexander01998
Great suggestion! Making the placement directions configurable would enhance the flexibility of the
AutoPlace
hack.You could implement this by adding an
EnumSetting
to allow users to select their preferred placement directions. Here's how you might do it:Then, adjust the
onUpdate
method to handle the selected option:Alternatively, if you prefer multiple checkboxes for more granular control, you could add:
And modify the
onUpdate
method accordingly:This gives users the flexibility to enable or disable placement in specific directions based on their needs.
Let me know your thoughts on these options!
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.
🛠️ Refactor suggestion
Simplify the condition for better readability.
The conditional statement can be simplified for clarity by checking
onlyBelowFeet
first before comparing positions.Refactor the condition:
📝 Committable suggestion
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.
Redundant call to
interactItem
.The call to
MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
is unnecessary in this context because you are already interacting with the block usinginteractBlock
. This might lead to unintended item interactions.Remove the redundant method call:
- MC.interactionManager.interactItem(player, Hand.MAIN_HAND);
📝 Committable suggestion
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.
interactBlock
andinteractItem
work in complicated and confusing ways. Sometimes both need to be called and sometimes even twice to match vanilla Minecraft behavior. I'd recommend instead using Wurst's utilitynet.wurstclient.util.InteractionSimulator
whenever possible.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.
@Alexander01998, thank you for the clarification. Using
InteractionSimulator
makes sense to ensure consistent behavior with vanilla Minecraft. I've updated the code to useInteractionSimulator
:(•ᴗ•) A curious rabbit tilts its head in understanding.
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.
Handle potential
NullPointerException
forMC.interactionManager
.Similar to the player entity,
MC.interactionManager
could be null. Relying on assertions might not prevent exceptions in production.Implement a null check:
📝 Committable suggestion