Feedback Can I get some feedback / help improving readability

It's an age-old story, I wrote this code n years ago. It works but looking at it now I'm having a tough time figuring out how to make it less confusing.

What it do: There are 9 chunks rendered at a time. The player character is always in the middle one. When the player character moves chunks it removes 3 chunks opposite to the direction of travel and creates 3 chunks in front of them.



I'm hoping someone can give feedback / advice on how I can simplify / tidy / improve this formula for getting the indexes of chunks being created / removed. Any suggestions on how I can make this look less confusing would be appreciated.

Code (CSharp):

1.        
2.         private void InputDevice_OnChunkChanged( Vector3Int newChunk,
3.                                                  Vector3Int lastChunk ) {
4.  
5.             Vector3Int dirOfTravel = newChunk - lastChunk;
6.  
7.             //create new chunks
8.             CreateChunk( new Vector3Int(
9.                 newChunk.x + ( dirOfTravel.x == 0 ? -1 : dirOfTravel.x ),
10.                 newChunk.y + ( dirOfTravel.y == 0 ? -1 : dirOfTravel.y ), 0) );
11.  
12.             CreateChunk( new Vector3Int(
13.                 newChunk.x + ( dirOfTravel.x == 0 ? +0 : dirOfTravel.x ),
14.                 newChunk.y + ( dirOfTravel.y == 0 ? +0 : dirOfTravel.y ), 0 ) );
15.  
16.             CreateChunk( new Vector3Int(
17.                 newChunk.x + ( dirOfTravel.x == 0 ? +1 : dirOfTravel.x ),
18.                 newChunk.y + ( dirOfTravel.y == 0 ? +1 : dirOfTravel.y ), 0 ) );
19.  
20.             //remove old chunks
21.             activeChunks.Remove( new Vector3Int(
22.                 lastChunk.x - ( dirOfTravel.x == 0 ? -1 : dirOfTravel.x ),
23.                 lastChunk.y - ( dirOfTravel.y == 0 ? -1 : dirOfTravel.y ), 0 ) );
24.  
25.             activeChunks.Remove( new Vector3Int(
26.                 lastChunk.x - ( dirOfTravel.x == 0 ? +0 : dirOfTravel.x ),
27.                 lastChunk.y - ( dirOfTravel.y == 0 ? +0 : dirOfTravel.y ), 0 ) );
28.  
29.             activeChunks.Remove( new Vector3Int(
30.                 lastChunk.x - ( dirOfTravel.x == 0 ? +1 : dirOfTravel.x ),
31.                 lastChunk.y - ( dirOfTravel.y == 0 ? +1 : dirOfTravel.y ), 0 ) );
32.         }

 

I would go with something like this:

Code (CSharp):

1.     private void InputDevice_OnChunkChanged(Vector2Int newChunk, Vector2Int lastChunk)
2.     {
3.         var dirOfTravel = newChunk - lastChunk;
4.  
5.         for(var i = -1; i <= 1; i++)
6.         {
7.             var dirTravelX = dirOfTravel.x == 0 ? i : dirOfTravel.x;
8.             var dirTravelY = dirOfTravel.y == 0 ? i : dirOfTravel.y;
9.          
10.             CreateChunk(new Vector2Int(newChunk.x + dirTravelX, newChunk.y + dirTravelY));
11.             activeChunks.Remove(new Vector2Int(lastChunk.x - dirTravelX, lastChunk.y - dirTravelY));
12.         }
13.     }

Caveat: if you can't use

Vector2Int

for some weird reason, or you're planning to implement vertical travel in the future, switch back to

Vector3Int

and put back the last

,0

-s in the two last lines. Also find a proper name for the variable

i

, I'm not sure what it is exactly, direction indicator or something? And finally since I don't know your choice of data structure, if you can't alternate the create new chunk and remove old chunk, then just duplicate the

for

loop and call

CreateChunk

in the first and call

Remove

in the second loop.

 

damn, can't believe I didn't think of that. I was trying to approach it from a different angle, but using a for loop is simple and sweet.

I recently changed from using meshes and mapping UVs to using TileMaps, and I don't like casting for WorldToCell and CellToWorld. But in light of your suggestion I swapped my Vector3Ints to Vector2Ints and found there's only 3 or 4 times I need to type cast and it's not called often.

Thanks for taking the time to reply.