fix: address PR review feedback for raw terrain parsing

- Fix integer division bug in scene map lookups (was truncating to 0)
- Fix indentation throughout Raw.cpp, DEVGMCommands.cpp
- Add missing <algorithm> and <set> includes in dZoneManager.cpp
- Add missing width/height/scaleFactor guards in SpawnAllScenePoints
- Fix %llu -> %zu for size_t format specifiers
- Simplify no-op worldY calculation (y / scale * scale -> y)
- Remove redundant ternary guards in GetSceneIDFromPosition
- Fix misleading "Spawned LOT" feedback message
- Update info.settings to use LwoNameValue::Insert API (post-merge fix)
- Refactor SceneColor to static constexpr std::array (no heap alloc)
- Make NiColor constructors constexpr
- Remove duplicate CDZoneTableTable.h include

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Aaron Kimbrell
2026-06-21 01:50:08 -05:00
parent 83e2ea4278
commit 1aeede3cd1
6 changed files with 264 additions and 303 deletions

View File

@@ -58,20 +58,22 @@ namespace Raw {
*/
static bool ReadChunk(std::istream& stream, Chunk& chunk, uint16_t version) {
try {
// Read basic chunk info
BinaryIO::BinaryRead(stream, chunk.id);
if (stream.fail()) {
return false;
}
// Read basic chunk info
BinaryIO::BinaryRead(stream, chunk.id);
if (stream.fail()) {
return false;
}
BinaryIO::BinaryRead(stream, chunk.width);
BinaryIO::BinaryRead(stream, chunk.height);
BinaryIO::BinaryRead(stream, chunk.offsetX);
BinaryIO::BinaryRead(stream, chunk.offsetZ);
BinaryIO::BinaryRead(stream, chunk.width);
BinaryIO::BinaryRead(stream, chunk.height);
BinaryIO::BinaryRead(stream, chunk.offsetX);
BinaryIO::BinaryRead(stream, chunk.offsetZ);
if (stream.fail()) {
return false;
} // For version < 32, shader ID comes before texture IDs
if (stream.fail()) {
return false;
}
// For version < 32, shader ID comes before texture IDs
if (version < 32) {
BinaryIO::BinaryRead(stream, chunk.shaderId);
}
@@ -344,30 +346,28 @@ namespace Raw {
}
// Calculate terrain bounds from all chunks
if (!outRaw.chunks.empty()) {
outRaw.minBoundsX = std::numeric_limits<float>::max();
outRaw.minBoundsZ = std::numeric_limits<float>::max();
outRaw.maxBoundsX = std::numeric_limits<float>::lowest();
outRaw.maxBoundsZ = std::numeric_limits<float>::lowest();
for (const auto& chunk : outRaw.chunks) {
// Calculate chunk bounds
const float chunkMinX = chunk.offsetX;
const float chunkMinZ = chunk.offsetZ;
const float chunkMaxX = chunkMinX + (chunk.width * chunk.scaleFactor);
const float chunkMaxZ = chunkMinZ + (chunk.height * chunk.scaleFactor);
// Update overall bounds
outRaw.minBoundsX = std::min(outRaw.minBoundsX, chunkMinX);
outRaw.minBoundsZ = std::min(outRaw.minBoundsZ, chunkMinZ);
outRaw.maxBoundsX = std::max(outRaw.maxBoundsX, chunkMaxX);
outRaw.maxBoundsZ = std::max(outRaw.maxBoundsZ, chunkMaxZ);
if (!outRaw.chunks.empty()) {
outRaw.minBoundsX = std::numeric_limits<float>::max();
outRaw.minBoundsZ = std::numeric_limits<float>::max();
outRaw.maxBoundsX = std::numeric_limits<float>::lowest();
outRaw.maxBoundsZ = std::numeric_limits<float>::lowest();
for (const auto& chunk : outRaw.chunks) {
const float chunkMinX = chunk.offsetX;
const float chunkMinZ = chunk.offsetZ;
const float chunkMaxX = chunkMinX + (chunk.width * chunk.scaleFactor);
const float chunkMaxZ = chunkMinZ + (chunk.height * chunk.scaleFactor);
outRaw.minBoundsX = std::min(outRaw.minBoundsX, chunkMinX);
outRaw.minBoundsZ = std::min(outRaw.minBoundsZ, chunkMinZ);
outRaw.maxBoundsX = std::max(outRaw.maxBoundsX, chunkMaxX);
outRaw.maxBoundsZ = std::max(outRaw.maxBoundsZ, chunkMaxZ);
}
LOG("Raw terrain bounds: X[%.2f, %.2f], Z[%.2f, %.2f]",
outRaw.minBoundsX, outRaw.maxBoundsX, outRaw.minBoundsZ, outRaw.maxBoundsZ);
}
}
LOG("Raw terrain bounds: X[%.2f, %.2f], Z[%.2f, %.2f]",
outRaw.minBoundsX, outRaw.maxBoundsX, outRaw.minBoundsZ, outRaw.maxBoundsZ);
}
}
return true;
return true;
} catch (const std::exception&) {
return false;
}
@@ -407,29 +407,23 @@ namespace Raw {
const float y = chunk.heightMap[heightIndex];
// Calculate world position
const float worldX = ((i) + (chunk.offsetX / chunk.scaleFactor)) * chunk.scaleFactor;
const float worldY = (y / chunk.scaleFactor) * chunk.scaleFactor;
const float worldZ = ((j) + (chunk.offsetZ / chunk.scaleFactor)) * chunk.scaleFactor;
const float worldX = (static_cast<float>(i) + (chunk.offsetX / chunk.scaleFactor)) * chunk.scaleFactor;
const float worldY = y;
const float worldZ = (static_cast<float>(j) + (chunk.offsetZ / chunk.scaleFactor)) * chunk.scaleFactor;
const NiPoint3 worldPos(worldX, worldY, worldZ);
// Get scene ID at this position
// Map heightmap position to scene map position
// The scene map is colorMapResolution x colorMapResolution
// We need to map from heightmap coordinates (i, j) to scene map coordinates
const float sceneMapI = ((i) / (chunk.width - 1)) * (chunk.colorMapResolution - 1);
const float sceneMapJ = ((j) / (chunk.height - 1)) * (chunk.colorMapResolution - 1);
const float sceneMapI = (static_cast<float>(i) / static_cast<float>(chunk.width - 1)) * static_cast<float>(chunk.colorMapResolution - 1);
const float sceneMapJ = (static_cast<float>(j) / static_cast<float>(chunk.height - 1)) * static_cast<float>(chunk.colorMapResolution - 1);
const uint32_t sceneI = std::min(static_cast<uint32_t>(sceneMapI), chunk.colorMapResolution - 1);
const uint32_t sceneJ = std::min(static_cast<uint32_t>(sceneMapJ), chunk.colorMapResolution - 1);
// Scene map uses the same indexing pattern as heightmap: row * width + col
const uint32_t sceneIndex = sceneI * chunk.colorMapResolution + sceneJ;
uint8_t sceneID = 0;
if (sceneIndex < chunk.sceneMap.size()) {
sceneID = chunk.sceneMap[sceneIndex];
}
}
outMesh.vertices.emplace_back(worldPos, sceneID);
if (i > 0 && j > 0) {
const uint32_t currentVert = vertexOffset + chunk.width * i + j;
@@ -450,49 +444,38 @@ namespace Raw {
}
}
vertexOffset += chunk.width * chunk.height;
vertexOffset += chunk.width * chunk.height;
}
}
}
bool WriteTerrainMeshToOBJ(const TerrainMesh& mesh, const std::string& path) {
try {
std::ofstream file(path);
if (!file.is_open()) {
LOG("Failed to open OBJ file for writing: %s", path.c_str());
bool WriteTerrainMeshToOBJ(const TerrainMesh& mesh, const std::string& path) {
try {
std::ofstream file(path);
if (!file.is_open()) {
LOG("Failed to open OBJ file for writing: %s", path.c_str());
return false;
}
for (const auto& v : mesh.vertices) {
const NiColor& color = SceneColor::Get(v.sceneID);
file << "v " << v.position.x << ' ' << v.position.y << ' ' << v.position.z
<< ' ' << color.m_Red << ' ' << color.m_Green << ' ' << color.m_Blue << '\n';
}
for (size_t i = 0; i < mesh.triangles.size(); i += 3) {
file << "f " << (mesh.triangles[i] + 1) << ' '
<< (mesh.triangles[i + 1] + 1) << ' '
<< (mesh.triangles[i + 2] + 1) << '\n';
}
file.close();
LOG("Successfully wrote terrain mesh to OBJ: %s (%zu vertices, %zu triangles)",
path.c_str(), mesh.vertices.size(), mesh.triangles.size() / 3);
return true;
} catch (const std::exception& e) {
LOG("Exception while writing OBJ file: %s", e.what());
return false;
}
// Create instance of SceneColor for color lookup
SceneColor sceneColor;
// Write vertices with colors
// OBJ format supports vertex colors as: v x y z r g b
for (const auto& v : mesh.vertices) {
file << "v " << v.position.x << ' ' << v.position.y << ' ' << v.position.z;
uint8_t sceneID = v.sceneID;
const NiColor& color = sceneColor.Get(sceneID);
file << ' ' << color.m_Red << ' ' << color.m_Green << ' ' << color.m_Blue;
file << '\n';
}
// Write faces (triangles)
for (size_t i = 0; i < mesh.triangles.size(); i += 3) {
// OBJ indices are 1-based
file << "f " << (mesh.triangles[i] + 1) << ' '
<< (mesh.triangles[i + 1] + 1) << ' '
<< (mesh.triangles[i + 2] + 1) << '\n';
}
file.close();
LOG("Successfully wrote terrain mesh to OBJ: %s (%zu vertices, %zu triangles)",
path.c_str(), mesh.vertices.size(), mesh.triangles.size() / 3);
return true;
} catch (const std::exception& e) {
LOG("Exception while writing OBJ file: %s", e.what());
return false;
}
}
} // namespace Raw

View File

@@ -110,7 +110,7 @@ void Zone::LoadZoneIntoMemory() {
// Generate terrain mesh
Raw::GenerateTerrainMesh(m_Raw, m_TerrainMesh);
LOG("Generated terrain mesh with %llu vertices and %llu triangles", m_TerrainMesh.vertices.size(), m_TerrainMesh.triangles.size() / 3);
LOG("Generated terrain mesh with %zu vertices and %zu triangles", m_TerrainMesh.vertices.size(), m_TerrainMesh.triangles.size() / 3);
// Write to OBJ
std::string objFileName = "terrain_" + std::to_string(m_ZoneID.GetMapID()) + ".obj";

View File

@@ -10,10 +10,11 @@
#include "VanityUtilities.h"
#include "WorldConfig.h"
#include "CDZoneTableTable.h"
#include <algorithm>
#include <chrono>
#include <cmath>
#include <set>
#include "eObjectBits.h"
#include "CDZoneTableTable.h"
#include "AssetManager.h"
#include <ranges>
@@ -331,20 +332,11 @@ LWOSCENEID dZoneManager::GetSceneIDFromPosition(const NiPoint3& position) const
if (heightI >= 0.0f && heightI < static_cast<float>(chunk.width) &&
heightJ >= 0.0f && heightJ < static_cast<float>(chunk.height)) {
// Map heightmap position to scene map position (same as GenerateTerrainMesh)
const float sceneMapI = (chunk.width > 1)
? (heightI / static_cast<float>(chunk.width - 1)) * static_cast<float>(chunk.colorMapResolution - 1)
: 0.0f;
const float sceneMapJ = (chunk.height > 1)
? (heightJ / static_cast<float>(chunk.height - 1)) * static_cast<float>(chunk.colorMapResolution - 1)
: 0.0f;
const float sceneMapI = (heightI / static_cast<float>(chunk.width - 1)) * static_cast<float>(chunk.colorMapResolution - 1);
const float sceneMapJ = (heightJ / static_cast<float>(chunk.height - 1)) * static_cast<float>(chunk.colorMapResolution - 1);
const uint32_t sceneI = (chunk.colorMapResolution > 0)
? std::min(static_cast<uint32_t>(sceneMapI), chunk.colorMapResolution - 1)
: 0;
const uint32_t sceneJ = (chunk.colorMapResolution > 0)
? std::min(static_cast<uint32_t>(sceneMapJ), chunk.colorMapResolution - 1)
: 0;
const uint32_t sceneI = std::min(static_cast<uint32_t>(sceneMapI), chunk.colorMapResolution - 1);
const uint32_t sceneJ = std::min(static_cast<uint32_t>(sceneMapJ), chunk.colorMapResolution - 1);
// Scene map uses the same indexing pattern as heightmap: row * width + col
const uint32_t sceneIndex = sceneI * chunk.colorMapResolution + sceneJ;