shader restructure #16
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe pull request introduces extensive changes across the project. Configuration files are updated for CMake integration and workspace settings. The build system and shader sources have been modified, with new GLSL shaders added for 2D batch rendering. Engine and scene management scripts in Lua are restructured to support both 2D and 3D scenes, while the C++ code is enhanced with new rendering, logging, input, shader, and terrain interfaces. Additionally, several new classes and functions are introduced to modularize shader construction and scene management, and UI layout adjustments are made via ImGui configuration changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main.lua
participant Engine as Engine Module
participant Scene as Scene Initialization
Main->>Engine: Check _G.initialized guard
alt Not initialized
Engine->>Main: Call initialize2DScene() or initialize3DScene() based on USE_3D
else Already initialized
Main-->>Main: Exit early
end
Main->>Engine: Proceed with UpdateScene()
Engine->>Engine: (Optionally) Create checker texture once
sequenceDiagram
participant LuaSys as LuaScriptSystem
participant Lua as Lua State
LuaSys->>Lua: CallGlobalFunction("UpdateScene")
Lua-->>LuaSys: Execute global UpdateScene function
alt Function Missing
LuaSys->>LuaSys: Log error for invalid call
end
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (45)
src/Core/Transform.h (2)
13-13: Consider Defining an Explicit Default Constructor Body
Although it’s not strictly necessary, providing an explicitly defined constructor body might be beneficial if future logic or initialization is needed.
33-46: Guarding Against Zero Scale
Excellent decision to return the identity matrix if scale is zero. However, exact zero comparisons can be risky when dealing with floating-point inaccuracies. Consider a small epsilon comparison if extremely small scale should also be treated as zero.- if (m_Data.scale == glm::vec3(0.0f)) { + const float EPSILON = 1e-6f; + if (glm::length(m_Data.scale) < EPSILON) { return glm::mat4(1.0f); }src/Scene/Scene.h (3)
42-46: CreateObject for Scene Composition
This method fosters flexible scene composition. A potential future enhancement: consider generating unique IDs or using an internal map if searching by name becomes a performance bottleneck.
69-72: CreateTerrain Method
Keep in mind that the declaration alone doesn’t give visibility into the terrain creation logic. Ensure it’s properly defined to handle repeated calls safely.
75-86: Linear Search for Objects
GetObjectuses a linear search that may be fine for small scenes. If scenes grow large, consider an indexed or hashed lookup for performance.src/Input/InputSystem.h (2)
26-30: Update Method
TheUpdate(float deltaTime)method is a good place to handle continuous input logic. Ensure complex camera or movement handling doesn’t bloat this method.
63-66: Mouse Sensitivity Controls
TheSetSensitivityandGetSensitivitypair is intuitive. Consider clamping the sensitivity to a safe range if extremely high or negative values might produce odd behavior.src/Application.h (3)
46-46: Possible spelling concern in 'indicies'.
Consider correcting the variable name toindicesfor consistency and clarity.- unsigned int indicies; + unsigned int indices;
67-71: Camera type setter and getter align with the overall design.
ConfigureCamera()allows flexible camera adjustments. Ensure it handles all camera-dependent resources (e.g., uniform buffers).
193-195: Default render and camera types are clear.
Initializingm_RenderTypetoRender3Dandm_CameraTypetoPerspectiveseems reasonable, but consider moving these defaults to configuration if they will vary by use case.sandbox/assets/scripts/main.lua (4)
8-8: Global toggle for 3D usage is clear but might be centralized.
local USE_3Dis straightforward. If you foresee toggling this frequently, consider storing it in a config or engine parameter to keep logic more centralized.
14-37: initialize2DScene is straightforward.
Scene creation checks and camera configuration are done properly. However, line 28 includesengine.log("abs")which might be unintended leftover debug text.- engine.log("abs")
130-158: Animation structure supports rotating quads.
Separating out the rotating elements from position-based ones is a good design. Check rotating speed to ensure it does not become visually overwhelming.
250-250: Explicit texture creation call might be optional.
Since texture creation occurs lazily, callingcreateCheckerTexture()again here is not strictly necessary unless you want it ready before rendering.src/Scene/Scene.cpp (1)
231-240: CreateTerrain method is neatly encapsulated.
Encapsulating terrain creation allows better modular control. Consider logging an error or warning if terrain creation fails.src/Renderer/Renderer2D.cpp (1)
60-76: Consider using standard containers for index data.
Heap-allocating and manually deleting the index array is valid, but using a standard container likestd::vector<uint32_t>can improve safety and readability.- uint32_t* quadIndices = new uint32_t[s_Data.MaxIndices]; - // ... - delete[] quadIndices; + std::vector<uint32_t> quadIndices(s_Data.MaxIndices); + // ...src/Core/Utils/Logging.h (1)
92-118: Timestamping approach is clear.
Converting the time to a localtime string and prefixing log messages with severity is a standard practice. No obvious issues here, though consider using UTC if consistent times across time zones are needed.sandbox/assets/scripts/engine.lua (2)
277-279: Separate global function vs engine member function.
Lines 277-279 show a standalonesetRenderTypefunction. Consider making it a field ofengine, likeengine.setRenderType, for consistency with other engine functions.--- function setRenderType(type) end +++ function engine.setRenderType(type) end
281-283: Getter pattern consistency.
getRenderType()is also defined outside theenginetable. Similar to above, it’s recommended to place it underengine.getRenderTypefor uniform naming and grouping.--- function getRenderType() end +++ function engine.getRenderType() endsrc/Application.cpp (2)
415-439: Be mindful of terrain re-initialization overhead.Repeatedly tearing down and recreating the terrain system when switching render types (2D <-> 3D) may be costly. Ensure this approach won’t degrade performance if toggled frequently.
441-467: Use dynamic window dimensions for perspective.Currently, the aspect ratio for the perspective camera is hard-coded to 1280×720. Consider retrieving real-time window sizes for better correctness.
- std::make_shared<PerspectiveCamera>(45.0f, 1280.0f / 720.0f) + auto width = static_cast<float>(m_Window->GetWidth()); + auto height = static_cast<float>(m_Window->GetHeight()); + std::make_shared<PerspectiveCamera>(45.0f, width / height)src/Scripting/LuaScriptSystem.cpp (6)
17-19: Unused forward declaration.
class DefaultShaders;is introduced but not referenced. Remove if unneeded or implement usage if planned.
168-174: Add default handling for unknown camera types.
setCameraTypelacks a fallback or error if the string is neither "orthographic" nor "perspective". Consider logging a warning or defaulting to a valid setting.
182-198: Validate unknown render types.Similarly,
setRenderTypeand related getters lack handling for invalid strings. Logging an error or defaulting to "3d" or "2d" could prevent inconsistent state.
429-475: 2D rendering API is well-structured.These additions (BeginScene/EndScene, drawing quads, textures, etc.) enable flexible 2D scripting.
- Consider concurrency aspects if invoked from multiple scripts simultaneously.
- Optionally allow custom cameras for 2D scenes.
476-512: SceneObject bindings are robust.Registering the
SceneObjectwith getter/setter fortransformand thecreateCubehelper is beneficial.
Consider letting Lua pass a custom shader to new objects, rather than forcingCreateBasicShader().
603-614: Leverage arguments or returns in 'CallGlobalFunction'.
CallGlobalFunctionquietly ignores exit codes and does not pass arguments. For advanced scripting tasks, consider capturing Lua return values or raising explicit errors.assets/shaders/batch_renderer_2d.vert (1)
16-22: Consider adding bounds checking for texture index.While the implementation is correct, it would be good to add a validation check for the texture index to ensure it's within the supported range.
void main() { v_Color = a_Color; v_TexCoord = a_TexCoord; + // Ensure texture index is within supported range (typically 0-31 for most GPUs) + v_TexIndex = clamp(a_TexIndex, 0.0, 31.0); - v_TexIndex = a_TexIndex; v_TilingFactor = a_TilingFactor; gl_Position = u_ViewProjection * vec4(a_Position, 1.0); }src/Renderer/IRenderer.h (1)
15-21: Consider adding thread safety to Statistics::Reset.The
memsetoperation might not be thread-safe if statistics are accessed from multiple threads.struct Statistics { uint32_t DrawCalls = 0; uint32_t VertexCount = 0; uint32_t IndexCount = 0; - void Reset() { memset(this, 0, sizeof(Statistics)); } + void Reset() { + DrawCalls = 0; + VertexCount = 0; + IndexCount = 0; + } };src/Shader/ShaderLibrary.h (2)
19-24: Add documentation for factory methods.Consider adding documentation for each factory method to describe the purpose and characteristics of each shader type (e.g., what uniforms they expect, their use cases).
26-28: Consider dependency injection for better testability.The static shader map could make unit testing challenging. Consider making it an instance member and using dependency injection, or provide a way to reset/mock the shader map for testing purposes.
src/Shader/ShaderFactory.cpp (1)
5-5: Use initialization list for better performance.Move the member initialization to the initialization list:
-ShaderFactory::ShaderFactory() { m_Version = "#version 330 core\n"; } +ShaderFactory::ShaderFactory() : m_Version("#version 330 core\n") {}🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 5-5: Variable 'm_Version' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
src/Scripting/LuaScriptSystem.h (1)
37-37: Add documentation for CallGlobalFunction.Add documentation to match the style of other methods in the class:
+ /// @brief Call a global Lua function by name + /// @param functionName The name of the global function to call void CallGlobalFunction(const std::string& functionName);src/Scene/SceneObject.h (1)
11-14: Consider making the name member protected.The
namemember is currently public, which breaks encapsulation. Consider making it protected and providing getter/setter methods:- std::string name; +protected: + std::string name; +public: + const std::string& GetName() const { return name; } + void SetName(const std::string& newName) { name = newName; }Also applies to: 40-40
src/Renderer/Renderer2D.h (2)
31-39: Consider adding rotation support for quads.The DrawQuad methods support position, size, color, and texture, but lack rotation parameters which are common in 2D rendering.
-void DrawQuad(const glm::vec2& position, const glm::vec2& size, const glm::vec4& color); +void DrawQuad(const glm::vec2& position, const glm::vec2& size, const glm::vec4& color, float rotation = 0.0f); -void DrawQuad(const glm::vec3& position, const glm::vec2& size, const glm::vec4& color); +void DrawQuad(const glm::vec3& position, const glm::vec2& size, const glm::vec4& color, float rotation = 0.0f); -void DrawQuad(const glm::vec2& position, const glm::vec2& size, - const std::shared_ptr<Texture>& texture, float tilingFactor = 1.0f, - const glm::vec4& tintColor = glm::vec4(1.0f)); +void DrawQuad(const glm::vec2& position, const glm::vec2& size, + const std::shared_ptr<Texture>& texture, float tilingFactor = 1.0f, + const glm::vec4& tintColor = glm::vec4(1.0f), float rotation = 0.0f); -void DrawQuad(const glm::vec3& position, const glm::vec2& size, - const std::shared_ptr<Texture>& texture, float tilingFactor = 1.0f, - const glm::vec4& tintColor = glm::vec4(1.0f)); +void DrawQuad(const glm::vec3& position, const glm::vec2& size, + const std::shared_ptr<Texture>& texture, float tilingFactor = 1.0f, + const glm::vec4& tintColor = glm::vec4(1.0f), float rotation = 0.0f);
11-13: Enhance class documentation.While the brief description is good, consider adding more detailed documentation about:
- The batching mechanism
- Performance characteristics
- Usage examples
src/Renderer/BatchRenderer2D.h (1)
59-67: Enhance statistics tracking.Consider adding more metrics to the Statistics struct:
- Vertex count
- Current batch size
- Texture switches
- Memory usage
struct Statistics { uint32_t DrawCalls = 0; uint32_t QuadCount = 0; + uint32_t VertexCount = 0; + uint32_t CurrentBatchSize = 0; + uint32_t TextureSwitches = 0; + size_t MemoryUsage = 0; void Reset() { DrawCalls = 0; QuadCount = 0; + VertexCount = 0; + CurrentBatchSize = 0; + TextureSwitches = 0; + MemoryUsage = 0; } };src/Scene/SceneManager.h (2)
28-28: Consider adding more context to trace logging.The trace log could include additional context such as previous scene name and transition timing.
- LOG_TRACE_CONCAT("Activated scene: ", name); + LOG_TRACE_CONCAT("Scene transition: '", (activeScene ? activeScene->GetName() : "none"), "' -> '", name, "'");
57-62: Consider adding error handling for Update method.The Update method should handle and log any exceptions that might occur during scene update.
void Update(float deltaTime) { std::lock_guard<std::mutex> lock(sceneMutex); if (activeScene) { + try { activeScene->OnUpdate(deltaTime); + } catch (const std::exception& e) { + LOG_ERROR_CONCAT("Scene update failed: ", e.what()); + } } }assets/shaders/batch_renderer_2d.frag (1)
1-8: Consider using GLSL 420 or higher for bindless textures.The current implementation uses GLSL 330 with a fixed array of texture samplers. Consider upgrading to GLSL 420 or higher to leverage bindless textures, which could improve performance by eliminating texture binding overhead.
-#version 330 core +#version 420 core + +#extension GL_ARB_bindless_texture : requiresrc/Shader/ShaderLibrary.cpp (2)
8-8: Consider using a thread-safe container for static shader storage.The static unordered_map might face race conditions in a multi-threaded environment. Consider using a concurrent container or adding synchronization.
56-65: Add retry mechanism for shader loading failures.The current implementation returns nullptr on shader loading failure. Consider adding a retry mechanism with exponential backoff for transient failures.
std::shared_ptr<Shader> ShaderLibrary::Load(const std::string& name, const std::string& vertexPath, const std::string& fragmentPath) { + const int maxRetries = 3; + int retryCount = 0; + std::shared_ptr<Shader> shader; + + while (retryCount < maxRetries) { + shader = Shader::CreateFromFiles(vertexPath, fragmentPath); + if (shader) break; + + LOG_WARN("Retry ", retryCount + 1, " of ", maxRetries, " for shader: ", name); + retryCount++; + std::this_thread::sleep_for(std::chrono::milliseconds(100 * (1 << retryCount))); + } + if (!shader) { LOG_ERROR("Failed to load shader from files: ", vertexPath, " and ", fragmentPath); return nullptr; } s_Shaders[name] = shader; return shader; }src/TerrainSystem/TerrainSystem.h (1)
97-98: Make initialization flag atomic for thread safety.The m_IsInitialized flag should be atomic to prevent race conditions in multi-threaded scenarios.
-bool m_IsInitialized = false; +std::atomic<bool> m_IsInitialized{false};src/Core/AssetManager.h (1)
161-215: Consider optimizing the cube mesh data.While the implementation is correct, there are opportunities for optimization:
- The vertex data has redundant positions that could be shared between faces
- The cube could be defined with 8 vertices instead of 24
Here's an optimized version:
- float vertices[] = { - // Front face - -0.5f, -0.5f, 0.5f, - 0.5f, -0.5f, 0.5f, - 0.5f, 0.5f, 0.5f, - -0.5f, 0.5f, 0.5f, - // Back face - -0.5f, -0.5f, -0.5f, - 0.5f, -0.5f, -0.5f, - 0.5f, 0.5f, -0.5f, - -0.5f, 0.5f, -0.5f, - }; + float vertices[] = { + // All 8 vertices of the cube + -0.5f, -0.5f, -0.5f, // 0 + 0.5f, -0.5f, -0.5f, // 1 + 0.5f, 0.5f, -0.5f, // 2 + -0.5f, 0.5f, -0.5f, // 3 + -0.5f, -0.5f, 0.5f, // 4 + 0.5f, -0.5f, 0.5f, // 5 + 0.5f, 0.5f, 0.5f, // 6 + -0.5f, 0.5f, 0.5f // 7 + }; - uint32_t indices[] = { - 0, 1, 2, 2, 3, 0, // Front - 1, 5, 6, 6, 2, 1, // Right - 5, 4, 7, 7, 6, 5, // Back - 4, 0, 3, 3, 7, 4, // Left - 3, 2, 6, 6, 7, 3, // Top - 4, 5, 1, 1, 0, 4 // Bottom - }; + uint32_t indices[] = { + 4, 5, 6, 6, 7, 4, // Front + 1, 5, 6, 6, 2, 1, // Right + 0, 1, 2, 2, 3, 0, // Back + 4, 0, 3, 3, 7, 4, // Left + 3, 2, 6, 6, 7, 3, // Top + 0, 1, 5, 5, 4, 0 // Bottom + };src/TerrainSystem/TerrainSystem.cpp (1)
24-50: Consider using initialization list for member variables.The constructor could be optimized by moving member variable initializations to the initialization list.
Here's an improved version:
-TerrainSystem::TerrainSystem() : m_NoiseGen(std::random_device{}()) { - m_TerrainTexture = Texture::Create("assets/textures/terrain_atlas.png"); - m_TerrainShader = ShaderLibrary::LoadTexturedShader(); - if (!m_TerrainShader) { - LOG_ERROR("Failed to load textured shader for terrain"); - return; - } - m_TerrainMaterial = std::make_shared<Material>(m_TerrainShader); - m_TerrainMaterial->SetTexture("u_Texture", m_TerrainTexture); - m_TerrainMaterial->SetVector4("u_Color", glm::vec4(1.0f)); - m_TerrainTransform.SetPosition(-8.0f, -10.0f, -8.0f); - m_TerrainTransform.SetScale(1.0f, 1.0f, 1.0f); - m_BaseHeight = 0.0f; - m_HeightScale = 20.0f; - m_NoiseScale = 4.0f; - GenerateMesh(); +TerrainSystem::TerrainSystem() + : m_NoiseGen(std::random_device{}()) + , m_TerrainTexture(Texture::Create("assets/textures/terrain_atlas.png")) + , m_TerrainShader(ShaderLibrary::LoadTexturedShader()) + , m_BaseHeight(0.0f) + , m_HeightScale(20.0f) + , m_NoiseScale(4.0f) +{ + if (!m_TerrainShader) { + LOG_ERROR("Failed to load textured shader for terrain"); + return; + } + m_TerrainMaterial = std::make_shared<Material>(m_TerrainShader); + m_TerrainMaterial->SetTexture("u_Texture", m_TerrainTexture); + m_TerrainMaterial->SetVector4("u_Color", glm::vec4(1.0f)); + m_TerrainTransform.SetPosition(-8.0f, -10.0f, -8.0f); + m_TerrainTransform.SetScale(1.0f, 1.0f, 1.0f); + GenerateMesh(); }🧰 Tools
🪛 Cppcheck (2.10-2)
[performance] 26-26: Variable 'm_TerrainTexture' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (39)
.vs/CMakeWorkspaceSettings.json(1 hunks).vs/ProjectSettings.json(1 hunks).vs/VSWorkspaceState.json(1 hunks)CMakeLists.txt(3 hunks)assets/shaders/batch_renderer_2d.frag(1 hunks)assets/shaders/batch_renderer_2d.vert(1 hunks)imgui.ini(4 hunks)sandbox/assets/scripts/engine.lua(2 hunks)sandbox/assets/scripts/init.lua(4 hunks)sandbox/assets/scripts/main.lua(1 hunks)sandbox/src/SandboxApp.cpp(2 hunks)sandbox/src/SandboxApp.h(1 hunks)src/Application.cpp(4 hunks)src/Application.h(2 hunks)src/Camera/CameraTypes.h(1 hunks)src/Core/AssetManager.h(2 hunks)src/Core/Transform.h(1 hunks)src/Core/Utils/Logging.h(3 hunks)src/Input/InputSystem.h(1 hunks)src/Renderer/BatchRenderer2D.h(1 hunks)src/Renderer/IRenderer.h(1 hunks)src/Renderer/Renderer2D.cpp(1 hunks)src/Renderer/Renderer2D.h(1 hunks)src/Renderer/Texture.h(2 hunks)src/Scene/Scene.cpp(4 hunks)src/Scene/Scene.h(1 hunks)src/Scene/SceneManager.h(1 hunks)src/Scene/SceneObject.h(1 hunks)src/Scripting/LuaScriptSystem.cpp(9 hunks)src/Scripting/LuaScriptSystem.h(1 hunks)src/Shader/DefaultShaders.h(0 hunks)src/Shader/Shader.cpp(1 hunks)src/Shader/ShaderFactory.cpp(1 hunks)src/Shader/ShaderFactory.h(1 hunks)src/Shader/ShaderLibrary.cpp(1 hunks)src/Shader/ShaderLibrary.h(1 hunks)src/TerrainSystem/TerrainSystem.cpp(3 hunks)src/TerrainSystem/TerrainSystem.h(3 hunks)src/UI/ImGuiOverlay.cpp(1 hunks)
💤 Files with no reviewable changes (1)
- src/Shader/DefaultShaders.h
✅ Files skipped from review due to trivial changes (2)
- .vs/ProjectSettings.json
- .vs/CMakeWorkspaceSettings.json
🧰 Additional context used
🪛 Cppcheck (2.10-2)
src/TerrainSystem/TerrainSystem.cpp
[performance] 26-26: Variable 'm_TerrainTexture' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
src/Shader/ShaderFactory.cpp
[performance] 5-5: Variable 'm_Version' is assigned in constructor body. Consider performing initialization in initialization list.
(useInitializationList)
🔇 Additional comments (84)
src/Core/Transform.h (7)
5-6: Good Encapsulation of Transformation Logic
The introduction of a dedicatedTransformclass is a clean approach, providing a clear interface for manipulating position, rotation, and scale.
7-10: Leverage Nested Data Struct for Readability
Using a nestedTransformDatastruct is a nice way to keep all transformation properties grouped and maintain readability.
15-16: Straightforward Accessors
Returning references toTransformDataallows direct manipulation when needed. Make sure this is used carefully, as external code can modify these fields without setter checks.
19-21: Position Setters and Getter
These methods are concise and consistent. Good job using both float parameters and aglm::vec3overload.
24-26: Rotation Setters and Getter
Rotation logic is straightforward. Confirm any usage always expects rotation in degrees, since you’re converting to radians internally.
29-31: Scale Setters and Getter
The scale logic looks good. Consider whether negative scaling or non-uniform scaling might be used, and if so, ensure it’s supported throughout the codebase.
48-49: Keep Data Private for Safety
Using a private memberTransformData m_Data;is a good approach to enforce encapsulation and method-based access.src/Scene/Scene.h (9)
5-5: Consistent Includes & Forward Declarations
Including headers for required dependencies in place of forward declarations clarifies the usage ofCameraType,SceneObject, and related classes. This aids in maintainability.Also applies to: 7-7
13-13: Scene Class Introduction
Centralizing scene logic into theSceneclass is an excellent move for better lifecycle management and a more modular design.
15-16: Documented Constructors
Well-documented constructor helps clarify how aSceneis initialized. The Doxygen-style comments are a good practice.Also applies to: 23-24
26-29: Lifecycle Hooks Declared
Methods likeOnCreate,OnActivate,OnDeactivate,OnUpdate, andOnRenderare clearly designated for different stages in a scene’s lifecycle. Be sure to provide robust implementations later to avoid confusion.Also applies to: 32-39
49-49: Getter for Scene Name
Short inline getter is succinct.
52-56: Terrain System Getter
Straightforward accessor. Ensure that any usage checks fornullptrifm_TerrainSystemis not created or disabled.
57-61: Camera & Terrain Settings
ProvidingSetCameraTypeandSetTerrainEnabledmethods is a clear approach to controlling scene-wide rendering features. The logging statement inSetCameraTypeis also a good debugging aid.Also applies to: 63-66
88-92: Adding Objects
Simple push_back approach is fine. Make sure you handle duplicates or rename scenarios if needed down the line.
95-101: Private Member Organization
Organizing private members likem_Name,m_RootObject, andm_TerrainSystemin one block is neat. Good job grouping new data, likem_EnableTerrainandm_CameraType, to keep them easy to find.Also applies to: 103-110
src/Input/InputSystem.h (8)
2-2: Logical Inclusion Order
IncludingGLFW/glfw3.hand then engine-specific headers clarifies dependencies. This eliminates potential re-declaration or ordering issues in some compilers.Also applies to: 8-8
17-19: InputSystem Constructor
Using constructor injection withWindow* windowandRenderer& rendereris an excellent way to manage dependencies.Also applies to: 24-24
32-32: Event Callback Flow
OnEvent(Event& e)and additional specialized methods (e.g.,OnKey) neatly separate event types. This design fosters future event expansions without cluttering the update loop.Also applies to: 34-34, 36-36, 38-38
44-44: Key/Mouse Queries & Mouse Position
The direct checks through GLFW are straightforward. Combined with the new property-based approach (e.g., toggles for camera), it ensures flexible input usage.Also applies to: 51-51, 57-57
69-74: Camera & Movement Toggles
Toggling camera and movement states is convenient. For complex systems, ensure the toggles are not left in ambiguous states if your logic depends on multiple toggled flags.
76-80: Key Callback System
Storing callbacks inm_KeyCallbacksis a helpful approach, providing a flexible hooking mechanism. Ensure a robust cleanup or reassignment strategy if keys are re-registered.
82-100: Private Methods for Input Handling
Encapsulating operations likeHandleMouseMovementandHandleKeyInputkeepsUpdatereadable. This separation of concerns helps maintain code clarity.
101-147: Well-Structured Member Variables
Grouping camera flags, movement speed, and smoothing parameters logically is helpful for future maintainability. Documenting each variable also aids new contributors.src/Application.h (6)
5-5: Use of new camera header looks good.
The inclusion of"Camera/CameraTypes.h"helps ensure type-safety and clarity for camera operations.
19-20: Forward declaration is consistent.
The forward declaration ofCameraTypeis a clean approach, particularly when paired with the included header to provide the full definition.
22-22: Enumeration approach is sufficiently clear.
DefiningRenderTypewithRender2DandRender3Das distinct enum values is straightforward and improves readability.
27-31: KeyToggleState struct is concise and well-documented.
The added fields effectively store toggle state information. No issues observed.
61-66: Render type setter and getter are coherent.
ConfigureForRenderType()is presumably defined externally to finalize the setup. Please confirm it properly updates and re-initializes render components as needed.
120-159: Toggle state management is well-structured.
The methods for adding, removing, and retrieving toggle states centralize input handling logic. If concurrency or multi-threaded input processing becomes a requirement, ensure proper synchronization around shared data.sandbox/assets/scripts/main.lua (9)
1-3: Initialization guard is a good safeguard.
Preventing multiple loads of the same script helps avoid re-initializing and potential double registration.
39-75: initialize3DScene logic is sound.
Properly sets camera to perspective, positions it, and creates a test cube. Looks good for basic 3D initialization.
77-89: Scene initialization gating is effective.
Using_G.sceneInitializedprevents multiple scene setups. The branching logic for 2D vs. 3D is well-structured.
92-94: Debug mode logging is useful.
Enabling FPS display will assist in performance tuning.
105-129: Checkerboard rendering is nicely implemented.
The approach of iterating rows/cols to draw quads is simple and effective. The color cycling logic is a neat aesthetic touch.
159-171: Bouncing quads logic is well-contained.
Sinusoidal updates for vertical offset are straightforward. Make sure performance remains acceptable if many objects are introduced.
173-193: Texture creation strategy is memory-friendly.
Lazily creating the checker texture ensures only one instance in memory. The parametertilingFactorsapproach is flexible.
195-241: UpdateScene handles the 2D/3D split well.
Rotating the camera in 3D and orchestrating the 2D pipeline with separate scene-begin/scene-end calls is a robust approach. Carefully ensure transitions between modes re-initialize or reset as needed.
243-249: createCheckerTexture function is consistent with lazy initialization.
No concerns; it ensures the texture is created only once.src/Scene/Scene.cpp (4)
60-60: String concatenation in logging is consistent.
Switching toLOG_TRACE_CONCATaligns with established logging macros and provides clearer output.
110-115: OnCreate doc clarifies terrain creation logic.
It’s good that you’re explicitly stating that terrain is conditionally built. Documentation sets clear expectations.
119-122: Conditional terrain setup is well-structured.
The check form_EnableTerrain && !m_TerrainSystemensures no redundant creation. This is a clear improvement.
215-229: RenderObject is robust with null checks.
Skipping objects without mesh/material is flexible. The recursive call approach for children is straightforward.src/Renderer/Renderer2D.cpp (14)
12-19: Struct design looks good.
TheQuadVertexstruct cleanly captures the required attributes for 2D rendering (position, color, texture coordinates, texture index, tiling factor). This is straightforward and appears sufficient for typical 2D sprite rendering.
20-39: Large static arrays may pose memory constraints.
DefiningMaxQuadsas 100k leads to 400k vertices and 600k indices. While this may be acceptable for many use cases, it could be large for memory-constrained environments.Would you like to gather usage data of typical max quads in your application? I can provide an automated script to scan for typical draw calls or buffer usage across the codebase.
43-59: Initialization steps look logical.
The creation of theVertexArrayandVertexBufferis consistent. The buffer layout correctly matches theQuadVertexdata structure. This lays a clean foundation for 2D batch rendering.
77-86: Robust shader creation check.
You handle the possibility of a null shader with an error log and early return, which is good. Ensure higher-level systems also handle this failure gracefully.
87-106: Texture sampling parameters are properly set.
UsingGL_LINEARfiltering is standard for 2D sprite rendering. The creation of a 1×1 white texture to serve as a default is a good approach.
107-109: Reminder on potential heap usage.
Allocating 400kQuadVertexobjects on the heap is valid, but consider verifying this usage does not exceed memory limits on low-end systems. Otherwise, no issues here.
111-116: Clean resource teardown at shutdown.
Callingdelete[]on theQuadVertexBufferBasepointer is correct. The defaultBeginScene()is empty, which is acceptable if the camera-based overload is primarily used.
117-127: Camera-based scene initiation.
Binding the shader and setting the view-projection matrix is standard practice. InvokingStartBatch()ensures a fresh batch for each new scene.
129-136: Scene finalization.
EndScene()performs aFlush()which is a consistent pipeline step. The approach is simple and easy to follow.
137-156: Efficient flush and stats tracking.
You batch-update the GPU buffer once per draw call group and bind textures in a loop. The stats increment logic is a nice addition for profiling. Looks good overall.
158-185: Appropriate use of transform matrices.
TheDrawQuadapproach of combining translation and scaling is a standard approach. Tiling factor and white texture usage are clear. The loop that sets vertex attributes is correct and minimal.
187-192: Overloaded function for convenience.
Forwarding 2D positions to the 3D overload is a neat trick for code reuse. Looks concise and consistent.
193-228: Texture slot management logic is solid.
You check if the texture is already bound; if not, allocate a new slot and add it. This is an effective approach for a batch-based texture system. Watch for edge cases if texture slot limits are exceeded frequently.
230-233: Resets the batch.
NextBatch()callingFlush()followed byStartBatch()is a clear pattern for continuing the pipeline when capacity is reached. Works well in a batch renderer design.src/Core/Utils/Logging.h (2)
54-79: Windows-specific logging class is well-structured.
The singleton pattern provides a convenient entry point for logging. TheLogConcattemplate function leveraging a stringstream is straightforward and effective for simple concatenation.
120-171: Convenience macros and variable logging.
All macros to handle trace, info, warn, error, etc., plus variable logging, provide a flexible approach. TheLogValuesfunction is especially powerful for printing multiple variables in one call. This is well done.sandbox/assets/scripts/engine.lua (3)
1-1: Lua annotation line is consistent.
Using LuaDoc for clarifying that all numbers are floats may help with editor inference and code assistance. Looks fine.
5-8: Script reorganization is clear.
Moving terrain and logging functionalities under distinct headings helps keep the engine API well-documented. This fosters readability for script users.
285-309: Comprehensive 2D rendering interface.
These functions (renderer2d_begin_scene,renderer2d_end_scene,draw_quad,draw_textured_quad) align with your new batch-based 2D renderer. The added LuaDoc is thorough, aiding script authors.src/Application.cpp (2)
114-114: Overlay initialization looks fine.The creation of
m_ImGuiOverlayusingstd::make_uniquealigns well with the existing pattern. No immediate concerns.
316-341: Verify availability of 'UpdateScene' function in Lua.The added
m_ScriptSystem->CallGlobalFunction("UpdateScene")call is contingent on a matching Lua global function. Consider robust error handling if the function is missing.✅ Verification successful
Verified: 'UpdateScene' Lua function exists
- The Lua function
UpdateSceneis defined insandbox/assets/scripts/main.lua.- No further action is required for this call as the global function is available.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if UpdateScene is defined in any Lua scripts rg -A 10 "function UpdateScene"Length of output: 755
src/Scripting/LuaScriptSystem.cpp (3)
7-14: New includes are appropriate.Including
Transform,SceneObject,Renderer2D, andShaderLibrarybroadens the Lua API accessibility. This is logically consistent with the expanded scripting features.
22-49: Documentation improvements are clear.The extended doc blocks for constructors and destructors are helpful. Nicely conveys initialization and cleanup details to maintainers.
95-127: Smooth integration for glm::vec3 and Transform.Registering
glm::vec3andTransformwithsol::overloadfosters flexible Lua usage. The approach looks sound.src/Camera/CameraTypes.h (1)
1-6: Enum class is a concise, type-safe choice.Defining
CameraTypeas an enum class in theEnginenamespace clarifies camera management.sandbox/src/SandboxApp.h (1)
12-13: LGTM! Good use of smart pointers for camera management.The addition of the orthographic camera using
std::shared_ptris a good choice for memory management and ownership semantics.assets/shaders/batch_renderer_2d.vert (1)
3-7: LGTM! Well-structured vertex attributes.The input attributes are properly defined with explicit locations, making the shader interface clear and maintainable.
src/Renderer/IRenderer.h (1)
5-14: LGTM! Well-designed renderer interface.The interface follows SOLID principles and provides a clear contract for renderer implementations.
src/Shader/ShaderFactory.h (1)
7-16: LGTM! Excellent use of builder pattern.The factory implementation provides a fluent interface through method chaining and clear separation of shader components.
src/Shader/ShaderLibrary.h (1)
9-17: LGTM! Well-designed shader management interface.The class provides a clean and efficient interface for shader management with appropriate use of shared pointers for resource handling.
src/Renderer/Renderer2D.h (1)
16-19: Consider thread safety in singleton implementation.The current singleton implementation might not be thread-safe in all scenarios. Consider using double-checked locking or call_once for thread-safe initialization.
// Option 1: Using std::call_once class Renderer2D : public IRenderer { public: static Renderer2D& Get() { static std::once_flag flag; std::call_once(flag, []() { instance = new Renderer2D(); }); return *instance; } private: static Renderer2D* instance; };src/Shader/Shader.cpp (2)
180-181: LGTM! Good improvement in memory management.The change to use
std::make_sharedinstead of rawnewis a better practice as it:
- Ensures exception safety
- Prevents memory leaks
- Is more efficient by combining allocation of control block and object
181-182: LGTM! Better validation approach.Using
GetProgram()for validation is a more direct way to check shader creation success, as it verifies the actual program handle rather than just the loading process.sandbox/src/SandboxApp.cpp (2)
12-14: LGTM! Good use of constants.Moving magic numbers to named constants improves code maintainability and readability.
31-42: LGTM! Better initialization flow.The script-first approach is a good design choice as it:
- Allows dynamic configuration of rendering type
- Provides more flexibility in setup
- Aligns with the shader factory restructuring objective
src/TerrainSystem/TerrainSystem.cpp (1)
29-34: LGTM! Good error handling.The addition of proper error handling for shader loading improves robustness and helps with debugging.
src/UI/ImGuiOverlay.cpp (1)
142-150: LGTM! Clean implementation of transform controls.The refactored code improves encapsulation by using the Transform class's GetData method to access transform data directly. The implementation is concise and follows best practices.
.vs/VSWorkspaceState.json (2)
14-19: LGTM! Added Ubuntu-22.04 WSL target with proper build configurations.The configuration correctly includes both Debug and Release build paths.
22-31: LGTM! Enhanced workspace navigation configuration.The expanded nodes provide quick access to key project directories, and the selected node is appropriately set to the root CMakeLists.txt.
imgui.ini (1)
70-73: LGTM! Added Renderer Stats window with appropriate dimensions.The new window is positioned for good visibility without overlapping other UI elements.
CMakeLists.txt (1)
63-63: LGTM! Added new source files for enhanced rendering and shader management.The additions of Renderer2D, ShaderLibrary, and ShaderFactory align well with the PR's objective of creating a shader factory.
Also applies to: 88-89
| /** | ||
| * @brief Logs a formatted message | ||
| * @tparam Args Types of format arguments | ||
| * @param level Severity level of the message | ||
| * @param format Format string | ||
| * @param args Format arguments | ||
| */ | ||
| template <typename... Args> | ||
| void LogFormat(LogLevel level, const char* format, Args... args) { | ||
| Log(level, format); // For now, just output the format string | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Format string usage is only partial.
Currently, LogFormat just logs the format string without consuming arguments. This might be a placeholder. Consider implementing a proper format expansion or removing the variadic signature if it's not yet needed.
void LogFormat(LogLevel level, const char* format, Args... args) {
- Log(level, format); // For now, just output the format string
+ // Example usage: implement a formatting library or a custom vsnprintf-based approach
+ char buffer[256];
+ std::snprintf(buffer, sizeof(buffer), format, args...);
+ Log(level, buffer);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @brief Logs a formatted message | |
| * @tparam Args Types of format arguments | |
| * @param level Severity level of the message | |
| * @param format Format string | |
| * @param args Format arguments | |
| */ | |
| template <typename... Args> | |
| void LogFormat(LogLevel level, const char* format, Args... args) { | |
| Log(level, format); // For now, just output the format string | |
| } | |
| /** | |
| * @brief Logs a formatted message | |
| * @tparam Args Types of format arguments | |
| * @param level Severity level of the message | |
| * @param format Format string | |
| * @param args Format arguments | |
| */ | |
| template <typename... Args> | |
| void LogFormat(LogLevel level, const char* format, Args... args) { | |
| // Example usage: implement a formatting library or a custom vsnprintf-based approach | |
| char buffer[256]; | |
| std::snprintf(buffer, sizeof(buffer), format, args...); | |
| Log(level, buffer); | |
| } |
| @@ -20,7 +20,7 @@ | |||
| #include "UI/ImGuiOverlay.h" | |||
|
|
|||
| // Shader system | |||
| #include "Shader/DefaultShaders.h" | |||
| #include "Scene/Scene.h" | |||
There was a problem hiding this comment.
💡 Codebase verification
Detected lingering reference to DefaultShaders in src/Scripting/LuaScriptSystem.cpp
The search identified a forward declaration for DefaultShaders in src/Scripting/LuaScriptSystem.cpp. This indicates that not all references have been removed despite the update in Application.cpp.
- src/Scripting/LuaScriptSystem.cpp: Contains a forward declaration (
class DefaultShaders;) along with a comment ("// Forward declare DefaultShaders").
Please review whether this reference is still required or if it should be removed or updated to align with the new shader implementation.
🔗 Analysis chain
Ensure thorough removal of old shader references.
The prior approach used DefaultShaders.h, which is now replaced by Scene/Scene.h. If DefaultShaders is fully deprecated, verify no orphaned references remain across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searches for remaining references to DefaultShaders
rg -A 2 "DefaultShaders"
Length of output: 221
| private: | ||
| std::string m_Version; | ||
| std::string m_VertexInputs; | ||
| std::string m_FragmentOutputs; | ||
| std::string m_Uniforms; | ||
| std::string m_VertexMain; | ||
| std::string m_FragmentMain; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for shader components.
The private members could benefit from validation to ensure all required components are set before building.
private:
std::string m_Version;
std::string m_VertexInputs;
std::string m_FragmentOutputs;
std::string m_Uniforms;
std::string m_VertexMain;
std::string m_FragmentMain;
+
+ bool ValidateComponents() const {
+ return !m_Version.empty() &&
+ !m_VertexMain.empty() &&
+ !m_FragmentMain.empty();
+ }Then update the Build method to use this validation:
std::shared_ptr<Shader> Build() {
if (!ValidateComponents()) {
throw std::runtime_error("Missing required shader components");
}
// ... existing build logic
}| ShaderFactory& ShaderFactory::AddVertexInput(const std::string& input) { | ||
| m_VertexInputs += input + "\n"; | ||
| return *this; | ||
| } | ||
|
|
||
| ShaderFactory& ShaderFactory::AddFragmentOutput(const std::string& output) { | ||
| m_FragmentOutputs += output + "\n"; | ||
| return *this; | ||
| } | ||
|
|
||
| ShaderFactory& ShaderFactory::AddUniform(const std::string& uniform) { | ||
| m_Uniforms += uniform + "\n"; | ||
| return *this; | ||
| } | ||
|
|
||
| ShaderFactory& ShaderFactory::SetVertexMain(const std::string& mainCode) { | ||
| m_VertexMain = mainCode; | ||
| return *this; | ||
| } | ||
|
|
||
| ShaderFactory& ShaderFactory::SetFragmentMain(const std::string& mainCode) { | ||
| m_FragmentMain = mainCode; | ||
| return *this; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation for shader components.
Consider validating inputs to prevent empty or malformed shader components. For example:
ShaderFactory& ShaderFactory::AddVertexInput(const std::string& input) {
+ if (input.empty()) {
+ throw std::invalid_argument("Vertex input cannot be empty");
+ }
m_VertexInputs += input + "\n";
return *this;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ShaderFactory& ShaderFactory::AddVertexInput(const std::string& input) { | |
| m_VertexInputs += input + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::AddFragmentOutput(const std::string& output) { | |
| m_FragmentOutputs += output + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::AddUniform(const std::string& uniform) { | |
| m_Uniforms += uniform + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::SetVertexMain(const std::string& mainCode) { | |
| m_VertexMain = mainCode; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::SetFragmentMain(const std::string& mainCode) { | |
| m_FragmentMain = mainCode; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::AddVertexInput(const std::string& input) { | |
| if (input.empty()) { | |
| throw std::invalid_argument("Vertex input cannot be empty"); | |
| } | |
| m_VertexInputs += input + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::AddFragmentOutput(const std::string& output) { | |
| m_FragmentOutputs += output + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::AddUniform(const std::string& uniform) { | |
| m_Uniforms += uniform + "\n"; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::SetVertexMain(const std::string& mainCode) { | |
| m_VertexMain = mainCode; | |
| return *this; | |
| } | |
| ShaderFactory& ShaderFactory::SetFragmentMain(const std::string& mainCode) { | |
| m_FragmentMain = mainCode; | |
| return *this; | |
| } |
| engine.warn(string.format("Invalid script name (potential path traversal): %s", name)) | ||
| return false |
There was a problem hiding this comment.
Fix string concatenation in error messages.
The string.format calls have mismatched format specifiers and arguments.
- engine.warn(string.format("Could not open source script ", sourcePath, ": ", sourceErr))
+ engine.warn(string.format("Could not open source script %s: %s", sourcePath, sourceErr))
- engine.warn(string.format("Failed to read from ", sourcePath))
+ engine.warn(string.format("Failed to read from %s", sourcePath))
- engine.warn(string.format("Failed to create destination file", destPath, ": ", destErr))
+ engine.warn(string.format("Failed to create destination file %s: %s", destPath, destErr))
- engine.warn(string.format("Failed to write to ", destPath))
+ engine.warn(string.format("Failed to write to %s", destPath))Also applies to: 39-40, 56-59, 63-66, 70-73
🛠️ Refactor suggestion
Reconsider using warnings for critical failures.
Several critical operations are now using warnings instead of errors, which might hide important issues:
- File open failures
- Read/write failures
- Size limit violations
Consider keeping errors for truly critical failures while using warnings for non-critical issues.
Also applies to: 39-40, 48-52, 56-59, 63-66, 70-73
| void Shutdown() { | ||
| // Clean up terrain resources | ||
| m_TerrainMesh.reset(); | ||
| m_TerrainMaterial.reset(); | ||
| m_IsInitialized = false; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding virtual destructor and RAII pattern.
The Shutdown method could be moved to the destructor to ensure proper cleanup. Also, consider making the destructor virtual since the class might be inherited.
-~TerrainSystem() = default;
+virtual ~TerrainSystem() {
+ Shutdown();
+}
-void Shutdown() {
+virtual void Shutdown() {
// Clean up terrain resources
m_TerrainMesh.reset();
m_TerrainMaterial.reset();
m_IsInitialized = false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void Shutdown() { | |
| // Clean up terrain resources | |
| m_TerrainMesh.reset(); | |
| m_TerrainMaterial.reset(); | |
| m_IsInitialized = false; | |
| } | |
| virtual ~TerrainSystem() { | |
| Shutdown(); | |
| } | |
| virtual void Shutdown() { | |
| // Clean up terrain resources | |
| m_TerrainMesh.reset(); | |
| m_TerrainMaterial.reset(); | |
| m_IsInitialized = false; | |
| } |
| @@ -94,14 +97,14 @@ target_include_directories(voxel-engine PUBLIC | |||
| ${CMAKE_SOURCE_DIR}/src | |||
| ${IMGUI_DIR} | |||
| ${IMGUI_DIR}/backends | |||
| ${CMAKE_SOURCE_DIR}/build/vcpkg_installed/x86-windows/include | |||
There was a problem hiding this comment.
Remove duplicate include directory entry.
The same include path ${CMAKE_SOURCE_DIR}/build/vcpkg_installed/x86-windows/include is added twice.
Apply this diff to fix the redundancy:
target_include_directories(voxel-engine PUBLIC
${CMAKE_SOURCE_DIR}/src
${IMGUI_DIR}
${IMGUI_DIR}/backends
${CMAKE_SOURCE_DIR}/build/vcpkg_installed/x86-windows/include
${CMAKE_SOURCE_DIR}/external/stb_image/src
${CMAKE_SOURCE_DIR}/external/json
${CMAKE_SOURCE_DIR}/external/glad/include
- ${CMAKE_SOURCE_DIR}/build/vcpkg_installed/x86-windows/include
${CMAKE_SOURCE_DIR}/external/sol2
)Also applies to: 104-104
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
factory for shader creation
Summary by CodeRabbit
New Features
BatchRenderer2Dclass for efficient 2D rendering with batching support.ShaderFactoryclass for modular shader creation.ShaderLibraryclass for managing shader creation and retrieval.Shutdownmethod in theTerrainSystemclass for proper resource management.Refactor
Transformentity for better encapsulation and data management.