Uploaded image for project: 'Minecraft'
  1. Minecraft
  2. MC-88101

Retracting blocks don't move/bounce entities correctly

    Details

    • Type: Bug
    • Status: Resolved
    • Resolution: Fixed
    • Affects Version/s: Minecraft 1.8.8, Minecraft 15w36b, Minecraft 15w36c, Minecraft 15w36d, Minecraft 15w37a
    • Fix Version/s: Minecraft 15w38a
    • Labels:
      None
    • Confirmation Status:
      Unconfirmed

      Description

      When blocks are retracted by a piston they don't move (or bounce in the case of slime blocks) entities.

      What I expected to happen was...:
      Blocks that are moved by pistons should move entities the same way no matter if retracting or extending.

      What actually happened was...:
      Retracting blocks barely move the entities in front of them. Also they move them in the wrong direction.

      Steps to Reproduce:
      (1 to 3 is just building up 2015-09-05_01.08.38.png)
      1. Place a sticky piston and power it.
      2. Place a slime block in front of it and a block/slime block on the side of that slime block. (Make sure the floor doesn't stick to the slime block)
      3. Summon/Place an armor stand in the spot where the block you just placed would be when the piston was retracted.
      4. Remove the power source and let the piston retract.
      5. See how the armor stand only moves a little bit and in the wrong direction (2015-09-05_01.08.45.png)

      Video (showing a similar way to reproduce the issue): https://youtu.be/ZkI3uU7hoBA?t=3m1s

      Fix

      The reason for the bug is that the piston tile entity is often making differences between extending and retracting pistons where they really just should work exactly the same with an inverted facing.
      (The only case where I believe that an actual difference is needed is for the piston arm, and that should be mostly (or even purely?) be rendering.)
      So I removed all dependencies on "this.extending" in the parts of code that is moving the entities and replaced the facing with the actually push direction of the piston.
      Here is the changed code. Or to make it easier to see the difference: https://github.com/Panda4994/Minecraft-Bugfixes/commit/dc46dd61eadc39729230cf6554c9adacd345ccdb#diff-20737173869e706ea6189fd5fe9f2e49 (I only posted the needed methods, hopefully the rest I clear from context)

      TileEntityPiston.java (in MCP)
      	...
      	/** ADDED:
      	 * Returns the actual direction the piston pushes in
      	 * 
      	 * @return	facing to the direction the piston pushes in
      	 */
      	public EnumFacing getPushFacing() {
      		if (this.extending) {
      			return this.field_174931_f;
      		} else {
      			return this.field_174931_f.getOpposite();
      		}
      	}
      	
      	// Method that moves the entities
      	private void func_145863_a(float p_145863_1_, float p_145863_2_) {
      		// Kept adjusting the first parameter as this was done for extending before as well
      		p_145863_1_ = 1.0F - p_145863_1_;
      		// Get actual push direction and replaced all calls of the facing in this method with this facing
      		// This way it will work exactly the same without having to mess with all the single values
      		EnumFacing pushFacing = getPushFacing();
      		
      		AxisAlignedBB var3 = Blocks.piston_extension.func_176424_a(this.worldObj, this.pos, this.field_174932_a, p_145863_1_, pushFacing);
      		if(var3 != null) {
      			List var4 = this.worldObj.getEntitiesWithinAABBExcludingEntity((Entity)null, var3);
      			 if(!var4.isEmpty()) {
      				this.field_174933_k.addAll(var4);
      					Iterator var5 = this.field_174933_k.iterator();
      
      					while(var5.hasNext()) {
      						Entity var6 = (Entity)var5.next();
      						// REMOVED: "&& this.extending", to make slime blocks bounce when the piston is retracting
      						if(this.field_174932_a.getBlock() == Blocks.slime_block) {
      							switch(TileEntityPiston.SwitchAxis.field_177248_a[pushFacing.getAxis().ordinal()]) {
      							case 1:
      								var6.motionX = (double)pushFacing.getFrontOffsetX();
      								break;
      							case 2:
      								var6.motionY = (double)pushFacing.getFrontOffsetY();
      								break;
      							case 3:
      								var6.motionZ = (double)pushFacing.getFrontOffsetZ();
      							}
      						}
      						// Made it move the entity also when it's a slime block. See MC-88100 for additional information about that.
      						var6.moveEntity((double)(p_145863_2_ * (float)pushFacing.getFrontOffsetX()), (double)(p_145863_2_ * (float)pushFacing.getFrontOffsetY()), (double)(p_145863_2_ * (float)pushFacing.getFrontOffsetZ()));
      					}
      
      					this.field_174933_k.clear();
      			}
      		}
      	}
      
      	...
      
      	public void update() {
      		this.lastProgress = this.progress;
      		if(this.lastProgress >= 1.0F) {
      			this.func_145863_a(1.0F, 0.25F);
      			this.worldObj.removeTileEntity(this.pos);
      			this.invalidate();
      			if(this.worldObj.getBlockState(this.pos).getBlock() == Blocks.piston_extension) {
      				this.worldObj.setBlockState(this.pos, this.field_174932_a, 3);
      				this.worldObj.notifyBlockOfStateChange(this.pos, this.field_174932_a.getBlock());
      			}
      		} else {
      			this.progress += 0.5F;
      			if(this.progress >= 1.0F) {
      				this.progress = 1.0F;
      			}
      		
      			// Call the movement method always, no matter if extending or retracting.
      			this.func_145863_a(this.progress, this.progress - this.lastProgress + 0.0625F);
      		}
      	}
      	...
      

      Link to the mentioned other post: MC-88100

      After changing this it worked better, but to my surprise it still didn't fully work. Looking around I found that the code to get the collision box of a moving block looks similar to what we just changed.
      So I also made that depended on the actual facing instead of messing with the values when the piston is retracting.
      Please note that the collision boxes (especially for moving blocks) need some more refactoring. Check MC-73302 for additional information.
      And the diff again: https://github.com/Panda4994/Minecraft-Bugfixes/commit/909b6c12204fc2f5d1d40d6918ebcbdd4f6e0d84#diff-8c85c5cb2ca98cf6d0f56ca85846ea55

      BlockPistonMoving.java (in MCP)
      	...
      	public AxisAlignedBB getCollisionBoundingBox(World worldIn, BlockPos pos, IBlockState state) {
      		TileEntityPiston var4 = this.func_176422_e(worldIn, pos);
      		if(var4 == null) {
      			return null;
      		} else {
      			float var5 = var4.func_145860_a(0.0F);
      			// Kept adjusting the first parameter as this was done for extending before as well
      			var5 = 1.0F - var5;
      
      			// Use actual push facing instead of facing instead of the if before (This way it for sure does the same for extending/retracting)
      			return this.func_176424_a(worldIn, pos, var4.func_174927_b(), var5, var4.getPushFacing());
      		}
      	}
      	...
      

      @Moderators

      This bug has been posted three times before: MC-75219, MC-74184 and MC-54954
      However all of these are quite old, less detailed and don't get updated to the current versions. So I thought I might just repost it with additional information and try to keep it updated

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                grum [Mojang] Grum (Erik Broes)
                Reporter:
                panda4994 Panda
              • Votes:
                18 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: