An add-on authors guide to the 4.ABCFEFE collision changes

This sub-forum is dedicated to add-ons and texture packs for Better Than Wolves.
Post Reply
User avatar
FlowerChild
Site Admin
Posts: 18753
Joined: Mon Jul 04, 2011 7:24 pm

An add-on authors guide to the 4.ABCFEFE collision changes

Post by FlowerChild »

Any add-on blocks are likely totally broken right now. DON'T PANIC! :)

Ok, first, a brief explanation of why this change was made to give some context to the madness:

Please read Item 1 ( Race Condition (Client vs. Server Thread) ) here: https://github.com/taurose/Unglitch

This is a real bitch and a half to resolve in a manner that separates collision data for both client and server without essentially having to modify every single vanilla block class in the game to adapt to the change.

In the end, I decided to leave the exist minX, maxX, etc. vanilla variables in the Block class intact so that vanilla references to those old vars would still compile and execute fine, without doing anything. This is why all your add-on blocks probably look like big 1m^3 cubes right now. On the bright side, this provides a clear indication that any given block with broken collisions needs to be refactored to fix them.

Secondly, I also wanted to prevent any future mistakes of the same kind that resulted in this whole mess from occurring so to that end, I decided on a system whereby instead of setting block bounds in internal variables as was done before, that the related functions would instead return temporary AxisAlignedBB objects, and that only the initial block bounds would be stored as a private variable that could never be modified post initialization. The following related code I've added to the Block class should be rather self-explanatory. I normally don't publish my code but I figured having the related code isolated, and to have access to my comments might be more useful than having to read through a deobfuscated mess:

Code: Select all

	//-------- Collision Handling Functionality -------//
    
    /**
     * This should never be modified after a block is initialized to avoid multithreading issues
     * that occurred with the old min/max bounds variables.
     */
    private AxisAlignedBB m_fixedBlockBounds = new AxisAlignedBB( 0D, 0D, 0D, 1D, 1D, 1D );
    private boolean m_bFixedBlockBoundsSet = false;
    
    /**
     * Should only ever be called once for a block.  Repeated calls will silently fail without
     * changing the bounds.
     */
    protected void InitBlockBounds( double dMinX, double dMinY, double dMinZ, 
    	double dMaxX, double dMaxY, double dMaxZ )
    {
    	if ( !m_bFixedBlockBoundsSet )
    	{
    		// only allow the bounds to be set before they're ever accessed to 
    		// discourage the kind of errors that necessitated it in the first 
    		// place: client and server threads modifying the min/max values 
    		// resulting in race conditions.
    		
    		m_fixedBlockBounds.setBounds( dMinX, dMinY, dMinZ, dMaxX, dMaxY, dMaxZ );    		
    	}
    }

    protected void InitBlockBounds( AxisAlignedBB bounds )
    {
    	if ( !m_bFixedBlockBoundsSet )
    	{
    		m_fixedBlockBounds.setBB( bounds );
    	}
    }
    
    protected AxisAlignedBB GetFixedBlockBoundsFromPool()
    {
		m_bFixedBlockBoundsSet = true;
		
    	return m_fixedBlockBounds.MakeTemporaryCopy();
    }

    public AxisAlignedBB getCollisionBoundingBoxFromPool( World world, int i, int j, int k )
    {
    	return GetBlockBoundsFromPoolBasedOnState( world, i, j, k ).offset( i, j, k );
    }
    
    public AxisAlignedBB GetBlockBoundsFromPoolBasedOnState( 
    	IBlockAccess blockAccess, int i, int j, int k )
    {
    	return GetFixedBlockBoundsFromPool();
    }
    
    public MovingObjectPosition collisionRayTrace( World world, int i, int j, int k, 
    	Vec3 startRay, Vec3 endRay )
    {
    	return CollisionRayTraceVsBlockBounds( world, i, j, k, startRay, endRay );
    }

    public MovingObjectPosition CollisionRayTraceVsBlockBounds( World world, int i, int j, int k, 
    	Vec3 startRay, Vec3 endRay )
    {
    	AxisAlignedBB collisionBox = GetBlockBoundsFromPoolBasedOnState( 
    		world, i, j, k ).offset( i, j, k );
    	
    	MovingObjectPosition collisionPoint = collisionBox.calculateIntercept( startRay, endRay );
    	
    	if ( collisionPoint != null )
    	{
    		collisionPoint.blockX = i;
    		collisionPoint.blockY = j;
    		collisionPoint.blockZ = k;
    	}
    	
    	return collisionPoint;
    }

    public AxisAlignedBB getSelectedBoundingBoxFromPool( World world, int i, int j, int k )
    {
    	return GetBlockBoundsFromPoolBasedOnState( world, i, j, k ).offset( i, j, k );
    }


    public AxisAlignedBB GetBlockBoundsFromPoolForItemRender( int iItemDamage )
    {
    	return GetFixedBlockBoundsFromPool();
    }
Now, I had essentially had to go through every block in the game and adapt it to this new system. This seriously sucked ass, taking dozens of hours overall. Adapting individual blocks really isn't that bad however, especially if it's your own code.

These are the steps I generally followed:

-Use InitBlockBounds() in your constructor to set what will be constant block bounds. This should replace any setBounds() calls that you previously had.

-If your block had a setBoundsBasedOnState() method, replace it instead with GetBlockBoundsFromPoolBasedOnState(). In many cases this is a very straightforward replacement of any setBounds() calls within that function with something akin to

