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.

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

[1.20.x] Fix Reach Issues (and eclipse compilation issues) #346

Merged
merged 7 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 11 additions & 7 deletions patches/net/minecraft/client/renderer/GameRenderer.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,26 @@
} catch (IOException ioexception) {
list1.forEach(p_172729_ -> p_172729_.getFirst().close());
throw new RuntimeException("could not reload shaders", ioexception);
@@ -834,12 +_,19 @@
@@ -834,12 +_,25 @@
this.minecraft.getProfiler().push("pick");
this.minecraft.crosshairPickEntity = null;
double d0 = (double)this.minecraft.gameMode.getPickRange();
- this.minecraft.hitResult = entity.pick(d0, p_109088_, false);
+ double entityReach = this.minecraft.player.getEntityReach(); // Note - MC-76493 - We must validate players cannot click-through objects.
+ this.minecraft.hitResult = entity.pick(Math.max(d0, entityReach), p_109088_, false); // Run pick() with the max of the two, so we can prevent click-through.
+ d0 = Math.max(d0, entityReach); // Calculate hit results with the max of both reach distances, and compare against the individual ones later. This prevents click-through.
this.minecraft.hitResult = entity.pick(d0, p_109088_, false);
Vec3 vec3 = entity.getEyePosition(p_109088_);
- boolean flag = this.minecraft.gameMode.hasFarPickRange();
+ boolean flag = false && this.minecraft.gameMode.hasFarPickRange();
+ if (false) { // These flags are unused with attributes.
boolean flag = this.minecraft.gameMode.hasFarPickRange();
d0 = flag ? 6.0 : d0;
boolean flag1 = !flag;
- double d1 = this.minecraft.hitResult != null ? this.minecraft.hitResult.getLocation().distanceToSqr(vec3) : d0 * d0;
+ double d1 = this.minecraft.hitResult != null && this.minecraft.hitResult.getType() != HitResult.Type.MISS ? this.minecraft.hitResult.getLocation().distanceToSqr(vec3) : d0 * d0;
+ }
+ // If we picked a block, we need to ignore entities past that block. Added != MISS check to not truncate on failed picks.
+ // Also fixes MC-250858
+ // Note: If vanilla ever actually fixes MC-250858 and adds this != MISS check, IPlayerExtension#getEntityReach needs to be updated from +2 to +3.
+ double d1 = this.minecraft.hitResult != null && this.minecraft.hitResult.getType() != HitResult.Type.MISS ? this.minecraft.hitResult.getLocation().distanceToSqr(vec3) : d0 * d0;
+ double blockReach = this.minecraft.player.getBlockReach();
+ // Discard the result as a miss if it is outside the block reach.
+ // Discard the block pick result as a miss if it is outside the block reach.
+ if (d1 > blockReach * blockReach) {
+ Vec3 pos = this.minecraft.hitResult.getLocation();
+ this.minecraft.hitResult = BlockHitResult.miss(pos, Direction.getNearest(vec3.x, vec3.y, vec3.z), BlockPos.containing(pos));
Expand Down
5 changes: 3 additions & 2 deletions patches/net/minecraft/world/entity/Entity.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@
public void setMaxUpStep(float p_275672_) {
this.maxUpStep = p_275672_;
}
@@ -3419,6 +_,118 @@
@@ -3419,6 +_,119 @@
public boolean mayInteract(Level p_146843_, BlockPos p_146844_) {
return true;
}
Expand Down Expand Up @@ -631,7 +631,8 @@
+ }
+
+ @Override
+ public final <T> @Nullable T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ @Nullable
+ public final <T> T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ // Entities are always saved, no setChanged() call is necessary.
+ return super.setData(type, data);
+ }
Expand Down
3 changes: 1 addition & 2 deletions patches/net/minecraft/world/entity/player/Player.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,10 @@
}

float f4 = 0.0F;
@@ -1193,11 +_,13 @@
@@ -1193,11 +_,12 @@

