New Hack: PortalESP (fixes #776) #835
Conversation
|
sexy |
|
@Alexander01998 please review this in the near future. A lot of people are waiting for this feature, it was also voted for multiple times in the poll for new features. PortalESP has been FULLY implemented and tested by this PR, the only thing left to make this feature a thing in the Wurst Client is a merge of this PR! Please review this ASAP as longer wait time directly leads to possible merge conflicts. |
Alexander01998
added
type:enhancement
There was a problem hiding this comment.
Hey @DrMaxNix,
Sorry for the late reply! A lot was happening when you submitted this and I'm just catching up on older pull requests. Luckily there was only a minor conflict caused by a Minecraft update, which was easy to fix.
More importantly, while examining your PR, I found that it duplicates a lot of code from Search and ChestESP. While this may not affect the functionality right now, maintaining two slightly different versions of the same code could become a problem in the future, as every change would have to be made twice and it's not obvious where the differences are.
Your PortalESP hack definitely seems like a great feature to have, but it's essential that it's integrated in a way that doesn't undermine the rest of the codebase. Ideally these common functionalities should be moved into utility or parent classes, which can then be extended or reused by both the existing modules and your new PortalESP. Unfortunately, that means I can't merge the pull request in its current state because it's going to need a fairly large refactoring first.
Please have patience as I work on integrating your changes into the main codebase in a sustainable and efficient way. If you're interested in helping out with the refactoring process, just let me know.
Thanks for your understanding and for your contributions so far.
|
Heyo, |
|
Hey DrMaxNix, I appreciate your willingness to help! As you can see, the wheels are already in motion to integrate your additions into the codebase. The refactoring should eventually lead to a unified system for Search-like hacks, once all the edge cases have been worked out. It would be great if you could explain each of the improvements and fixes that you've made in the duplicate code parts. Alternatively, you could also document these changes within the code by adding comments. This would provide context and help me better understand your changes. Right now these changes can be easy to miss amongst all the unchanged code and sometimes it isn't entirely clear to me why certain changes were made. One such example is the line Looking forward to your input on this. |
|
Sure! I can't remember all of the improvements because it is more than 3 months now that I've written that code.. But I'll go through everything an try to document all the improvements as good as I can!
These are all the changes I could find by looking through the code again.. In case I missed a change: generally speaking EVERY change I made to snippets used from the existing codebase have a reason! (Why should I change stuff that's already working..?) I still remember about a year ago I had a lot of minecraft crashes when using portals a lot.. These crashes are probably caused by all the previously mentioned bugs and fixing them for all the other hacks would be a HUGE performance boost.. Those optimizations even allow you to increase the chunk radius by quite a lot without the game starting to lag :) |
Removed the ForkJoinPool and made sure the groups are only updated when necessary.
WalkthroughThe update introduces a new feature to the Wurst Client, a Minecraft hack client, by adding the Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 8
Files ignored due to filter (1)
- src/main/resources/assets/wurst/lang/en_us.json
Files selected for processing (5)
- src/main/java/net/wurstclient/hack/HackList.java (1 hunks)
- src/main/java/net/wurstclient/hacks/PortalEspHack.java (1 hunks)
- src/main/java/net/wurstclient/hacks/portalesp/PortalEspBlockGroup.java (1 hunks)
- src/main/java/net/wurstclient/hacks/portalesp/PortalEspRenderer.java (1 hunks)
- src/main/java/net/wurstclient/util/ChunkSearcherMulti.java (1 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/net/wurstclient/hack/HackList.java
Additional comments (Suppressed): 9
src/main/java/net/wurstclient/hacks/PortalEspHack.java (3)
106-117: Ensure that the thread pool is properly initialized and that there are no potential race conditions or deadlocks. Also, consider handling exceptions that may occur during the execution of tasks in the thread pool.
120-131: When disabling the feature, ensure that all resources are properly cleaned up to prevent memory leaks. This includes shutting down the executor service, clearing collections, and closing buffers.
210-241: Ensure that OpenGL settings are correctly set and reset before and after rendering. Incorrect settings can lead to visual glitches or performance issues.
src/main/java/net/wurstclient/hacks/portalesp/PortalEspBlockGroup.java (4)
31-37: Ensure that the
enabledCheckboxSetting is not null when passed to this constructor. If it's null, theisEnabled()method will always return true, which might not be the expected behavior.48-54: The
getBoxmethod returns null if the block atposcannot be clicked. Ensure that all callers of this method handle the null return value appropriately to avoid potential NullPointerExceptions.61-64: If
enabledis null, this method will return true. Make sure this is the intended behavior and that there are no cases whereenabledshould be checked but is null.81-84: This method returns an unmodifiable view of the
boxeslist. Any attempt to modify the returned list will result in an UnsupportedOperationException. Ensure that all callers of this method are aware of this and do not attempt to modify the returned list.src/main/java/net/wurstclient/hacks/portalesp/PortalEspRenderer.java (2)
33-34: Ensure that the static VertexBuffer objects
solidBoxandoutlinedBoxare thread-safe. If multiple threads access these objects simultaneously, it could lead to data races or inconsistent states.108-123: The
prepareBuffers()andcloseBuffers()methods are public and static, which means they can be called from anywhere in your codebase. This could potentially lead to issues if they're called while the buffers are being used for rendering. Consider adding synchronization or other concurrency control mechanisms to prevent potential race conditions.
| public static void closeBuffers() | ||
| { | ||
| Stream.of(solidBox, outlinedBox).filter(Objects::nonNull) | ||
| .forEach(VertexBuffer::close); | ||
| } |
There was a problem hiding this comment.
Closing the VertexBuffer objects without nullifying them could potentially lead to NullPointerExceptions if they are accessed after being closed. After closing the buffers, consider setting them to null.
public static void closeBuffers()
{
Stream.of(solidBox, outlinedBox).filter(Objects::nonNull)
.forEach(VertexBuffer::close);
+ solidBox = null;
+ outlinedBox = null;
}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Files selected for processing (1)
- src/main/java/net/wurstclient/hacks/PortalEspHack.java (1 hunks)
Additional comments (Suppressed): 1
src/main/java/net/wurstclient/hacks/PortalEspHack.java (1)
- 144-177: The
onRender()method sets and resets several OpenGL settings. It's important to ensure that these settings are correctly set and reset, as incorrect settings can cause rendering issues. Please verify that these settings are correct for your use case.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- src/main/resources/assets/wurst/lang/en_us.json
Files selected for processing (1)
- src/main/java/net/wurstclient/hack/HackList.java (1 hunks)
Files skipped from review due to trivial changes (1)
- src/main/java/net/wurstclient/hack/HackList.java
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (2)
- src/main/java/net/wurstclient/hacks/PortalEspHack.java (1 hunks)
- src/main/java/net/wurstclient/hacks/portalesp/PortalEspRenderer.java (1 hunks)
Additional comments: 2
src/main/java/net/wurstclient/hacks/PortalEspHack.java (2)
133-140: The
onUpdatemethod checks if the chunk searchers have changed and if the groups are up-to-date. If the coordinator is done and the groups are not up-to-date, it callsupdateGroupBoxes. This is a good approach as it ensures that the ESP boxes are only updated when necessary, which can save on performance. However, it's important to ensure that thecoordinator.isDone()method accurately reflects the completion state of all chunk searchers to prevent any race conditions or missed updates.180-195: The
updateGroupBoxesmethod clears the groups and then adds all the found portal blocks to their respective groups. TheaddToGroupBoxesmethod checks each result against the block types of the groups and adds the position to the group if it matches. This is a straightforward and efficient way to update the groups. However, it's important to ensure that theclearmethod ofPortalEspBlockGroupis thread-safe if it's possible forupdateGroupBoxesto be called from multiple threads.
| import net.minecraft.util.math.BlockPos; | ||
| import net.minecraft.util.math.Box; | ||
| import net.minecraft.util.math.Vec3d; | ||
| import net.wurstclient.util.RegionPos; | ||
| import net.wurstclient.util.RenderUtils; | ||
| import net.wurstclient.util.RotationUtils; | ||
|
|
||
| public final class PortalEspRenderer | ||
| { | ||
| private static VertexBuffer solidBox; | ||
| private static VertexBuffer outlinedBox; | ||
|
|
||
| private final MatrixStack matrixStack; | ||
| private final RegionPos region; | ||
| private final Vec3d start; | ||
|
|
||
| public PortalEspRenderer(MatrixStack matrixStack, float partialTicks) | ||
| { | ||
| this.matrixStack = matrixStack; | ||
| region = RenderUtils.getCameraRegion(); | ||
| start = RotationUtils.getClientLookVec(partialTicks) | ||
| .add(RenderUtils.getCameraPos()).subtract(region.toVec3d()); | ||
| } | ||
|
|
||
| public void renderBoxes(PortalEspBlockGroup group) | ||
| { | ||
| float[] colorF = group.getColorF(); | ||
|
|
||
| for(Box box : group.getBoxes()) | ||
| { | ||
| matrixStack.push(); | ||
|
|
||
| matrixStack.translate(box.minX - region.x(), box.minY, | ||
| box.minZ - region.z()); | ||
|
|
||
| matrixStack.scale((float)(box.maxX - box.minX), | ||
| (float)(box.maxY - box.minY), (float)(box.maxZ - box.minZ)); | ||
|
|
||
| Matrix4f viewMatrix = matrixStack.peek().getPositionMatrix(); | ||
| Matrix4f projMatrix = RenderSystem.getProjectionMatrix(); | ||
| ShaderProgram shader = RenderSystem.getShader(); | ||
|
|
||
| RenderSystem.setShaderColor(colorF[0], colorF[1], colorF[2], 0.25F); | ||
| solidBox.bind(); | ||
| solidBox.draw(viewMatrix, projMatrix, shader); | ||
| VertexBuffer.unbind(); | ||
|
|
||
| RenderSystem.setShaderColor(colorF[0], colorF[1], colorF[2], 0.5F); | ||
| outlinedBox.bind(); | ||
| outlinedBox.draw(viewMatrix, projMatrix, shader); | ||
| VertexBuffer.unbind(); | ||
|
|
||
| matrixStack.pop(); | ||
| } | ||
| } | ||
|
|
||
| public void renderLines(PortalEspBlockGroup group) | ||
| { | ||
| Matrix4f matrix = matrixStack.peek().getPositionMatrix(); | ||
| Tessellator tessellator = RenderSystem.renderThreadTesselator(); | ||
| BufferBuilder bufferBuilder = tessellator.getBuffer(); | ||
|
|
||
| float[] colorF = group.getColorF(); | ||
| RenderSystem.setShaderColor(colorF[0], colorF[1], colorF[2], 0.5F); | ||
|
|
||
| bufferBuilder.begin(VertexFormat.DrawMode.DEBUG_LINES, | ||
| VertexFormats.POSITION); | ||
|
|
||
| for(Box box : group.getBoxes()) | ||
| { | ||
| Vec3d end = box.getCenter().subtract(region.toVec3d()); | ||
|
|
||
| bufferBuilder | ||
| .vertex(matrix, (float)start.x, (float)start.y, (float)start.z) | ||
| .next(); | ||
|
|
||
| bufferBuilder | ||
| .vertex(matrix, (float)end.x, (float)end.y, (float)end.z) | ||
| .next(); | ||
| } | ||
|
|
||
| tessellator.draw(); | ||
| } | ||
|
|
||
| public static void prepareBuffers() | ||
| { | ||
| closeBuffers(); | ||
| solidBox = new VertexBuffer(VertexBuffer.Usage.STATIC); | ||
| outlinedBox = new VertexBuffer(VertexBuffer.Usage.STATIC); | ||
|
|
||
| Box box = new Box(BlockPos.ORIGIN); | ||
| RenderUtils.drawSolidBox(box, solidBox); | ||
| RenderUtils.drawOutlinedBox(box, outlinedBox); | ||
| } | ||
|
|
||
| public static void closeBuffers() | ||
| { | ||
| Stream.of(solidBox, outlinedBox).filter(Objects::nonNull) | ||
| .forEach(VertexBuffer::close); | ||
| } |
There was a problem hiding this comment.
The PortalEspRenderer class is well-structured and follows Java conventions. However, there are a few points to consider:
-
The
renderBoxesmethod (lines 48-78) does not check if thePortalEspBlockGroupis enabled before rendering. If the enabled check is indeed happening outside this method as mentioned by the previous reviewer, then this is acceptable. However, it's typically good practice to ensure that such checks are close to the rendering logic to avoid unnecessary processing and to make the code easier to understand and maintain. If the enabled state can change dynamically, consider adding a check here as well to prevent rendering when not needed. -
The
renderLinesmethod (lines 80-106) uses a debug draw mode (line 89) which might not be intended for production use. Ensure that this is the correct draw mode for the intended visual effect. -
The
prepareBuffersmethod (lines 108-117) is creating newVertexBufferobjects without checking if they already exist, which could lead to memory leaks. It's good practice to check if the buffers are already initialized before creating new ones. This is especially important in a game context where memory management is crucial for performance. -
The
closeBuffersmethod (lines 119-123) closes the vertex buffers but does not set them to null, which could potentially lead toNullPointerExceptionsif they are accessed after being closed. As per the previous review, setting them to null after closing would be a safer approach.
Here's a proposed fix for the prepareBuffers and closeBuffers methods:
public static void prepareBuffers() {
closeBuffers();
solidBox = new VertexBuffer(VertexBuffer.Usage.STATIC);
outlinedBox = new VertexBuffer(VertexBuffer.Usage.STATIC);
Box box = new Box(BlockPos.ORIGIN);
RenderUtils.drawSolidBox(box, solidBox);
RenderUtils.drawOutlinedBox(box, outlinedBox);
}
public static void closeBuffers() {
Stream.of(solidBox, outlinedBox).filter(Objects::nonNull)
.forEach(VertexBuffer::close);
solidBox = null;
outlinedBox = null;
}| public void onRender(MatrixStack matrixStack, float partialTicks) | ||
| { | ||
| // GL settings | ||
| GL11.glEnable(GL11.GL_BLEND); | ||
| GL11.glBlendFunc(GL11.GL_SRC_ALPHA, GL11.GL_ONE_MINUS_SRC_ALPHA); | ||
| GL11.glEnable(GL11.GL_CULL_FACE); | ||
| GL11.glDisable(GL11.GL_DEPTH_TEST); | ||
|
|
||
| matrixStack.push(); | ||
| RenderUtils.applyRegionalRenderOffset(matrixStack); | ||
|
|
||
| PortalEspRenderer espRenderer = | ||
| new PortalEspRenderer(matrixStack, partialTicks); | ||
|
|
||
| if(style.getSelected().hasBoxes()) | ||
| { | ||
| RenderSystem.setShader(GameRenderer::getPositionProgram); | ||
| groups.stream().filter(PortalEspBlockGroup::isEnabled) | ||
| .forEach(espRenderer::renderBoxes); | ||
| } | ||
|
|
||
| if(style.getSelected().hasLines()) | ||
| { | ||
| RenderSystem.setShader(GameRenderer::getPositionProgram); | ||
| groups.stream().filter(PortalEspBlockGroup::isEnabled) | ||
| .forEach(espRenderer::renderLines); | ||
| } | ||
|
|
||
| matrixStack.pop(); | ||
|
|
||
| // GL resets | ||
| RenderSystem.setShaderColor(1, 1, 1, 1); | ||
| GL11.glEnable(GL11.GL_DEPTH_TEST); | ||
| GL11.glDisable(GL11.GL_BLEND); | ||
| } |
There was a problem hiding this comment.
The onRender method sets up the OpenGL environment, pushes the current matrix stack, applies a regional render offset, and then renders the portal ESP boxes and lines if enabled. After rendering, it pops the matrix stack and resets the OpenGL state. This is a standard approach for rendering in Minecraft mods. However, it's important to ensure that the OpenGL state is always properly reset after rendering to avoid affecting other render operations in the game. The use of RenderSystem.setShader(GameRenderer::getPositionProgram) before rendering each group is redundant and could be moved outside of the if statements to avoid setting the shader multiple times unnecessarily.
Description
This PR adds a new hack: PortalESP. It highlights nether portals, end portals, end portal frames and end gateways in a similar way that ChestESP does. It is highly customizable and tuned for efficiency.
Screenshots
Summary by CodeRabbit
New Features
PortalEspHackfor enhanced in-game portal visibility, allowing players to see different types of portals with customizable colors and styles.PortalEspHackthat includes solid and outlined boxes, and connecting lines for a better visual experience.Enhancements
HackListwith the addition of thePortalEspHackto offer players new capabilities.Please note that these changes are part of a gaming modification and should be used in accordance with the game's terms of service.