search

KosmosPrime / OpenComputers

Home of the OpenComputers mod for Minecraft.
https://oc.cil.li
Other
34 stars 15 forks source link

robot.swing() doesn't work #12

Closed bobozee closed 5 months ago

bobozee commented 5 months ago

Problem: Calling robot.swing() will always return false, no matter energy level, equipped tool, selected item or block in front of the robot.

Steps to reproduce:

  1. Create a fresh installation of minecraft 1.16.5 with forge 36.2.24 and run it with java 8
  2. Place a robot, place a block in front of it and give it an appropriate tool, run lua and then robot.swing().
  3. The robot returns false no matter what.

No matter if called through the lua interpreter or through a standalone program, robot.swing() never works.

bobozee commented 5 months ago

I debugged a bit, seems to be a deobfuscation fail, which makes sense porting between version.

On main/scala/server/agent/Player line 454, a onBlockClicked() is called on PlayerInteractionManagerHelper, which (in isDestroyingBlock()) uses a ObfuscationReflectionHelper to find the value of a player's gamemode. It never manages to find the field with the obfuscated code given (probably changed between versions), and resorts to the catch of the try block, which returns true. This in turn triggers the condition on line 454 in the Player class, and the actual breakTime is never sent, instead 1/20 (for air) or 0 (for anything non-air). Anything that returns breakTime 0 causes robot.swing() to fail, as defined in line 124 in the trait Agent.

Will try to fix soon and post results.

bobozee commented 5 months ago

(By the way I really advise to send error messages when catching exceptions)

KosmosPrime commented 5 months ago

[...] uses a ObfuscationReflectionHelper to find the value of a player's gamemode. It never manages to find the field with the obfuscated code given (probably changed between versions), and resorts to the catch of the try block, which returns true.

I don't know how you've managed to get that to throw an exception. The actual field access, which could fail due to a name change, happens during class init and failure should still crash because that exception is never caught, and gets turned into a ExceptionInInitializerError making the whole class unusable. I think ObfuscationReflectionHelper is legacy code as access transformers are checked at compile time, but kept using it to avoid changing too much.

(By the way I really advise to send error messages when catching exceptions)

It's not me, it's them.

In spite of that I can reproduce the bug, it's much more mundane though. The final parameter of PlayerInteractionManager.handleBlockBreakAction is the build height, trying to dig a block at or above that will fail. For some reasons I had left it at zero.

bobozee commented 5 months ago

Oh hey, I'm very glad you responded so quickly! I suppose I mistook the flow of the step-by-step debugger. I've never really modded any minecraft, this is my first time diving into source code. Though I'm still glad my findings quickly pointed you to the faulty area :) I'm very happy that you managed to find this issue and fix it swiftly. Def. gets a star from me ^^

Edit: Even posted a new release with the bugfix, wow, extra kudos!