Code: Select all

        	return AxisAlignedBB.getAABBPool().getAABB(         	
        		0D, 0D, 0.375D, 1D, 1D, 0.625D );
to return a temporary bounding object instead.

-In a lot of cases you can now completely delete any additional getCollisionBoundingBoxFromPool(), collisionRayTrace(), and getSelectedBoundingBoxFromPools() methods your block previously contained, as those methods now rely on common GetBlockBoundsFromPoolBasedOnState() functionality by default. In many cases, block code ends up being a lot simpler with a lot less code duplication as a result of these changes. Given it now also all relies on AxisAlignedBB objects instead of indivdual min/max doubles, you can leverage a lot of the functionality of that class to make things even simpler.

-Remove and replace any additional calls to setBlockBounds() that you may have within your block, as these won't do squat anymore.

-setBlockBoundsForItemRender() has been entirely deprecated and should be replaced with implementations of GetBlockBoundsFromPoolForItemRender().

-setBlockBoundsForItemRender() has been entirely deprecated and should be replaced with implementations of GetBlockBoundsFromPoolForItemRender().

-RenderBlocks.setRenderBoundsFromBlock() has also been deprecated, since it refers to the old vanilla block bounds, and calls to it should largely be replaced by stuff like:

Code: Select all

        rendererBlocks.setRenderBounds( GetBlockBoundsFromPoolBasedOnState( rendererBlocks.blockAccess, i, j, k ) );
I think that about covers it. If add-on authors have any questions, please fire away. I realize this is a rather radical refactoring and I'm happy to help make that easier.
User avatar
jorgebonafe
Posts: 2714
Joined: Mon Sep 19, 2011 3:22 am
Location: Brasil

Re: An add-on authors guide to the 4.ABCFEFE collision chang

Post by jorgebonafe »

FlowerChild wrote:How did the transition go Jorge? Was my writeup helpful? Sorry to derail your thread. Feel free to respond over there.
It wasn't difficult at all. But all my blocks were quite simple.

The astrolabe blocks had a default block collision box, I just added InitBlockBounds to the constructor.

The arrow marker doesn't have any collision, but it's shaped like a pressure plate. I had some code to draw the arrow in the correct direction, which I'll post down here, and for some reason, the block showed as a full cube instead of the flat shape it should have been.
Spoiler
Show

Code: Select all

public boolean RenderBlock(RenderBlocks var1, int var2, int var3, int var4) {
  super.RenderBlock(var1, var2, var3, var4);
  IBlockAccess var5 = var1.blockAccess;
	

  int metadata = var5.getBlockMetadata(var2, var3, var4);
  int facing = GetFacingFromMetadata(metadata);
  
  if (facing == 2) {
  	var1.SetUvRotateTop(3);
  } else if (facing == 4) {
  	var1.SetUvRotateTop(1);
  } else if (facing == 5) {
  	var1.SetUvRotateTop(2);
  }
  
  var1.renderFaceYPos(this, (double) var2, (double) var3, (double) var4, this.blockIcon);
  var1.setRenderBoundsFromBlock(this);
  var1.renderStandardBlock(this, var2, var3, var4);  
  var1.ClearUvRotation();

  return true;
}
The problem was on the code var1.setRenderBoundsFromBlock(this); , I removed this line and everything worked out.
I'll admit a lot of the rendering (and just minecraft) code in general confuses me, and I'm not sure why this line was needed in the first place, or if removing it might cause some trouble in the future... I'm just hoping for the best at this point :)
Better Than Wolves was borne of anal sex. True Story.
User avatar
FlowerChild
Site Admin
Posts: 18753
Joined: Mon Jul 04, 2011 7:24 pm

Re: An add-on authors guide to the 4.ABCFEFE collision chang

Post by FlowerChild »

Thanks for the write up Jorge!
jorgebonafe wrote: The problem was on the code var1.setRenderBoundsFromBlock(this); , I removed this line and everything worked out.
Ah, yes, I neglected to mention that one in the OP. The "FromBlock" that method is referencing, are the old vanilla bounds. You'd probably be better off replacing it with something along the lines of:

Code: Select all

        renderer.setRenderBounds( GetBlockBoundsFromPoolBasedOnState( 
        	renderer.blockAccess, i, j, k ) );
Or you might end up with visual glitches if something else is setting the render bounds before your block, and you just have removed the call entirely.

Actually, you just made me realize I'm still calling that method myself in a number of blocks, but they're all full 1m cubes, so it's not noticeable. Will change that over and add a note to the OP about this.

Thanks man!
User avatar
jorgebonafe
Posts: 2714
Joined: Mon Sep 19, 2011 3:22 am
Location: Brasil

Re: An add-on authors guide to the 4.ABCFEFE collision chang

Post by jorgebonafe »

All right, thanks for the tip, I'll change this and post a new version.
And also thanks for the information on this thread. Yes, it was very helpful :)
Better Than Wolves was borne of anal sex. True Story.
User avatar
FlowerChild
Site Admin
Posts: 18753
Joined: Mon Jul 04, 2011 7:24 pm

Re: An add-on authors guide to the 4.ABCFEFE collision chang

Post by FlowerChild »

jorgebonafe wrote: And also thanks for the information on this thread. Yes, it was very helpful :)
You're most welcome man. I felt really bad doing this to you guys (Yhetti in particular with all the blocks in Deco), but after thinking about it a long time there's was really no way to fix this for good without breaking everything first.

If it's any consolation, I must have clocked in somewhere over 60-80 hours converting all the blocks in the game to the new format :P
Post Reply