for(LivingEntity livingentity : this.level()
.getEntitiesOfClass(LivingEntity.class, p_36347_.getBoundingBox().inflate(1.0, 0.25, 1.0))) {
+ //PATCH 1.20.2: Entity reach distance was redone, this needs to be verified.
+ double entityReachSq = Mth.square(this.getEntityReach()); // Use entity reach instead of constant 9.0. Vanilla uses bottom center-to-center checks here, so don't update this to use canReach, since it uses closest-corner checks.
if (livingentity != this
&& livingentity != p_36347_
Expand Down
3 changes: 2 additions & 1 deletion patches/net/minecraft/world/item/Item.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,14 @@
}

protected static BlockHitResult getPlayerPOVHitResult(Level p_41436_, Player p_41437_, ClipContext.Fluid p_41438_) {
@@ -313,11 +_,13 @@
@@ -313,11 +_,14 @@
float f5 = Mth.sin(-f * (float) (Math.PI / 180.0));
float f6 = f3 * f4;
float f7 = f2 * f4;
- double d0 = 5.0;
- Vec3 vec31 = vec3.add((double)f6 * 5.0, (double)f5 * 5.0, (double)f7 * 5.0);
+ double d0 = p_41437_.getBlockReach();
+ if (!p_41437_.isCreative() && d0 != 0) d0 += 0.5D; // The vanilla constant here was 5.0, but the default survival block reach is 4.5. Creative default is already 5.0.
+ Vec3 vec31 = vec3.add((double)f6 * d0, (double)f5 * d0, (double)f7 * d0);
return p_41436_.clip(new ClipContext(vec3, vec31, ClipContext.Block.OUTLINE, p_41438_, p_41437_));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
}

public boolean triggerEvent(int p_58889_, int p_58890_) {
@@ -185,6 +_,19 @@
@@ -185,6 +_,20 @@

public BlockEntityType<?> getType() {
return this.type;
Expand All @@ -61,7 +61,8 @@
+ }
+
+ @Override
+ public final <T> @Nullable T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ @Nullable
+ public final <T> T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ setChanged();
+ return super.setData(type, data);
}
Expand Down
5 changes: 3 additions & 2 deletions patches/net/minecraft/world/level/chunk/LevelChunk.java.patch
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
this.blockEntities.values().forEach(p_187988_ -> {
Level level = this.level;
if (level instanceof ServerLevel serverlevel) {
@@ -646,6 +_,26 @@
@@ -646,6 +_,27 @@
return new LevelChunk.BoundTickingBlockEntity<>(p_156376_, p_156377_);
}

Expand All @@ -113,7 +113,8 @@
+ }
+
+ @Override
+ public <T> @Nullable T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ @Nullable
+ public <T> T setData(net.neoforged.neoforge.attachment.AttachmentType<T> type, T data) {
+ setUnsaved(true);
+ return attachmentHolder.setData(type, data);
+ }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,9 @@ public static <T> BlockCapability<T, Void> createVoid(ResourceLocation name, Cla
}

// INTERNAL
private static final CapabilityRegistry<BlockCapability<?, ?>> registry = new CapabilityRegistry<>(BlockCapability::new);

// Requires explicitly-typed constructor due to ECJ inference failure.
private static final CapabilityRegistry<BlockCapability<?, ?>> registry = new CapabilityRegistry<BlockCapability<?, ?>>(BlockCapability::new);

private BlockCapability(ResourceLocation name, Class<T> typeClass, Class<C> contextClass) {
super(name, typeClass, contextClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,9 @@ public static <T> EntityCapability<T, Void> createVoid(ResourceLocation name, Cl
}

// INTERNAL
private static final CapabilityRegistry<EntityCapability<?, ?>> registry = new CapabilityRegistry<>(EntityCapability::new);

// Requires explicitly-typed constructor due to ECJ inference failure.
private static final CapabilityRegistry<EntityCapability<?, ?>> registry = new CapabilityRegistry<EntityCapability<?, ?>>(EntityCapability::new);

private EntityCapability(ResourceLocation name, Class<T> typeClass, Class<C> contextClass) {
super(name, typeClass, contextClass);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ public static <T> ItemCapability<T, Void> createVoid(ResourceLocation name, Clas
}

// INTERNAL
private static final CapabilityRegistry<ItemCapability<?, ?>> registry = new CapabilityRegistry<>(ItemCapability::new);

// Requires explicitly-typed constructor due to ECJ inference failure.
private static final CapabilityRegistry<ItemCapability<?, ?>> registry = new CapabilityRegistry<ItemCapability<?, ?>>(ItemCapability::new);

private ItemCapability(ResourceLocation name, Class<T> typeClass, Class<C> contextClass) {
super(name, typeClass, contextClass);
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/net/neoforged/neoforge/common/NeoForgeMod.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,8 @@ public class NeoForgeMod {
private static final DeferredRegister<Codec<? extends StructureModifier>> STRUCTURE_MODIFIER_SERIALIZERS = DeferredRegister.create(NeoForgeRegistries.Keys.STRUCTURE_MODIFIER_SERIALIZERS, "neoforge");
private static final DeferredRegister<HolderSetType> HOLDER_SET_TYPES = DeferredRegister.create(NeoForgeRegistries.Keys.HOLDER_SET_TYPES, "neoforge");

@SuppressWarnings({ "unchecked", "rawtypes" })
private static final DeferredHolder<ArgumentTypeInfo<?, ?>, EnumArgument.Info<?>> ENUM_COMMAND_ARGUMENT_TYPE = COMMAND_ARGUMENT_TYPES.register("enum", () -> ArgumentTypeInfos.registerByClass(EnumArgument.class, new EnumArgument.Info()));
@SuppressWarnings({ "unchecked", "rawtypes" }) // Uses Holder instead of DeferredHolder as the type due to weirdness between ECJ and javac.
private static final Holder<ArgumentTypeInfo<?, ?>> ENUM_COMMAND_ARGUMENT_TYPE = COMMAND_ARGUMENT_TYPES.register("enum", () -> ArgumentTypeInfos.registerByClass(EnumArgument.class, new EnumArgument.Info()));
private static final DeferredHolder<ArgumentTypeInfo<?, ?>, SingletonArgumentInfo<ModIdArgument>> MODID_COMMAND_ARGUMENT_TYPE = COMMAND_ARGUMENT_TYPES.register("modid", () -> ArgumentTypeInfos.registerByClass(ModIdArgument.class,
SingletonArgumentInfo.contextFree(ModIdArgument::modIdArgument)));

Expand All @@ -184,7 +184,7 @@ public class NeoForgeMod {
public static final Holder<Attribute> BLOCK_REACH = ATTRIBUTES.register("block_reach", () -> new RangedAttribute("neoforge.block_reach", 4.5D, 0.0D, 1024.0D).setSyncable(true));

/**
* Attack Range represents the distance at which a player may attack an entity. The default is 3 blocks. Players in creative mode have an additional 3 blocks of entity reach.
* Attack Range represents the distance at which a player may attack an entity. The default is 3 blocks. Players in creative mode have an additional 2 blocks of entity reach.
* The default of 3.0 is technically considered a bug by Mojang - see MC-172289 and MC-92484. However, updating this value would allow for longer-range attacks on vanilla servers, which makes some people mad.
*
* @see IPlayerExtension#getEntityReach()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ private Player self() {
}

/**
* The entity reach is increased by 3 for creative players, unless it is currently zero, which disables attacks and entity interactions.
* The entity reach is increased by 2 for creative players, unless it is currently zero, which disables attacks and entity interactions.
Technici4n marked this conversation as resolved.
Show resolved Hide resolved
*
* @return The entity reach of this player.
*/
default double getEntityReach() {
double range = self().getAttributeValue(NeoForgeMod.ENTITY_REACH.value());
return range == 0 ? 0 : range + (self().isCreative() ? 3 : 0);
return range == 0 ? 0 : range + (self().isCreative() ? 2 : 0);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ public static DeferredRegister.Blocks createBlocks(String modid) {

private final ResourceKey<? extends Registry<T>> registryKey;
private final String namespace;
private final Map<DeferredHolder<T, ?>, Supplier<? extends T>> entries = new LinkedHashMap<>();
private final Set<DeferredHolder<T, ?>> entriesView = Collections.unmodifiableSet(entries.keySet());
private final Map<DeferredHolder<T, ? extends T>, Supplier<? extends T>> entries = new LinkedHashMap<>();
private final Set<DeferredHolder<T, ? extends T>> entriesView = Collections.unmodifiableSet(entries.keySet());
private final Map<ResourceLocation, ResourceLocation> aliases = new HashMap<>();

@Nullable
Expand Down
Loading