MCGalaxy icon indicating copy to clipboard operation
MCGalaxy copied to clipboard

TNT can turn a custom block into a Physics block.

Open rdebath opened this issue 2 years ago • 3 comments

Examples seen include Block 80 turned into Cobblestone door and block 73 turned into FHL. Block must not have "lavakills" set. It appears that the TNT explosion makes the block fall and when it stops falling it's changed into a physics block with the same number.

Happens with physics is set to "4". (all of 2..4)

rdebath avatar Oct 28 '21 18:10 rdebath

This is caused by two bad interactions with the ExtBlock field in PhysicsArgs struct

  1. in ExtraInfoPhysics.cs, DoDrop calls lvl.AddUpdate(index, C.Block, C.data) However, unlike AddUpdate(int index, BlockID block) method, AddUpdate(int index, BlockID block, PhysicsArgs data) does not automatically set the ExtBlock bits to the appropriate values based on block argument
  2. in Level.Physics.cs, when PhysicsTick is calculating the block ID to send to client for physics block updates, it ignores the ExtBlock bits if the physics update entry has one or more types set (and C.Data does)

Unfortunately ExtBlock behaves in this terrible way because it's also (ab?)used for other purposes, e.g. see RevertPhysics in Level.Physics.cs

UnknownShadow200 avatar Oct 29 '21 00:10 UnknownShadow200

Well, I'd suggest that we don't try to drop/dissipate custom blocks then. Looks like if it's a custom block at this line update to air or use CoalOre (ie: a safe block) as a substitute.

Though, as far as I can see the only normal Type that puts a block in a Value is revert so it should be safe to use it as long as neither Type is Revert or Custom.

I've also gone through the other calls to AddUpdate, the only call that looks dubious to me is the dissipate one; which doesn't seem to be using the value, though IMO it should use it so that it doesn't dissipate a random block that the player has later placed. There are quite a few places that use default(PhysicsArgs) unnecessarily but they seem to all use a safe constant or a nearby fixup.

One other thing this line args.Raw |= (uint)(PhysicsArgs.ExtBit * (block >> Block.ExtendedShift)); Looks wrong for "10bit blocks"

rdebath avatar Oct 30 '21 15:10 rdebath

After getting a reliable triggering for this issue I can reliably say that the below seems to prevent it ever happening. The replacement by TNT_Nuke does happen correctly (and is very obvious when it does!). NB: The real replacement should be something like Block.Obsidian as this one can trigger explosion chains.

commit dd9439224450ae477ac8e423cbb949d0d634c8f5 (HEAD -> devl)
Author: Robert de Bath <[email protected]>
Date:   Fri Nov 26 19:33:10 2021 +0000

    Prevent Custom blocks turning into physics blocks

diff --git a/MCGalaxy/Levels/Level.Physics.cs b/MCGalaxy/Levels/Level.Physics.cs
index 31e1ef7fe..6fdd0c3c2 100644
--- a/MCGalaxy/Levels/Level.Physics.cs
+++ b/MCGalaxy/Levels/Level.Physics.cs
@@ -247,7 +247,7 @@ namespace MCGalaxy {
         internal bool AddUpdate(int index, BlockID block, bool overRide = false) {
             PhysicsArgs args = default(PhysicsArgs);
             args.Raw |= (uint)(PhysicsArgs.ExtBit * (block >> Block.ExtendedShift));
-            return AddUpdate(index, block, args, overRide);
+            return AddUpdate(index, (byte)block, args, overRide);
         }

         internal bool AddUpdate(int index, BlockID block, PhysicsArgs data, bool overRide = false) {
@@ -274,7 +274,10 @@ namespace MCGalaxy {
                     return false;
                 }

-                data.Data = (byte)block;
+                if ((byte)block != block)
+                    data.Data = Block.TNT_Nuke;
+                else
+                    data.Data = (byte)block;
                 Update update; update.Index = index; update.data = data;
                 ListUpdate.Add(update);

rdebath avatar Nov 26 '21 21:11 rdebath