Implemented/Fixed Mine Door rotation#58
Conversation
Issue Warlander/DeedPlanner3 Warlander#7
Warlander
left a comment
There was a problem hiding this comment.
I think door direction rotation should be happening on graphics level only, without Ground model needing to be aware of it as we only really need it for rendering - and it shouldn't be a performance issue given we only need this info if something is changing with the ground.
| ground.data = newData; | ||
| ground.Tile.Map.Ground.SetGroundData(ground.Tile.X, ground.Tile.Y, ground.data, ground.RoadDirection); | ||
| ground.DoorDirection = ground.Tile.UpdateDoorDirection(); | ||
| ground.Tile.Map.Ground.SetGroundData(ground.Tile.X, ground.Tile.Y, ground.data, ground.RoadDirection, ground.DoorDirection); |
There was a problem hiding this comment.
Why do we need to serialize it?
| Tex2d = tex2d; | ||
| Tex3d = tex3d; | ||
| Diagonal = diagonal; | ||
| IsCaveDoor = ShortName is "wcaDoor" or "gcaDoor" or "scaDoor" or "mcaDoor"; |
There was a problem hiding this comment.
I would lean more towards optional XML tag here, similar to how openings and diagonal-enabled ground types are defined.
There was a problem hiding this comment.
As cave doors are the only ground types that can rotate, I used the "Cave door" category name to set it to true
| // Cave.Initialize(this, Database.DefaultCaveData); | ||
| } | ||
|
|
||
| public DoorDirection UpdateDoorDirection() |
There was a problem hiding this comment.
Update would indicate something within the tile changes but it's not the case, the door direction seems to be calculated here.
There was a problem hiding this comment.
Renamed it to CalculateDoorOrientation
| // Rotating vectors based on DoorDirection | ||
| if (dir == DoorDirection.E) | ||
| { | ||
| (v00, v10, v11, v01) = (v10, v11, v01, v00); |
There was a problem hiding this comment.
That's interesting syntax, honestly it makes lots of sense here! 👀
| DoorDirection doorDir = doorDirectionsArray[x, y]; | ||
| Vector2Int selfCoords = new Vector2Int(x, y); | ||
|
|
||
| int westSlot = vertexIndex; |
There was a problem hiding this comment.
I think syntax where these values are only initialized once would be easier to read here.
There was a problem hiding this comment.
In the newest commit I changed it a bit:
var (wOffset, nOffset, eOffset, sOffset) = (data.IsCaveDoor ? doorOrientation : DoorOrientation.N) switch
{
DoorOrientation.E => (9, 0, 3, 6),
DoorOrientation.S => (6, 9, 0, 3),
DoorOrientation.W => (3, 6, 9, 0),
_ => (0, 3, 6, 9),
};
int westSlot = vertexIndex + wOffset;
int northSlot = vertexIndex + nOffset;
int eastSlot = vertexIndex + eOffset;
int southSlot = vertexIndex + sOffset;
| private int[,] slopesArray; | ||
| private GroundData[,] dataArray; | ||
| private RoadDirection[,] directionsArray; | ||
| private DoorDirection[,] doorDirectionsArray; |
There was a problem hiding this comment.
Just as a sanity check, is it guaranteed to be correct initially? All door directions will be set to N by default.
There was a problem hiding this comment.
For cave doors it is guaranteed to be incorrect almost always initially. But then it should almost immediately correct itself as of the current version.
| private bool needsUvUpdate = false; | ||
|
|
||
| public void Initialize(int width, int height, OverlayMesh newOverlayMesh) | ||
| public void Initialize(Map map, int width, int height, OverlayMesh newOverlayMesh) |
There was a problem hiding this comment.
I'm not a fan of map knowing about GroundMesh and GroundMesh about the map, but this entire part of code needs later refactor anyway so I'm ok with it for now.
This PR is related to issue #7 .