Skip to content

Fix runtime issue with renderer initialization - #146

Merged
Av3boy merged 3 commits into
mainfrom
checkpoint/multiple-windows-build-main
Sep 13, 2026
Merged

Av3boy merged 3 commits into
mainfrom
checkpoint/multiple-windows-build-main

Conversation

@Av3boy

@Av3boy Av3boy commented Sep 13, 2026

Copy link
Copy Markdown
Owner

Fix runtime issue

Contents

The contents of this PR already have been fixed on other branches, but to make the main branch work in some degree, this PR was created.

Checklist

  • I have merged the latest changes from main to my branch.
  • I have tested my changes and any affected components.
  • I have added the proper documentation about my changes
  • I have made sure there is no overlapping work.
  • I have discussed any / all issues brought up from code review.

@Av3boy Av3boy added this to the SharpEngine 1.0 MVP milestone Sep 13, 2026
@Av3boy Av3boy self-assigned this Sep 13, 2026
Copilot AI lite review requested due to automatic review settings September 13, 2026 12:57
@Av3boy Av3boy added the bug Something isn't working label Sep 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Update the parameter documentation and remove the generated imgui.ini file.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds the missing Renderer constructor for runtime initialization and includes an ImGui state file.

Changes:

  • Adds a three-argument Renderer constructor.
  • Adds persisted ImGui layout state.
File summaries
File Summary Review
SharpEngine.Core/Renderers/Renderer.cs Enables renderer construction. Correct the parameter documentation to describe the camera.
imgui.ini Adds persisted ImGui layout state. Remove the generated file and ignore imgui.ini.
Review details

Suppressed comments (1)

SharpEngine.Core/Renderers/Renderer.cs:45

  • The parameter documentation is inaccurate: this parameter is the camera, not the game. Please describe it as the camera used by the renderer so the generated public API documentation is not misleading.
    /// <param name="camera">The game the renderer is being used for.</param>
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread imgui.ini Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Remove tracked imgui.ini and correct the constructor documentation.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

SharpEngine.Core/Renderers/Renderer.cs:45

  • This public XML documentation describes camera as the game, but the parameter is a CameraView; generated API docs will mislead callers about what this constructor accepts. Please document the camera passed to the renderer.
  • Files reviewed: 1/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@sonarqubecloud

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The remaining documentation nit is non-blocking.

Review details

Suppressed comments (1)

SharpEngine.Core/Renderers/Renderer.cs:45

  • This new public overload documents camera as "the game", but the parameter is a CameraView; this makes the generated API documentation misleading. Describe it as the camera used by the renderer, consistent with Window's constructor documentation.
    /// <param name="camera">The game the renderer is being used for.</param>
  • Files reviewed: 1/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@Av3boy
Av3boy merged commit d751b44 into main Sep 13, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants