feat: Displace better player check & options#1
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the Displace combat feature to base activation on actual attacks (via AttackEntityEvent) and adds new configuration toggles to control when displacement is allowed.
Changes:
- Add
weapon-onlyandknockback-onlyoptions to Displace and register them as module properties. - Replace mouse-button targeting with
AttackEntityEvent-driven targeting and track the attacked entity per tick. - Rename Gradle root project from the template name to
SUIM.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
src/main/java/coffee/axle/suim/feature/combat/Displace.java |
Adds new options and switches Displace activation/target selection to use AttackEntityEvent. |
settings.gradle.kts |
Updates rootProject.name to match the project branding (SUIM). |
Comments suppressed due to low confidence (1)
src/main/java/coffee/axle/suim/feature/combat/Displace.java:204
findClosestTargetappears to be dead code after switching targeting toAttackEntityEvent(there are no remaining call sites in this class). Either remove the unused method (and any now-unneeded imports/state it supports) or re-integrate it if the module is still intended to support "hold attack" targeting.
private EntityPlayer findClosestTarget(double rangeSq) {
EntityPlayer closest = null;
double closestDist = rangeSq;
for (EntityPlayer player : mc.theWorld.playerEntities) {
if (player == mc.thePlayer || player.isDead || player.deathTime != 0)
continue;
double dist = mc.thePlayer.getDistanceSqToEntity(player);
if (dist < closestDist) {
closestDist = dist;
closest = player;
}
}
return closest;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Entity target = attackedLivingEntity; | ||
| attackedLivingEntity = null; | ||
|
|
||
| active = target != null && (hasKBEnchant || anyMovementKey()); | ||
| hasKB = hasKBEnchant; | ||
| boolean weaponAllowed = !weaponOnlyEnabled || isHoldingAllowedWeapon(); | ||
| boolean knockbackFlag = !knockbackOnlyEnabled || hasKB; | ||
| active = target != null && weaponAllowed && (hasKB || anyMovementKey()); | ||
| if (!active) { | ||
| displaceThisTick = false; | ||
| compensateNextTick = false; |
There was a problem hiding this comment.
compensateNextTick compensation can't trigger because it depends on active being true in onPrePlayerInput, but active requires target != null and attackedLivingEntity is always cleared in onPreMotion. Unless AttackEntityEvent fires every tick, the next tick will have active == false and compensateNextTick gets reset, skipping the strafe compensation. Consider keeping the last target for a short window (ticks) or deriving active from keyBindAttack.isKeyDown() while a recent target exists, rather than clearing the target immediately.
| if (!(e.target instanceof EntityLivingBase)) return; | ||
| // if (!(e.target instanceof EntityPlayer)) return; |
There was a problem hiding this comment.
AttackEntityEvent stores any EntityLivingBase as the target, but the delay-window tracking/pruning logic (targetWindowStartTicks + pruneTargetDelayStates) only retains EntityPlayer entries. For non-player targets the entry will be pruned immediately, making delay/window behavior inconsistent. Either restrict onAttack to EntityPlayer (as the commented line suggests) or update the pruning/window logic to support non-player EntityLivingBase targets.
| if (!(e.target instanceof EntityLivingBase)) return; | |
| // if (!(e.target instanceof EntityPlayer)) return; | |
| if (!(e.target instanceof EntityPlayer)) | |
| return; |
| public void onAttack(AttackEntityEvent e) { | ||
| if (e.entityPlayer != mc.thePlayer) | ||
| return; | ||
| if (!(e.target instanceof EntityLivingBase)) return; | ||
| // if (!(e.target instanceof EntityPlayer)) return; | ||
| attackedLivingEntity = e.target; | ||
| } |
There was a problem hiding this comment.
onAttack updates module state (attackedLivingEntity) even when the module is disabled or when the client/player context is invalid. Other modules (e.g. HitSelect/MoreKB) guard AttackEntityEvent handlers with manager.isModuleEnabled(...) and Utils.nullCheck(). Add those guards here to avoid stale target state carrying over when toggling the module.
| import net.minecraft.entity.Entity; | ||
| import net.minecraft.entity.EntityLivingBase; | ||
| import net.minecraft.entity.player.EntityPlayer; | ||
| import net.minecraft.util.ChatComponentText; |
There was a problem hiding this comment.
ChatComponentText is imported but never used in this class. Please remove the unused import to keep the file clean and avoid warnings.
| import net.minecraft.util.ChatComponentText; |
No description provided.