Repository navigation
fix: compile against UE 5.8 - #1
Merged
Merged
Conversation
UE 5.8 changed FJsonObject::Values to key by UE::TSharedString, and BuildSettingsVersion.V7 promotes unreachable-code warnings to errors. JSON keys (Bridge/MsgpackHelpers, RpcHandlers_Camera/Control/Step): - Iterate Values with `const auto&` instead of TPair<FString, ...>. - Convert keys with FString(*Key) where an FString is needed. This compiles on both 5.7 (FString keys) and 5.8. Unreachable code (-Wunreachable-code, -Wunreachable-code-loop-increment): - MjOrbitCameraActor: drop a dead `if (false) continue;`. - MjQuickConvertComponent, MjEditorOpHandlers, MjStateCollectorTests: replace for-loops that always break on the first element with a direct first-element check. Behavior is unchanged.
owenpkent
marked this pull request as draft
September 13, 2026 21:16
3 of 6 tasks
finger563
marked this pull request as ready for review
September 17, 2026 04:20
There was a problem hiding this comment.
🟡 Changes recommended
Two remaining direct FSharedString concatenations still prevent UE 5.8 compilation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates URLab for Unreal Engine 5.8 compatibility while retaining 5.7 support.
Changes:
- Adapts JSON key handling to
FSharedString. - Removes unreachable and always-breaking loops.
- Preserves first-element behavior with direct checks.
File summaries
| File | Description |
|---|---|
Source/URLabEditor/Private/Tests/MjStateCollectorTests.cpp |
Replaces an always-breaking sensor loop. |
Source/URLabEditor/Private/MjEditorOpHandlers.cpp |
Directly checks the first manager iterator result. |
Source/URLab/Private/MuJoCo/Convert/MjQuickConvertComponent.cpp |
Directly uses the first visual hull. |
Source/URLab/Private/Cinematics/MjOrbitCameraActor.cpp |
Removes unreachable code. |
Source/URLab/Private/Bridge/RpcHandlers_Step.cpp |
Converts JSON keys for step/reset handling. |
Source/URLab/Private/Bridge/RpcHandlers_Control.cpp |
Updates JSON iteration and key conversion. |
Source/URLab/Private/Bridge/RpcHandlers_Camera.cpp |
Converts camera JSON keys to FString. |
Source/URLab/Private/Bridge/MsgpackHelpers.cpp |
Materializes JSON keys before MessagePack encoding. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Verification note, since the automated review landed on "changes recommended" and this PR is a dependency of rammp-org/ramms-sim#44. Both inline comments claim the branch still fails to compile on 5.8, at Checked by building, not by reading. Against UE 5.8.2 (
The PIE suites exercise the bridge paths these files serve, including the step and reset RPCs. One thing the review did get right indirectly: swapping the explicit 🤖 Generated with Claude Code |
finger563
added a commit
to rammp-org/ramms-sim
that referenced
this pull request
Sep 17, 2026
Both dependency PRs are merged, so the pointers move off the PR branch heads and onto the branches this superproject tracks: - unreal-robotics-lab b4ff4e3 -> b23d0f3, the merge of rammp-org/UnrealRoboticsLab#1 into ramms/v0.6.0-beta. - RammsCrowd 58a5802 -> ee84151, the squash of rammp-org/ramms-crowd#4 onto main. Both merged trees are byte-identical to the commits already verified, so this is a pointer move rather than a content change. Re-verified anyway on UE 5.8.2 (macOS/arm64): RammsEditor builds clean, run_chaos.sh PASS, run_mj_full.sh PASS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011dbmXppHLtQ8ukzeBgu7ZQ
finger563
added a commit
to rammp-org/ramms-sim
that referenced
this pull request
Sep 17, 2026
* chore(engine): upgrade the project to Unreal Engine 5.8
Verified against UE 5.8.2 on macOS/arm64: RammsEditor builds clean and both
PIE suites (run_chaos.sh, run_mj_full.sh) pass.
Project:
- Ramms.uproject: EngineAssociation 5.7 -> 5.8.
- Ramms.Target.cs / RammsEditor.Target.cs: BuildSettingsVersion V6 -> V7 and
EngineIncludeOrderVersion Unreal5_7 -> Unreal5_8. V7 is what a binary 5.8
engine expects; it also promotes the ReturnType / Dangling / UnreachableCode
warning levels to errors.
- Ramms.Build.cs: the include path "Ramms/Variant_Offroad" did not match the
folder on disk ("Variant_OffRoad"). 5.8 no longer tolerates the mismatch.
(Same fix as #41, which found it on case-sensitive file systems.)
Submodules, both pointing at the fix branches of the open PRs:
- unreal-robotics-lab -> b4ff4e3 (rammp-org/UnrealRoboticsLab#1): FJsonObject
key type and unreachable-code fixes.
- RammsCrowd -> 58a5802 (rammp-org/ramms-crowd#4): 5.8 moved the Mass element
base types into the new engine module MassCore, so the crowd fragments and
tags no longer linked.
Docs and scripts: every actionable "UE 5.7" reference bumped (README, CLAUDE.md,
copilot-instructions, the Linux build/package scripts, setup_urlab.sh, the
Pixel Streaming and parallel-sim docs, the dojo asset pipeline). Two facts were
re-verified against the installed 5.8 engine rather than assumed: the FBX
Interchange default is still on, and reload_packages still returns the same
tuple. PIXEL_STREAMING_PLAN records one real change - 5.8 adds Android and IOS
to the PixelStreaming2 module allowlists, but still not LinuxArm64.
run_mj_full.sh gets its executable bit, which it was missing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dbmXppHLtQ8ukzeBgu7ZQ
* chore(deps): point the 5.8 submodules at their merged commits
Both dependency PRs are merged, so the pointers move off the PR branch heads
and onto the branches this superproject tracks:
- unreal-robotics-lab b4ff4e3 -> b23d0f3, the merge of
rammp-org/UnrealRoboticsLab#1 into ramms/v0.6.0-beta.
- RammsCrowd 58a5802 -> ee84151, the squash of rammp-org/ramms-crowd#4 onto
main.
Both merged trees are byte-identical to the commits already verified, so this
is a pointer move rather than a content change. Re-verified anyway on UE 5.8.2
(macOS/arm64): RammsEditor builds clean, run_chaos.sh PASS, run_mj_full.sh PASS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dbmXppHLtQ8ukzeBgu7ZQ
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
URLab on
ramms/v0.6.0-betadoes not compile against UE 5.8. This fixes the errors that blockRammsEditorfor rammp-org/ramms-sim#18 (the 5.8 upgrade).FJsonObject::Valuesto key byUE::TSharedString. Loops now useconst auto&, and keys are converted withFString(*Key)where anFStringis needed. This form compiles on 5.7 too.BuildSettingsVersion.V7promotes-Wunreachable-codeto an error. Removed a deadif (false) continue;and replaced three always-break loops with a direct first-element check. Behavior is unchanged..clang-format(clang-format 20).Touched:
Bridge/MsgpackHelpers,Bridge/RpcHandlers_{Camera,Control,Step},Cinematics/MjOrbitCameraActor,Convert/MjQuickConvertComponent,URLabEditor/MjEditorOpHandlers,URLabEditor/Tests/MjStateCollectorTests.After merging
Bump
Plugins/unreal-robotics-labin ramms-sim, in rammp-org/ramms-sim#18 or right after it merges.Test plan
RammsEditorbuilds on Linux (Ubuntu 22.04) with UE 5.8.2 and bundled clang 20.1.8, and the editor opens the project.