At long last... Piston animations!

* Fixes #3198
* Fixes #57 (again lol)

src/Blocks/BlockPiston.cpp

@@ -214,36 +214,58 @@ bool cBlockPistonHandler::CanPushBlock(
 
 void cBlockPistonHandler::ExtendPiston(Vector3i a_BlockPos, cWorld & a_World)
 {
-	BLOCKTYPE pistonBlock;
-	NIBBLETYPE pistonMeta;
-	a_World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
-
-	if (IsExtended(pistonMeta))
 	{
-		// Already extended, bail out
+		// Broadcast block action first. Will do nothing if piston cannot in fact push
-		return;
+
+		BLOCKTYPE pistonBlock;
+		NIBBLETYPE pistonMeta;
+		a_World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
+		a_World.BroadcastBlockAction(a_BlockPos, PistonExtendAction, pistonMeta, pistonBlock);
 	}
 
-	Vector3i pushDir = MetadataToOffset(pistonMeta);
+	// Client expects the server to "play" the animation before setting the final blocks
+	// However, we don't confuse animation with the underlying state of the world, so emulate by delaying 1 tick
+	// (Probably why vanilla has so many dupe glitches with sand and pistons lolol)
 
-	Vector3iSet blocksPushed;
+	a_World.ScheduleTask(1, [a_BlockPos](cWorld & World)
-	if (!CanPushBlock(a_BlockPos + pushDir, a_World, true, blocksPushed, pushDir))
+		{
-	{
+			BLOCKTYPE pistonBlock;
-		// Can't push anything, bail out
+			NIBBLETYPE pistonMeta;
-		return;
+			World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
-	}
 
-	a_World.BroadcastBlockAction(a_BlockPos, 0, pistonMeta, pistonBlock);
+			if ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock))
-	a_World.BroadcastSoundEffect("block.piston.extend", a_BlockPos, 0.5f, 0.7f);
+			{
+				// Ensure we operate on a piston to avoid spurious behaviour
+				// Note that the scheduled task may result in the block type of a_BlockPos changing
+				return;
+			}
 
-	PushBlocks(blocksPushed, a_World, pushDir);
+			if (IsExtended(pistonMeta))
+			{
+				// Already extended, bail out
+				return;
+			}
 
-	// Set the extension and the piston base correctly
+			Vector3i pushDir = MetadataToOffset(pistonMeta);
-	Vector3i extensionPos = a_BlockPos + pushDir;
+			Vector3iSet blocksPushed;
-	a_World.SetBlock(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta | 0x8);
+			if (!CanPushBlock(a_BlockPos + pushDir, World, true, blocksPushed, pushDir))
-	a_World.SetBlock(
+			{
-		extensionPos.x, extensionPos.y, extensionPos.z,
+				// Can't push anything, bail out
-		E_BLOCK_PISTON_EXTENSION, pistonMeta | (IsSticky(pistonBlock) ? 8 : 0)
+				return;
+			}
+			PushBlocks(blocksPushed, World, pushDir);
+
+			// Set the extension and the piston base correctly
+			Vector3i extensionPos = a_BlockPos + pushDir;
+			World.SetBlock(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta | 0x8);
+			World.SetBlock(
+				extensionPos.x, extensionPos.y, extensionPos.z,
+				E_BLOCK_PISTON_EXTENSION, pistonMeta | (IsSticky(pistonBlock) ? 8 : 0)
+			);
+
+			// Play sound effect only if extended successfully
+			World.BroadcastSoundEffect("block.piston.extend", a_BlockPos, 0.5f, 0.7f);
+		}
 	);
 }
 
