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

Particle.onGround is not updated correctly #6595

Closed
Snownee opened this issue Mar 28, 2020 · 1 comment
Closed

Particle.onGround is not updated correctly #6595

Snownee opened this issue Mar 28, 2020 · 1 comment

Comments

@Snownee
Copy link

Snownee commented Mar 28, 2020

Minecraft Version: 1.15.2

Forge Version: 31.1.18

Description of issue:
Particle.onGround is not updated correctly, causing particles behave differently between vanilla and forge.

Vanilla:
Vanilla
Forge:
Forge

@Snownee Snownee added the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Mar 28, 2020
@FoxSamu
Copy link

FoxSamu commented Mar 30, 2020

This also happens to digging particles. They seem to collide but their velocities are not changed correctly. Possibly this bug is caused by decompiling as vanilla just works fine but the decompiled code acts pretty weird, and forge does not seem to touch net.minecraft.client.particle.Particle at all.

Code analysis. The bug is in the move(x, y, z) method of net.minecraft.client.particle.Particle:

   // MCP mappings: snapshot-20200322-1.15.1
   // Local variables are remapped, old names are commented between [brackets].
   public void move(double x, double y, double z) {
      if (!this.collidedY) {
         double lastX = x; // [d0]
         double lastY = y; // [d1]
         if (this.canCollide && (x != 0.0D || y != 0.0D || z != 0.0D)) {
            Vec3d moveVec = Entity.collideBoundingBoxHeuristically((Entity)null, new Vec3d(x, y, z), this.getBoundingBox(), this.world, ISelectionContext.dummy(), new ReuseableStream<>(Stream.empty())); // [vec3d]
            x = moveVec.x;
            y = moveVec.y;
            z = moveVec.z;
         }

         if (x != 0.0D || y != 0.0D || z != 0.0D) {
            this.setBoundingBox(this.getBoundingBox().offset(x, y, z));
            this.resetPositionToBB();
         }

         // 'a < b && a >= b' is equal to 'a == b'
         // This condition can be evaluated to: 'Math.abs(y) == (double)1.0E-5F'
         // The chance that this evaluates to true is 1 upon 9223372036854775808
         // For this to work properly, 'lastY' must more than/equal to approx. zero and 'y' must be less than approx. zero.
         if (Math.abs(y) >= (double)1.0E-5F && Math.abs(y) < (double)1.0E-5F) {
            this.collidedY = true;
         }

         // Wait: are we comparing variables to itself?
         //              vvvvvv This condition is always false, causing onGround to be always false!
         this.onGround = y != y && lastY < 0.0D;
         if (lastX != x) {
            this.motionX = 0.0D;
         }

         //  vvvvvv This condition is also always false!
         if (z != z) {
            this.motionZ = 0.0D;
         }
      }
   }

This method can be patched to solve the problem:

   public void move(double x, double y, double z) {
      if (!this.collidedY) {
         double lastX = x; // [d0]
         double lastY = y; // [d1]
         double lastZ = z; // < Create variable 'lastZ' and assign it to 'z'.
         if (this.canCollide && (x != 0.0D || y != 0.0D || z != 0.0D)) {
            Vec3d moveVec = Entity.collideBoundingBoxHeuristically((Entity)null, new Vec3d(x, y, z), this.getBoundingBox(), this.world, ISelectionContext.dummy(), new ReuseableStream<>(Stream.empty())); // [vec3d]
            x = moveVec.x;
            y = moveVec.y;
            z = moveVec.z;
         }

         if (x != 0.0D || y != 0.0D || z != 0.0D) {
            this.setBoundingBox(this.getBoundingBox().offset(x, y, z));
            this.resetPositionToBB();
         }

         //           vvvvv Use 'lastY' instead of 'y'.
         if (Math.abs(lastY) >= (double)1.0E-5F && Math.abs(y) < (double)1.0E-5F) {
            this.collidedY = true;
         }

         //              vvvvv Use 'lastY' instead of 'y'.
         this.onGround = lastY != y && lastY < 0.0D;
         if (lastX != x) {
            this.motionX = 0.0D;
         }

         //  vvvvv Use 'lastZ' instead of 'z'
         if (lastZ != z) {
            this.motionZ = 0.0D;
         }
      }
   }

@autoforge autoforge bot removed the Triage This request requires the active attention of the Triage Team. Requires labelling or reviews. label Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants