Skip to content
Draft
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 210 additions & 0 deletions src/c#/main/entity/TreeModel.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
using UnityEngine;

namespace beyondnations {

/// <summary>
/// Creates a procedural 3D tree model.
/// This replaces the simple primitive shapes (Cylinder + Cube) with a proper mesh-based tree.
/// Can be easily replaced with an imported 3D model file when available.
/// </summary>
public class TreeModel {

public static GameObject CreateTree(Vector3 position, int height, string name = "Tree") {
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CreateTree method doesn't validate the height parameter. If height is 0 or negative, it will create malformed meshes (e.g., cylinder with height <= 0, leaves positioned at negative or zero Y). Consider adding validation to ensure height is positive (e.g., height > 0) and throwing an ArgumentException or clamping to a minimum value like 1.

Suggested change
public static GameObject CreateTree(Vector3 position, int height, string name = "Tree") {
public static GameObject CreateTree(Vector3 position, int height, string name = "Tree") {
if (height <= 0) {
height = 1;
}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added height validation that clamps to minimum of 1 and logs a debug warning when invalid input is provided. (commit 40efa9d)

GameObject treeRoot = new GameObject(name);
treeRoot.transform.position = position;

// Create trunk
GameObject trunk = CreateTrunk(height);
trunk.transform.parent = treeRoot.transform;
trunk.transform.localPosition = Vector3.zero;

// Create leaves/canopy
GameObject leaves = CreateLeaves(height);
leaves.transform.parent = treeRoot.transform;
leaves.transform.localPosition = new Vector3(0, height - 1, 0);

return treeRoot;
Comment on lines +13 to +34
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The procedurally generated meshes are not explicitly destroyed when the tree GameObject is destroyed. While Unity's garbage collector will eventually clean them up, it's best practice to explicitly destroy mesh resources in the destroyGameObject method to prevent memory leaks, especially if many trees are created and destroyed frequently. Consider storing references to the meshes and calling Object.Destroy(mesh) in AppleTree.destroyGameObject().

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a DestroyTree() method that explicitly destroys mesh resources before destroying the GameObject. Updated AppleTree.destroyGameObject() to use this method. (commit 40efa9d)

}

private static GameObject CreateTrunk(int height) {
GameObject trunk = new GameObject("Trunk");

MeshFilter meshFilter = trunk.AddComponent<MeshFilter>();
MeshRenderer meshRenderer = trunk.AddComponent<MeshRenderer>();

// Create a cylindrical trunk mesh
Mesh trunkMesh = CreateCylinderMesh(0.5f, height, 8);
meshFilter.mesh = trunkMesh;

// Set brown bark material (matching original color)
Material trunkMaterial = new Material(Shader.Find("Standard"));
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Shader.Find("Standard") may return null if the Standard shader is not available in the project (e.g., if using a different render pipeline like URP or HDRP). Consider adding a null check and fallback, or using a more universally available shader like "Unlit/Color" or handling the null case gracefully.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added shader fallback logic with multiple options: Standard → Unlit/Color → Diffuse. Extracted to GetDefaultShader() helper method to avoid duplication. (commit 40efa9d)

trunkMaterial.color = new Color(0.5f, 0.25f, 0);
meshRenderer.material = trunkMaterial;

return trunk;
}

private static GameObject CreateLeaves(int height) {
GameObject leaves = new GameObject("Leaves");

MeshFilter meshFilter = leaves.AddComponent<MeshFilter>();
MeshRenderer meshRenderer = leaves.AddComponent<MeshRenderer>();

// Create a spherical canopy mesh
Mesh leavesMesh = CreateSphereMesh(2.5f, 10, 10);
meshFilter.mesh = leavesMesh;

// Set green foliage material (matching original color)
Material leavesMaterial = new Material(Shader.Find("Standard"));
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using Shader.Find("Standard") may return null if the Standard shader is not available in the project (e.g., if using a different render pipeline like URP or HDRP). Consider adding a null check and fallback, or using a more universally available shader like "Unlit/Color" or handling the null case gracefully.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added shader fallback logic with multiple options: Standard → Unlit/Color → Diffuse. Extracted to GetDefaultShader() helper method to avoid duplication. (commit 40efa9d)

leavesMaterial.color = Color.green;
meshRenderer.material = leavesMaterial;

return leaves;
}

/// <summary>
/// Creates a cylindrical mesh for the trunk
/// </summary>
private static Mesh CreateCylinderMesh(float radius, float height, int segments) {
Mesh mesh = new Mesh();
mesh.name = "CylinderMesh";

int vertexCount = segments * 2 + 2; // Top and bottom circles plus centers
Vector3[] vertices = new Vector3[vertexCount];
Vector3[] normals = new Vector3[vertexCount];
Vector2[] uvs = new Vector2[vertexCount];

// Create vertices
float angleStep = 360f / segments * Mathf.Deg2Rad;

// Bottom circle
for (int i = 0; i < segments; i++) {
float angle = i * angleStep;
float x = Mathf.Cos(angle) * radius;
float z = Mathf.Sin(angle) * radius;
vertices[i] = new Vector3(x, 0, z);
normals[i] = new Vector3(x, 0, z).normalized;
uvs[i] = new Vector2((float)i / segments, 0);
}

// Top circle
for (int i = 0; i < segments; i++) {
float angle = i * angleStep;
float x = Mathf.Cos(angle) * radius * 0.8f; // Slightly tapered
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trunk is now tapered (top radius is 0.8x the bottom radius), which differs from the original uniform cylinder primitive. While this may be a visual improvement, it represents a change in appearance from the existing implementation. Consider documenting this change or making the taper configurable.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added documentation in the class summary noting that "the trunk has a slight taper (top is 0.8x the bottom radius) for more realistic appearance." (commit 40efa9d)

float z = Mathf.Sin(angle) * radius * 0.8f;
vertices[segments + i] = new Vector3(x, height, z);
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trunk mesh extends from y=0 to y=height, but the original primitive cylinder was centered and extended from y=-height to y=+height (due to the default cylinder having height 2 and being scaled by height in the Y axis). This changes the trunk's vertical positioning - it's now bottom-aligned instead of center-aligned at the tree's root position. The leaves will also be positioned differently relative to the trunk.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the trunk positioning to match Unity's primitive cylinder behavior. The mesh now extends from -height/2 to +height/2, and the trunk is positioned at localPosition (0, height/2, 0) to center it at the tree's root position, matching the original implementation. (commit 40efa9d)

normals[segments + i] = new Vector3(x, 0, z).normalized;
uvs[segments + i] = new Vector2((float)i / segments, 1);
}

// Center points for caps
vertices[segments * 2] = new Vector3(0, 0, 0); // Bottom center
vertices[segments * 2 + 1] = new Vector3(0, height, 0); // Top center
normals[segments * 2] = Vector3.down;
normals[segments * 2 + 1] = Vector3.up;
uvs[segments * 2] = new Vector2(0.5f, 0.5f);
uvs[segments * 2 + 1] = new Vector2(0.5f, 0.5f);

// Create triangles
int[] triangles = new int[segments * 12];
int triIndex = 0;

// Side triangles
for (int i = 0; i < segments; i++) {
int next = (i + 1) % segments;

// First triangle
triangles[triIndex++] = i;
triangles[triIndex++] = segments + i;
triangles[triIndex++] = next;

// Second triangle
triangles[triIndex++] = next;
triangles[triIndex++] = segments + i;
triangles[triIndex++] = segments + next;
}

// Bottom cap
for (int i = 0; i < segments; i++) {
int next = (i + 1) % segments;
triangles[triIndex++] = segments * 2;
triangles[triIndex++] = next;
triangles[triIndex++] = i;
}

// Top cap
for (int i = 0; i < segments; i++) {
int next = (i + 1) % segments;
triangles[triIndex++] = segments * 2 + 1;
triangles[triIndex++] = segments + i;
triangles[triIndex++] = segments + next;
}

mesh.vertices = vertices;
mesh.normals = normals;
mesh.uv = uvs;
mesh.triangles = triangles;

mesh.RecalculateBounds();
return mesh;
}

/// <summary>
/// Creates a spherical mesh for the leaves
/// </summary>
private static Mesh CreateSphereMesh(float radius, int latitudeSegments, int longitudeSegments) {
Mesh mesh = new Mesh();
mesh.name = "SphereMesh";

int vertexCount = (latitudeSegments + 1) * (longitudeSegments + 1);
Vector3[] vertices = new Vector3[vertexCount];
Vector3[] normals = new Vector3[vertexCount];
Vector2[] uvs = new Vector2[vertexCount];

int vertIndex = 0;
for (int lat = 0; lat <= latitudeSegments; lat++) {
float theta = lat * Mathf.PI / latitudeSegments;
float sinTheta = Mathf.Sin(theta);
float cosTheta = Mathf.Cos(theta);

for (int lon = 0; lon <= longitudeSegments; lon++) {
float phi = lon * 2 * Mathf.PI / longitudeSegments;
float sinPhi = Mathf.Sin(phi);
float cosPhi = Mathf.Cos(phi);

Vector3 normal = new Vector3(cosPhi * sinTheta, cosTheta, sinPhi * sinTheta);
vertices[vertIndex] = normal * radius;
normals[vertIndex] = normal;
uvs[vertIndex] = new Vector2((float)lon / longitudeSegments, (float)lat / latitudeSegments);
vertIndex++;
}
}

int[] triangles = new int[latitudeSegments * longitudeSegments * 6];
int triIndex = 0;

for (int lat = 0; lat < latitudeSegments; lat++) {
for (int lon = 0; lon < longitudeSegments; lon++) {
int current = lat * (longitudeSegments + 1) + lon;
int next = current + longitudeSegments + 1;

triangles[triIndex++] = current;
triangles[triIndex++] = next;
triangles[triIndex++] = current + 1;

triangles[triIndex++] = current + 1;
triangles[triIndex++] = next;
triangles[triIndex++] = next + 1;
}
}

mesh.vertices = vertices;
mesh.normals = normals;
mesh.uv = uvs;
mesh.triangles = triangles;

mesh.RecalculateBounds();
return mesh;
}
}
}
11 changes: 11 additions & 0 deletions src/c#/main/entity/TreeModel.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 2 additions & 21 deletions src/c#/main/entity/entities/AppleTree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
namespace beyondnations {

public class AppleTree : Entity {
private GameObject trunk;
private GameObject leaves;
private int height;

public AppleTree(Vector3 position, int height) : base(EntityType.TREE, "Tree") {
Expand All @@ -14,25 +12,8 @@ public AppleTree(Vector3 position, int height) : base(EntityType.TREE, "Tree") {
}

public override void createGameObject(Vector3 position) {
GameObject gameObject = new GameObject();
gameObject.transform.position = position;
gameObject.name = "AppleTree";

trunk = GameObject.CreatePrimitive(PrimitiveType.Cylinder);
trunk.transform.localScale = new Vector3(1, height, 1);
trunk.GetComponent<Renderer>().material.color = new Color(0.5f, 0.25f, 0);
trunk.transform.position = position;
trunk.transform.parent = gameObject.transform;
trunk.name = "Trunk";
UnityEngine.Object.Destroy(trunk.GetComponent<CapsuleCollider>());

leaves = GameObject.CreatePrimitive(PrimitiveType.Cube);
leaves.transform.localScale = new Vector3(3, 3, 3);
leaves.GetComponent<Renderer>().material.color = Color.green;
leaves.transform.position = position + new Vector3(0, height - 1, 0);
leaves.transform.parent = gameObject.transform;
leaves.name = "Leaves";
UnityEngine.Object.Destroy(leaves.GetComponent<BoxCollider>());
// Use the new 3D tree model instead of primitives
GameObject gameObject = TreeModel.CreateTree(position, height, "AppleTree");

setGameObject(gameObject);

Expand Down
13 changes: 7 additions & 6 deletions src/c#/tests/entity/TestAppleTree.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,19 @@ public static void testInstantiation() {
int height = 5;
AppleTree tree = new AppleTree(new Vector3(0, 0, 0), height);

// check
// check - verify tree is created with proper structure
UnityEngine.Debug.Assert(tree.getType() == EntityType.TREE);
UnityEngine.Debug.Assert(tree.getGameObject().name == "AppleTree");
UnityEngine.Debug.Assert(tree.getGameObject().transform.position == new Vector3(0, 0, 0));
UnityEngine.Debug.Assert(tree.getGameObject().transform.localScale == new Vector3(1, 1, 1));
UnityEngine.Debug.Assert(tree.getGameObject().transform.childCount == 2);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).name == "Trunk");
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).transform.localScale == new Vector3(1, height, 1));
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).GetComponent<Renderer>().material.color == new Color(0.5f, 0.25f, 0));
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).name == "Leaves");
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).transform.localScale == new Vector3(3, 3, 3));
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).GetComponent<Renderer>().material.color == Color.green);