@@ -253,52 +275,70 @@ void cBlockPistonHandler::ExtendPiston(Vector3i a_BlockPos, cWorld & a_World)
 
 void cBlockPistonHandler::RetractPiston(Vector3i a_BlockPos, cWorld & a_World)
 {
-	BLOCKTYPE pistonBlock;
-	NIBBLETYPE pistonMeta;
-	a_World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
-
-	if (!IsExtended(pistonMeta))
 	{
-		// Already retracted, bail out
+		BLOCKTYPE pistonBlock;
-		return;
+		NIBBLETYPE pistonMeta;
+		a_World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
+		a_World.BroadcastBlockAction(a_BlockPos, PistonRetractAction, pistonMeta, pistonBlock);
 	}
 
-	Vector3i pushDir = MetadataToOffset(pistonMeta);
+	a_World.ScheduleTask(1, [a_BlockPos](cWorld & World)
+		{
+			BLOCKTYPE pistonBlock;
+			NIBBLETYPE pistonMeta;
+			World.GetBlockTypeMeta(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta);
 
-	// Check the extension:
+			if ((pistonBlock != E_BLOCK_PISTON) && !IsSticky(pistonBlock))
-	Vector3i extensionPos = a_BlockPos + pushDir;
+			{
-	if (a_World.GetBlock(extensionPos) != E_BLOCK_PISTON_EXTENSION)
+				// Ensure we operate on a piston to avoid spurious behaviour
-	{
+				// Note that the scheduled task may result in the block type of a_BlockPos changing
-		LOGD("%s: Piston without an extension - still extending, or just in an invalid state?", __FUNCTION__);
+				return;
-		return;
+			}
-	}
 
-	// Remove Extension
+			if (!IsExtended(pistonMeta))
-	a_World.SetBlock(extensionPos.x, extensionPos.y, extensionPos.z, E_BLOCK_AIR, 0);
+			{
+				// Already retracted, bail out
+				return;
+			}
 
-	a_World.SetBlock(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta & ~(8));
+			Vector3i pushDir = MetadataToOffset(pistonMeta);
-	a_World.BroadcastBlockAction(a_BlockPos, 1, pistonMeta & ~(8), pistonBlock);
-	a_World.BroadcastSoundEffect("block.piston.contract", a_BlockPos, 0.5f, 0.7f);
 
-	if (!IsSticky(pistonBlock))
+			// Check the extension:
-	{
+			Vector3i extensionPos = a_BlockPos + pushDir;
-		// No need for block pulling, bail out
+			if (World.GetBlock(extensionPos) != E_BLOCK_PISTON_EXTENSION)
-		return;
+			{
-	}
+				LOGD("%s: Piston without an extension - still extending, or just in an invalid state?", __FUNCTION__);
+				return;
+			}
 
-	// Get the block to pull
+			// Remove extension, update base state
-	a_BlockPos += pushDir * 2;
+			World.SetBlock(extensionPos.x, extensionPos.y, extensionPos.z, E_BLOCK_AIR, 0);
-	// Try to "push" the pulling block in the opposite direction
+			World.SetBlock(a_BlockPos.x, a_BlockPos.y, a_BlockPos.z, pistonBlock, pistonMeta & ~(8));
-	pushDir *= -1;
 
-	Vector3iSet pushedBlocks;
+			// (Retraction is always successful, but play in the task for consistency)
-	if (!CanPushBlock(a_BlockPos, a_World, false, pushedBlocks, pushDir))
+			World.BroadcastSoundEffect("block.piston.contract", a_BlockPos, 0.5f, 0.7f);
-	{
-		// Not pushable, bail out
-		return;
-	}
 
-	PushBlocks(pushedBlocks, a_World, pushDir);
+			if (!IsSticky(pistonBlock))
+			{
+				// No need for block pulling, bail out
+				return;
+			}
+
+			// Get the block to pull
+			Vector3i AdjustedPosition = a_BlockPos + pushDir * 2;
+			// Try to "push" the pulling block in the opposite direction
+			pushDir *= -1;
+
+			Vector3iSet pushedBlocks;
+			if (!CanPushBlock(AdjustedPosition, World, false, pushedBlocks, pushDir))
+			{
+				// Not pushable, bail out
+				return;
+			}
+
+			PushBlocks(pushedBlocks, World, pushDir);
+		}
+	);
 }
 
 

src/Blocks/BlockPiston.h

@@ -100,6 +100,12 @@ private:
 
 	typedef std::unordered_set<Vector3i, VectorHasher<int>> Vector3iSet;
 
+	/** Piston extension block action */
+	static const Byte PistonExtendAction = 0U;
+
+	/** Piston retraction block action */
+	static const Byte PistonRetractAction = 1U;
+
 	/** Returns true if the piston (specified by blocktype) is a sticky piston */
 	static inline bool IsSticky(BLOCKTYPE a_BlockType) { return (a_BlockType == E_BLOCK_STICKY_PISTON); }
 

src/Simulator/IncrementalRedstoneSimulator/PistonHandler.h

@@ -36,39 +36,25 @@ public:
 	virtual cVector3iArray Update(cWorld & a_World, Vector3i a_Position, BLOCKTYPE a_BlockType, NIBBLETYPE a_Meta, PoweringData a_PoweringData) const override
 	{
 		// LOGD("Evaluating pisty the piston (%d %d %d)", a_Position.x, a_Position.y, a_Position.z);
-		auto Data = static_cast<cIncrementalRedstoneSimulator *>(a_World.GetRedstoneSimulator())->GetChunkData();
-		auto DelayInfo = Data->GetMechanismDelayInfo(a_Position);
 
-		// Delay is used here to prevent an infinite loop (#3168)
+		bool ShouldBeExtended = (a_PoweringData.PowerLevel != 0);
-		if (DelayInfo == nullptr)
+		if (ShouldBeExtended == cBlockPistonHandler::IsExtended(a_Meta))
 		{
-			bool ShouldBeExtended = (a_PoweringData.PowerLevel != 0);
+			return {};
-			if (ShouldBeExtended != cBlockPistonHandler::IsExtended(a_Meta))
+		}
-			{
+
-				Data->m_MechanismDelays[a_Position] = std::make_pair(1, ShouldBeExtended);
+		if (ShouldBeExtended)
-			}
+		{
+			cBlockPistonHandler::ExtendPiston(a_Position, a_World);
 		}
 		else
 		{
-			int DelayTicks;
+			cBlockPistonHandler::RetractPiston(a_Position, a_World);
-			bool ShouldBeExtended;
-			std::tie(DelayTicks, ShouldBeExtended) = *DelayInfo;
-
-			if (DelayTicks == 0)
-			{
-				if (ShouldBeExtended)
-				{
-					cBlockPistonHandler::ExtendPiston(a_Position, a_World);
-				}
-				else
-				{
-					cBlockPistonHandler::RetractPiston(a_Position, a_World);
-				}
-
-				Data->m_MechanismDelays.erase(a_Position);
-			}
 		}
 
+		// It is necessary to delay after a signal to prevent an infinite loop (#3168)
+		// However, that is present as a side effect of the implementation of piston animation in Blocks\BlockPiston.cpp
+
 		return {};
 	}
 

src/Simulator/IncrementalRedstoneSimulator/RedstoneSimulatorChunkData.h

@@ -60,7 +60,6 @@ public:
 
 	/** Structure storing position of mechanism + it's delay ticks (countdown) & if to power on */
 	std::unordered_map<Vector3i, std::pair<int, bool>, VectorHasher<int>> m_MechanismDelays;
-	std::unordered_map<Vector3i, bool, VectorHasher<int>> m_UpdateOncePositions;
 
 private: