Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts TombEngine (TEN) level compilation to emit normalized color values, addressing the longstanding issue where exported colors were effectively in a 0.0–2.0 range instead of 0.0–1.0.
Changes:
- Normalizes room ambient light, vertex colors, static mesh tint, and dynamic light colors for TombEngine export.
- Normalizes item/static instance color tint written into TombEngine item tables.
- Updates custom-vertex lighting normalization logic to target a 0..1 output range.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| TombLib/TombLib/LevelData/Compilers/TombEngine/Rooms.cs | Applies multiple color normalization steps for room geometry, ambient, static meshes, vertex colors, and dynamic lights in TEN export. |
| TombLib/TombLib/LevelData/Compilers/TombEngine/LevelCompilerTombEngine.cs | Normalizes exported item tint color for TEN items table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| return Vector3.Max(output, new Vector3()) * (1.0f / 128.0f); | ||
| // Normalize to 0...1 range | ||
| return Vector3.Max(output, new Vector3()) * (1.0f / 255.0f); |
There was a problem hiding this comment.
CalculateLightForCustomVertex still doesn’t reliably normalize into the 0..1 range: ambientColor is commonly passed as room.Properties.AmbientLight * 128, and AmbientLight is stored in a 0..2 range (so the max input here is 256). Dividing by 255 can therefore yield values > 1.0 even before adding other lights (e.g. 256/255 ≈ 1.0039), and it’s also inconsistent with the rest of the file which normalizes via * 0.5f (i.e. effectively /256). Consider switching to a consistent /256 factor (or 0.5f / 128.0f) and clamping to Vector3.One if TombEngine strictly expects 0..1.
| return Vector3.Max(output, new Vector3()) * (1.0f / 255.0f); | |
| output = Vector3.Max(output, Vector3.Zero); | |
| return Vector3.Clamp(output * (1.0f / 256.0f), Vector3.Zero, Vector3.One); |
| Color = new Vector4(instance.Color.X, instance.Color.Y, instance.Color.Z, 1.0f), | ||
| HitPoints = 0, | ||
| Color = new Vector4(instance.Color.X * 0.5f, instance.Color.Y * 0.5f, instance.Color.Z * 0.5f, 1.0f), // Normalize to 0...1 range | ||
| HitPoints = 0, |
There was a problem hiding this comment.
There’s a tab-indented line here (HitPoints = 0,) which is visually misaligned with surrounding entries (spaces) and will keep showing up as noisy diffs depending on editor settings. Re-indent this line consistently with the rest of the object initializer.
| HitPoints = 0, | |
| HitPoints = 0, |
| trVertex.Position = new Vector3(Position.X, -(Position.Y + room.WorldPos.Y), Position.Z); | ||
| trVertex.Color = color; | ||
| trVertex.Color = color * 0.5f; // Normalize to 0...1 range | ||
| trVertex.IsOnPortal = false; |
There was a problem hiding this comment.
Normalization is applied in multiple places via hard-coded * 0.5f factors (vertex colors, ambient light, instance colors, light colors). To reduce the risk of future inconsistencies (like the current /255 vs *0.5f mismatch), consider centralizing this into a small helper/constant (e.g., NormalizeTeColor(...)).
| Pitch = ToTrAngle(instance.RotationX), | ||
| Roll = ToTrAngle(-instance.Roll), | ||
| Color = new Vector4(instance.Color.X, instance.Color.Y, instance.Color.Z, 1.0f), | ||
| Color = new Vector4(instance.Color.X * 0.5f, instance.Color.Y * 0.5f, instance.Color.Z * 0.5f, 1.0f), // Normalize to 0...1 range |
There was a problem hiding this comment.
For readability and to avoid repeating the normalization factor per-component, consider constructing this as new Vector4(instance.Color * 0.5f, 1.0f) (or a shared normalization helper) instead of multiplying X/Y/Z separately.
| Color = new Vector4(instance.Color.X * 0.5f, instance.Color.Y * 0.5f, instance.Color.Z * 0.5f, 1.0f), // Normalize to 0...1 range | |
| Color = new Vector4(instance.Color * 0.5f, 1.0f), // Normalize to 0...1 range |
This pull request addresses the old bug about the 0.0f - 2.0f color range. After merging this pull request, TE will emit colors in 0.0f - 1.0f range for TEN target.