neoforged / NeoForge

Neo Modding API for Minecraft, based on Forge
https://projects.neoforged.net/neoforged/neoforge
Other
1.13k stars 165 forks source link

Shears & Tripwire - Expected behavior that doesn't happen #338

Closed allChuu closed 9 months ago

allChuu commented 9 months ago

Minecraft Version: 1.20 / 1.20.1 / 1.20.3 & 1.20.4

Logs: {Link(s) to GitHub Gist with full latest.log and/or crash report} - No crashes or error

Steps to Reproduce:

  1. Place a tripwire
  2. Use shears to desactivate the tripwire
  3. Tripwire is desactivated - The hooks facing "middle" and doesnt activate when an entity touch it.

Description of issue: When Neoforge is installed and use, this vanilla minecraft mechanic doesnt work.

This is with NeoForge : https://streamable.com/1ixazs - Fresh install / absolutly no mods, no shaders, no saves, nothing except for Neoforge

This is the vanilla Minecraft behavior on the latest version : https://streamable.com/2ue347 - Same setup than above, fresh install

XFactHD commented 9 months ago

A quick code analysis points at this patch being at fault: https://github.com/neoforged/NeoForge/blob/fc75842a884cd65348146b0b809829ebc832aac4/patches/net/minecraft/server/level/ServerPlayerGameMode.java.patch#L43-L82 To be honest, that whole patch is a mess and I currently can't think of any way to make that not completely break stuff like this.

TelepathicGrunt commented 9 months ago

The ServerPlayerGameMode.java.patch is not the cause. I replaced the patched destroyBlock method with the original vanilla method and tripwire shearing is still broken. Which mean the issue lies elsewhere. Possible the level snapshot system? More testing is needed

Edit: not block snapshot

TelepathicGrunt commented 9 months ago

This Forge/Neo patch is the issue: https://github.com/search?q=repo%3Aneoforged%2FNeoForge%20MC-129055&type=code

It aimed to solve this vanilla bug: https://bugs.mojang.com/browse/MC-129055

The original patch came from: https://github.com/MinecraftForge/MinecraftForge/pull/7718

This patch might had worked in an older mc version? Seemed broken in 1.20.1. So at some point, the patch is causing more issues than it solves. Now how to fix the patch is the question or should we just yeet the patch and go back to vanilla behavior.

TelepathicGrunt commented 9 months ago

Ok if bedrock edition is correct, then left click with shears IS supposed to break the wire without triggering the tripewire hook. That's the mojang intended behavior. The patch was to move a misplaced line of code into an intended if statement. So this patch IS the correct behavior mojang wanted. Disabled state is not supposed to be obtainable in survival. @Matyrobbrt We can close this issue report. No bug here with the patch

X66Herobrine66X commented 5 months ago

This is NOT invalid. This is a bug and needs to be fixed. I don't know what @Shadows-of-Fire was thinking.

TelepathicGrunt commented 5 months ago

@X66Herobrine66X can you provide evidence of why it is a bug? Last I investigated and asked around and debated, all evidence pointed to shear left click to breaking the block without triggering wire as correct. Left click leaving wire in a disabled glitchy state was the bug

X66Herobrine66X commented 5 months ago

@X66Herobrine66X can you provide evidence of why it is a bug? Last I investigated and asked around and debated, all evidence pointed to shear left click to breaking the block without triggering wire as correct. Left click leaving wire in a disabled glitchy state was the bug

Isn't the fact that there's a disarmed block state at all evidence enough? Why would Mojang add such a thing and then make it impossible to obtain in survival. Additionally, I checked the latest snapshot of 1.20.4. I was able to disarm the tripwire as normal by left clicking. It did not break the block. Hence, intended feature. It's only in Forge that the bug occurs.

TelepathicGrunt commented 5 months ago

@X66Herobrine66X the block staying is the bug as confirmed by Mojang here https://bugs.mojang.com/browse/MC-129055

The blockstate is only for making sure change doesn’t propagate to the tripwire hook in the same tick iirc.

The shear is supposed to break the wire without triggering the hook.

No other items has an “interact-like” behavior on left click. If it was supposed to keep block but put it into disabled state, it would be shear right click. But since it is on left click, it should be a breaking action.

Java edition is supposed to be in parity with Bedrock editing as well and Bedrock breaks the wire.

And lastly, the code for the disabling is very clearly misplaced in the source code. There’s an if statement with literally nothing in it and the disabling right above it. Clearly the code was supposed to be in the if statement but was missed.

All evidence points to the vanilla behavior as the bug and NOT the intended result that Mojang wanted. The Forge/NeoForge behavior is how the Shears are supposed to work as according to Mojang themselves

heipiao233 commented 5 months ago

Why should a mod loader fix vanilla bugs? The players have their right to use the bug as the bug doesn't crash anything. If a server owner doesn't want the player to use this bug, he should install a mod that fixes the bug.

TelepathicGrunt commented 5 months ago

@heipiao233 Because Forge and NeoForge have a history of fixing bugs that either impact or cause issues for a lot of mods or are high impact to players. It also helps prevent multiple mods all trying to fix the same vanilla bug with coremods when the modloaders can easily do it in a patch and sometimes much more efficiently and safer than mods could do.

https://github.com/search?q=repo%3Aneoforged%2FNeoForge%20mc-&type=code

Some other modloaders may not have the same principles where they would consider fixing shears as off the table. But they will still fix stuff like removing modded dimensions will brick a world because the level.dat file saves the dimension's custom chunk generator/biome source info and explodes when trying to load that missing dimension. All modloaders patched that vanilla bug due to severity.

KnightMiner commented 5 months ago

Why should a mod loader fix vanilla bugs? The players have their right to use the bug as the bug doesn't crash anything. If a server owner doesn't want the player to use this bug, he should install a mod that fixes the bug.

Because the bug breaks mods. Thats why the fix was added.

I honestly don't see why you want the broken tripwire so badly, it was ridiculously buggy. After "disarming", the block only had a random chance of breaking when you tried removing it later, even with an empty hand. It was clearly completely broken.

If you insist on using the internal state, nothing stops you from writing a mod that adds it in a stable way instead of forcing modders who write custom shears variants to deal with the brokenness that is tripwire disarming in vanilla.