Skip to content

Remove client-engine MTP CI overrides - #5955

Open
Mackamuir wants to merge 1 commit into
ss14Starlight:starlight-devfrom
Mackamuir:move_SRT_MTP
Open

Remove client-engine MTP CI overrides#5955
Mackamuir wants to merge 1 commit into
ss14Starlight:starlight-devfrom
Mackamuir:move_SRT_MTP

Conversation

@Mackamuir

Copy link
Copy Markdown
Contributor

REQUIRES ss14Starlight/RobustToolbox#24

Short description

Cleans up some of the overrides I had to do to get MTP working, since MTP is now supported in the engine.

Why we need to add this

Just general cleanup and parity

Checks

  • I do not require assistance to complete the PR.
  • Before posting/requesting review of a PR, I have verified that the changes work.
  • I have added screenshots/videos of the changes, or this PR does not change in-game mechanics.
  • I affirm that my changes are licensed under the MIT License and grant permission for use in this repository under its conditions.

@Mackamuir
Mackamuir requested a review from a team September 3, 2026 01:12
@github-actions github-actions Bot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. Changes: No C# size/S S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. labels Sep 3, 2026
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 83cffd3e-4462-43c0-9505-2575c84a0659

📥 Commits

Reviewing files that changed from the base of the PR and between 3637d90 and 8bb0dbc.

📒 Files selected for processing (2)
  • Directory.Build.props
  • Directory.Build.targets
💤 Files with no reviewable changes (2)
  • Directory.Build.props
  • Directory.Build.targets

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


📝 Summary

Summary

  • Removes the global NUnit runner configuration from Directory.Build.props.
  • Removes the late NUnit runner configuration and explicit NUnit3.TestAdapter reference from Directory.Build.targets.
  • Keeps the Reviewdog configuration unchanged.
  • Supports the required RobustToolbox MTP integration. This removes client-engine MTP overrides without changing in-game mechanics.

User impact

Players should see no gameplay change. The change aligns the client with engine-provided MTP support.

Risk areas

  • Test projects may no longer receive the NUnit runner and adapter configuration from these shared build files.
  • The change depends on RobustToolbox PR #24 for engine-level MTP support.

Guideline gaps

  • The available PR context does not identify the exact client-engine MTP overrides removed.
  • The available PR context does not explain where the removed MTP behavior now resides in the Shared or engine code.
  • The available PR context does not include ownership comments for Starlight-specific code.
  • The available PR context does not address localization, breaking changes, or design impact.

Walkthrough

The build configuration no longer enables the NUnit runner globally or conditionally. The explicit NUnit3.TestAdapter reference was removed. Reviewdog configuration remains.

Changes

NUnit runner configuration removal

Layer / File(s) Summary
Remove NUnit build configuration
Directory.Build.props, Directory.Build.targets
Global and conditional NUnit runner settings, adapter versioning, generated executable settings, and the explicit adapter reference were removed. Reviewdog settings remain active.

Estimated code review effort: 1 (Trivial) | ~5 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description explains that the pull request removes MTP-related client-engine overrides and performs cleanup after engine support was added. This matches the changes to NUnit runner configuration.
Title check ✅ Passed The title is concise and identifies the removal of client-engine MTP CI overrides. This matches the removal of NUnit runner and adapter configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Content + Integration Test Results

Tests 📝 Passed ✅ Failed ❌ Duration ⏱️
1697 1677 0 12m 21s

🎉 No failed tests in this run.

Github Test Reporter by CTRF 💚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changes: No C# S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant