Skip to content
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.

Sign up for GitHub

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

Split NoSlowdown with checkboxes #800

Closed
wants to merge 8 commits into from
Closed

Split NoSlowdown with checkboxes #800

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dd08c59
Split NoSlowdown with checkboxes
ThisTestUser Mar 17, 2023
2922676
Merge branch 'master' into noslowdown-seperate
ThisTestUser Jul 13, 2023
b4e3733
Merge branch 'master' into noslowdown-seperate
ThisTestUser Dec 27, 2023
81c6020
Merge branch 'master' into noslowdown-seperate
Alexander01998 Dec 27, 2023
f3acf76
Refactor and cleanup NoSlowdown
ThisTestUser Dec 27, 2023
58da731
Improve code performance
ThisTestUser Dec 28, 2023
9afb47c
Merge branch 'master' into noslowdown-seperate
ThisTestUser Feb 29, 2024
9d93305
Merge branch 'master' into noslowdown-seperate
ThisTestUser May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 48 additions & 1 deletion src/main/java/net/wurstclient/hacks/NoSlowdownHack.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,63 @@
import net.wurstclient.Category;
import net.wurstclient.SearchTags;
import net.wurstclient.hack.Hack;
import net.wurstclient.settings.CheckboxSetting;
import net.wurstclient.settings.EnumSetting;

@SearchTags({"no slowdown", "no slow down"})
public final class NoSlowdownHack extends Hack
{
private final CheckboxSetting blocks =
new CheckboxSetting("Cancel block slowness",
"Cancels slowness effects caused by honey and soul sand.", true);

private final EnumSetting<ItemSlowness> items = new EnumSetting<>(
"Cancel item slowness", ItemSlowness.values(), ItemSlowness.ALL);

public NoSlowdownHack()
{
super("NoSlowdown");
setCategory(Category.MOVEMENT);
addSetting(blocks);
addSetting(items);
}

public boolean noBlockSlowness()
{
return isEnabled() && blocks.isChecked();
}

public boolean noItemSlowness()
{
return isEnabled() && items.getSelected() == ItemSlowness.ALL;
}

public boolean noItemSlownessExceptShields()
{
return isEnabled()
&& items.getSelected() == ItemSlowness.EXCEPT_SHIELDS;
}

private enum ItemSlowness
{
ALL("All"),
EXCEPT_SHIELDS("Except shields"),
NONE("None");

private final String name;

private ItemSlowness(String name)
{
this.name = name;
}

@Override
public String toString()
{
return name;
}
}

// See BlockMixin.onGetVelocityMultiplier() and
// ClientPlayerEntityMixin.wurstIsUsingItem()
// ClientPlayerEntityMixin.onTickMovementItemUse()
}
2 changes: 1 addition & 1 deletion src/main/java/net/wurstclient/mixin/BlockMixin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private static void onShouldDrawSide(BlockState state, BlockView world,
private void onGetVelocityMultiplier(CallbackInfoReturnable<Float> cir)
{
HackList hax = WurstClient.INSTANCE.getHax();
if(hax == null || !hax.noSlowdownHack.isEnabled())
if(hax == null || !hax.noSlowdownHack.noBlockSlowness())
return;

if(cir.getReturnValueF() < 1)
Expand Down
7 changes: 6 additions & 1 deletion src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import net.minecraft.entity.MovementType;
import net.minecraft.entity.effect.StatusEffect;
import net.minecraft.entity.effect.StatusEffects;
import net.minecraft.util.UseAction;
import net.minecraft.util.math.Vec3d;
import net.wurstclient.WurstClient;
import net.wurstclient.event.EventManager;
Expand Down Expand Up @@ -75,7 +76,11 @@ private void onTick(CallbackInfo ci)
ordinal = 0), method = "tickMovement()V")
private void onTickMovementItemUse(CallbackInfo ci)
{
if(WurstClient.INSTANCE.getHax().noSlowdownHack.isEnabled())
if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
|| (WurstClient.INSTANCE.getHax().noSlowdownHack
.noItemSlownessExceptShields()
&& ((ClientPlayerEntity)(Object)this).getActiveItem()
.getUseAction() != UseAction.BLOCK))
Comment on lines +100 to +104
Copy link