// Verify trunk and leaves have MeshFilter and MeshRenderer components (3D model)
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).GetComponent<MeshFilter>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).GetComponent<MeshRenderer>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).GetComponent<MeshFilter>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).GetComponent<MeshRenderer>() != null);

Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test now only verifies that MeshFilter and MeshRenderer components exist, but doesn't verify that the meshes are actually assigned (meshFilter.mesh != null) or that the renderers have materials with the correct colors. This reduces test coverage compared to the original tests which verified colors. Consider adding assertions to check that meshes are non-null and materials have the expected colors (brown for trunk, green for leaves).

Suggested change
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).GetComponent<MeshFilter>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(0).GetComponent<MeshRenderer>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).GetComponent<MeshFilter>() != null);
UnityEngine.Debug.Assert(tree.getGameObject().transform.GetChild(1).GetComponent<MeshRenderer>() != null);
Transform trunkTransform = tree.getGameObject().transform.GetChild(0);
Transform leavesTransform = tree.getGameObject().transform.GetChild(1);
MeshFilter trunkMeshFilter = trunkTransform.GetComponent<MeshFilter>();
MeshRenderer trunkMeshRenderer = trunkTransform.GetComponent<MeshRenderer>();
MeshFilter leavesMeshFilter = leavesTransform.GetComponent<MeshFilter>();
MeshRenderer leavesMeshRenderer = leavesTransform.GetComponent<MeshRenderer>();
// Components should exist
UnityEngine.Debug.Assert(trunkMeshFilter != null);
UnityEngine.Debug.Assert(trunkMeshRenderer != null);
UnityEngine.Debug.Assert(leavesMeshFilter != null);
UnityEngine.Debug.Assert(leavesMeshRenderer != null);
// Meshes should be assigned
UnityEngine.Debug.Assert(trunkMeshFilter.mesh != null);
UnityEngine.Debug.Assert(leavesMeshFilter.mesh != null);
// Materials should be assigned and have expected colors
UnityEngine.Debug.Assert(trunkMeshRenderer.material != null);
UnityEngine.Debug.Assert(leavesMeshRenderer.material != null);
// Brown trunk
UnityEngine.Debug.Assert(trunkMeshRenderer.material.color == new Color(0.545f, 0.271f, 0.075f));
// Green leaves
UnityEngine.Debug.Assert(leavesMeshRenderer.material.color == Color.green);

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhanced test coverage to verify:

  • Mesh components exist (MeshFilter, MeshRenderer)
  • Meshes are assigned (not null)
  • Materials are assigned with correct colors (brown trunk: 0.5f, 0.25f, 0 and green leaves)
    (commit 40efa9d)

// clean up
GameObject.Destroy(tree.getGameObject());
Expand Down