-
Notifications
You must be signed in to change notification settings - Fork 180
feat: Raw reading on zone load and scene tools #1910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
added option to automatically write the raw obj file Added scene colors to the obj use proper scene colors from hf
…lameUniverse/DarkflameServer into raw-parsing-for-scene-data
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements raw terrain file reading on zone load to enable scene identification from player positions. Key changes include parsing .raw terrain files, building a scene adjacency graph from scene transitions, and adding developer commands for scene debugging and visualization.
- Adds comprehensive RAW terrain file parsing with scene map support
- Implements scene ID lookup from world positions and scene adjacency graph construction
- Adds developer commands for scene debugging and visualization with OBJ export capability
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/worldconfig.ini | Adds config option to export terrain meshes to OBJ files for debugging |
| dZoneManager/dZoneManager.h | Declares scene position lookup, adjacency list access, and scene graph building methods |
| dZoneManager/dZoneManager.cpp | Implements scene position queries, scene graph construction from transitions, and global scene connectivity |
| dZoneManager/Zone.h | Adds Raw terrain data storage and scene lookup methods |
| dZoneManager/Zone.cpp | Implements zone file version checking, Raw file loading, and optional OBJ export |
| dZoneManager/Raw.h | Defines complete terrain chunk structures, scene vertex data, and mesh generation/export functions |
| dZoneManager/Raw.cpp | Implements RAW file parsing, terrain mesh generation with scene IDs, and OBJ export with scene colors |
| dZoneManager/CMakeLists.txt | Adds Raw.cpp to build and includes SceneColor.h path |
| dNavigation/dTerrain/* | Removes old unused terrain parsing code |
| dNavigation/dNavMesh.cpp | Removes unused RawFile.h include |
| dNavigation/CMakeLists.txt | Removes dTerrain subdirectory from build |
| dGame/dUtilities/SlashCommands/DEVGMCommands.h | Declares four new scene debugging commands |
| dGame/dUtilities/SlashCommands/DEVGMCommands.cpp | Implements GetScene, GetAdjacentScenes, SpawnScenePoints, and SpawnAllScenePoints commands |
| dGame/dUtilities/SlashCommandHandler.cpp | Registers the four new scene debugging commands |
| dCommon/dClient/SceneColor.h | Defines color palette for visualizing scene IDs in OBJ exports |
| dCommon/NiColor.h | Adds RGB color structure with grayscale conversion |
Comments suppressed due to low confidence (1)
dZoneManager/Raw.cpp:1
- Potential division by zero in GetSceneIDFromPosition if chunk dimensions are 1. Similar to the issue in GenerateTerrainMesh, if chunk.width or chunk.height equals 1, this will cause division by zero. Add validation before these calculations.
#include "Raw.h"
| BinaryIO::BinaryRead(stream, chunk.width); | ||
| BinaryIO::BinaryRead(stream, chunk.height); | ||
| BinaryIO::BinaryRead(stream, chunk.offsetX); | ||
| BinaryIO::BinaryRead(stream, chunk.offsetZ); | ||
|
|
||
| if (stream.fail()) { |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation in ReadChunk function. Lines 61-67 use single tab indentation while surrounding code uses double tab indentation. According to the project's coding standards, indentation should be consistent with tabs (4 spaces). Lines 61-67 should be indented to match the surrounding code structure.
| BinaryIO::BinaryRead(stream, chunk.width); | |
| BinaryIO::BinaryRead(stream, chunk.height); | |
| BinaryIO::BinaryRead(stream, chunk.offsetX); | |
| BinaryIO::BinaryRead(stream, chunk.offsetZ); | |
| if (stream.fail()) { | |
| BinaryIO::BinaryRead(stream, chunk.width); | |
| BinaryIO::BinaryRead(stream, chunk.height); | |
| BinaryIO::BinaryRead(stream, chunk.offsetX); | |
| BinaryIO::BinaryRead(stream, chunk.offsetZ); | |
| if (stream.fail()) { |
| 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); | ||
| } LOG("Raw terrain bounds: X[%.2f, %.2f], Z[%.2f, %.2f]", | ||
| outRaw.minBoundsX, outRaw.maxBoundsX, outRaw.minBoundsZ, outRaw.maxBoundsZ); | ||
| } |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation. This line and lines 275-286 appear to have incorrect indentation (single tab instead of double tab) based on the surrounding code structure. The entire loop body should be indented consistently with the rest of the function.
| 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); | |
| } LOG("Raw terrain bounds: X[%.2f, %.2f], Z[%.2f, %.2f]", | |
| outRaw.minBoundsX, outRaw.maxBoundsX, outRaw.minBoundsZ, outRaw.maxBoundsZ); | |
| } | |
| 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); | |
| } | |
| 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); | ||
| } |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG statement appears on the same line as a closing brace without proper indentation or line break. This should be on its own line with proper indentation for readability.
| } 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); | |
| } |
| } LOG("Raw terrain bounds: X[%.2f, %.2f], Z[%.2f, %.2f]", | ||
| outRaw.minBoundsX, outRaw.maxBoundsX, outRaw.minBoundsZ, outRaw.maxBoundsZ); | ||
| } | ||
| } return true; |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return statement appears on the same line as a closing brace. This should be on its own line with proper indentation for readability and consistency with the rest of the codebase.
| } return true; | |
| } | |
| return true; |
| 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 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 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); |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation starting at line 325. Lines 325-350 appear to use incorrect indentation (should be triple tab based on nesting level). The code should maintain consistent tab indentation throughout the nested loops.
| 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 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 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); | |
| 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 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 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 (m_SceneAdjacencyList.find(sceneID) == m_SceneAdjacencyList.end()) { | ||
| m_SceneAdjacencyList[sceneID] = std::vector<LWOSCENEID>(); | ||
| } |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inefficient double lookup in associency list. The code performs find() then uses operator[] which causes a second lookup. Use insert() or emplace() to avoid the double lookup: m_SceneAdjacencyList.try_emplace(sceneID, std::vector<LWOSCENEID>()); or check the return value of insert/emplace.
| if (m_SceneAdjacencyList.find(sceneID) == m_SceneAdjacencyList.end()) { | |
| m_SceneAdjacencyList[sceneID] = std::vector<LWOSCENEID>(); | |
| } | |
| m_SceneAdjacencyList.try_emplace(sceneID, std::vector<LWOSCENEID>()); |
| const float sceneMapI = ((i) / (chunk.width - 1)) * (chunk.colorMapResolution - 1); | ||
| const float sceneMapJ = ((j) / (chunk.height - 1)) * (chunk.colorMapResolution - 1); |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential division by zero if chunk.width or chunk.height equals 1. If a chunk has width or height of 1, the expressions chunk.width - 1 or chunk.height - 1 will be 0, causing division by zero. Add a check to ensure width and height are greater than 1 before this calculation.
| const float sceneMapI = ((i) / (chunk.width - 1)) * (chunk.colorMapResolution - 1); | |
| const float sceneMapJ = ((j) / (chunk.height - 1)) * (chunk.colorMapResolution - 1); | |
| const float sceneMapI = (chunk.width > 1) ? ((static_cast<float>(i) / static_cast<float>(chunk.width - 1)) * static_cast<float>(chunk.colorMapResolution - 1)) : 0.0f; | |
| const float sceneMapJ = (chunk.height > 1) ? ((static_cast<float>(j) / static_cast<float>(chunk.height - 1)) * static_cast<float>(chunk.colorMapResolution - 1)) : 0.0f; |
| #ifndef __RAW_H__ | ||
| #define __RAW_H__ |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Include guard uses reserved identifier pattern. Names containing double underscores (__) are reserved for the implementation in C++. Use a standard include guard pattern like RAW_H or DZONEMANAGER_RAW_H instead of __RAW_H__.
| #ifndef __RAW_H__ | |
| #define __RAW_H__ | |
| #ifndef DZONEMANAGER_RAW_H | |
| #define DZONEMANAGER_RAW_H |
| #ifndef NICOLOR_H | ||
| #define NICOLOR_H |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Include guard naming is inconsistent with other files in the project. While this guard is correctly formed (not using reserved identifiers), it should follow the project's naming convention. Consider using a more specific name like DCOMMON_NICOLOR_H to avoid potential conflicts.
| #ifndef SCENE_COLOR_H | ||
| #define SCENE_COLOR_H |
Copilot
AI
Nov 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Include guard naming should be more specific to avoid conflicts. Consider using a more specific name like DCOMMON_DCLIENT_SCENECOLOR_H to match the file's location in the project structure and avoid potential naming conflicts.
the main reason behind doing this is to get what scene the player is in.
also implemented building an adjacency list of scenes and getting adjacent scenes for a given scene
Also make an option for outputting the raw as obj on world load
and add scene colors to the obj