@coderabbitai coderabbitai bot May 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logic to selectively prevent item slowness based on new settings.

Consider breaking this complex conditional into multiple lines or extracting it into a method for better readability and maintainability.

- if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
-	|| (WurstClient.INSTANCE.getHax().noSlowdownHack
-		.noItemSlownessExceptShields()
-		&& ((ClientPlayerEntity)(Object)this).getActiveItem()
-			.getUseAction() != UseAction.BLOCK))
-	hideNextItemUse = true;
+ boolean shouldHideItemUse = WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
+	|| (WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlownessExceptShields()
+		&& getActiveItemUseAction() != UseAction.BLOCK);
+ hideNextItemUse = shouldHideItemUse;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
|| (WurstClient.INSTANCE.getHax().noSlowdownHack
.noItemSlownessExceptShields()
&& ((ClientPlayerEntity)(Object)this).getActiveItem()
.getUseAction() != UseAction.BLOCK))
boolean shouldHideItemUse = WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness()
|| (WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlownessExceptShields()
&& getActiveItemUseAction() != UseAction.BLOCK);
hideNextItemUse = shouldHideItemUse;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Good idea, bad example. Creating a local variable for WurstClient.INSTANCE.getHax().noSlowdownHack would improve readability in this case.

But also, it seems odd that there are two separate methods noItemSlowness() and noItemSlownessExceptShields() here that both need to be called together like that. A better solution might be to have a single noItemSlowness(boolean shield) method, which would handle the special case for shields internally.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Alexander01998, you're absolutely right on both counts.

Creating a local variable for WurstClient.INSTANCE.getHax().noSlowdownHack does indeed make the code cleaner and more readable by reducing redundancy. Additionally, your suggestion to combine noItemSlowness() and noItemSlownessExceptShields() into a single method that internally handles the shield case is an excellent idea. It not only simplifies the calling code but also encapsulates the logic related to item slowness in a more maintainable way.

Here's how the revised code might look:

NoSlowdownHack noSlowdownHack = WurstClient.INSTANCE.getHax().noSlowdownHack;
boolean shouldHideItemUse = noSlowdownHack.noItemSlowness(getActiveItemUseAction() != UseAction.BLOCK);
hideNextItemUse = shouldHideItemUse;

In this version, noItemSlowness(boolean shield) would internally decide the logic based on whether the active item's use action is BLOCK, simplifying the external checks.

Thank you for your insightful suggestions!

New Learnings Added 
User: Alexander01998
PR: Wurst-Imperium/Wurst7#800
File: src/main/java/net/wurstclient/mixin/ClientPlayerEntityMixin.java:100-104
Timestamp: 2024-05-07T00:00:25.901Z
Learning: Alexander01998 suggested improving readability by creating a local variable for `WurstClient.INSTANCE.getHax().noSlowdownHack` and combining `noItemSlowness()` and `noItemSlownessExceptShields()` into a single method `noItemSlowness(boolean shield)`.

Note: Learnings are effective only in the context of specific code segments. To apply general coding guidelines, please configure review instructions.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more like this:

// The cast to Object and back is necessary because this code is inside of a mixin.
boolean shield = ((ClientPlayerEntity)(Object)this).getActiveItem().getUseAction() == UseAction.BLOCK;
if(WurstClient.INSTANCE.getHax().noSlowdownHack.noItemSlowness(shield))
	hideNextItemUse = true;

Or, if keeping the two methods:

NoSlowdownHack noSlowdown = WurstClient.INSTANCE.getHax().noSlowdownHack;
boolean shield = ((ClientPlayerEntity)(Object)this).getActiveItem().getUseAction() == UseAction.BLOCK;
if(noSlowdown.noItemSlowness() || (noSlowdown.noItemSlownessExceptShields() && shield)
	hideNextItemUse = true;

But the first solution is much cleaner.

hideNextItemUse = true;
}

Expand Down