fix(input): stop crash when pressing Ctrl/Tab with no default bind

[soloturn]

Fixes #5224 - pressing RIGHT_CTRL (or LEFT_CTRL/TAB once something else already claims the key) crashes with IllegalArgumentException: moduleName must not be null or empty.

Root cause

SortOrderSystem.postBegin() creates fallback BindableButtonImpl instances for RIGHT_CTRL, LEFT_CTRL and TAB whenever BindsManager.getKeyBinds() doesn't already have an entry for that physical key - the common case, since only LEFT_CTRL has a @DefaultBinding (CrouchButton). Each fallback built its id with the single-arg constructor:

new SimpleUri("ctrlMod")
new SimpleUri("changeFocus")

SimpleUri(String) only populates moduleName/objectName when the string contains a : separator; without one it silently returns an invalid URI (both names empty) instead of failing fast:

public SimpleUri(String simpleUri) {
    if (simpleUri == null) { return; }
    String[] split = simpleUri.split(MODULE_SEPARATOR, 2);
    if (split.length > 1) {
        moduleName = new Name(split[0]);
        objectName = new Name(split[1]);
    }
}

The bind system doesn't notice - BindableButtonImpl just carries the id around - until NUIManagerInternal.bindEvent() calls event.getId().getModuleName()/getObjectName() straight into new ResourceUrn(...), which does validate and throws. Matches the stack trace in the issue exactly (bindEvent, BindableButtonImpl.updateBindState).

Fix

1. SortOrderSystem - build the ids with the proper SimpleUri(Name, String) constructor and TerasologyConstants.ENGINE_MODULE, the same convention the engine's own built-in binds use.
2. NUIManagerInternal.bindEvent() - added a defensive event.getId().isValid() guard that logs and returns instead of constructing the ResourceUrn, so any other malformed bind id (module code, not just this call site) degrades to a log line instead of taking down the input pipeline.

Verification

:engine:compileJava and :engine-tests:compileTestJava both clean. No existing test exercises SortOrderSystem or NUIManagerInternal.bindEvent directly - both need a live BindsManager/NUI event pipeline, not something unit-testable in isolation.

Root-caused and fixed by tracing the exact SimpleUri construction/validation path named in the issue's own stack trace, matching soloturn's comment confirming this still reproduces on current develop.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 86612f69-9fd4-4f00-8a2a-ea31814188ae

📥 Commits

Reviewing files that changed from the base of the PR and between 4d5b8ce and 0ffe98a.

📒 Files selected for processing (1)

• engine/src/main/java/org/terasology/engine/rendering/nui/SortOrderSystem.java

🚧 Files skipped from review as they are similar to previous changes (1)

• engine/src/main/java/org/terasology/engine/rendering/nui/SortOrderSystem.java

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Summary by CodeRabbit

• Bug Fixes
  • Improved keyboard shortcut handling for Right Ctrl, Left Ctrl, and Tab.
  • Invalid input binding identifiers are now safely ignored instead of causing malformed event processing.
  • Added clearer logging when an invalid binding is encountered.

Walkthrough

The change scopes fallback key-binding identifiers to the engine module and rejects invalid bind IDs before NUI bind-event processing.

Changes

Bind Identifier Handling

Layer / File(s)	Summary
Engine-scoped fallback bindings engine/src/main/java/org/terasology/engine/rendering/nui/SortOrderSystem.java	Fallback bindings for Right Ctrl, Left Ctrl, and Tab now use engine-scoped SimpleUri values.
Bind ID validation engine/src/main/java/org/terasology/engine/rendering/nui/internal/NUIManagerInternal.java	Invalid bind IDs are logged and ignored before ResourceUrn creation and bind-event processing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 0ffe9

This change prevents malformed input bindings from crashing the input pipeline and safely ignores invalid identifiers. No actionable merge-blocking risk remains after normal checks and review.

Poem

I’m a rabbit with binds in a row,
Engine-scoped keys now know where to go.
Bad IDs stop at the gate,
Ctrl and Tab bind cleanly in state.
Hop, hop—NUI stays straight!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the fix for the Ctrl/Tab crash caused by missing default binds.
Description check	✅ Passed	The description directly explains the crash, root cause, code changes, and verification steps.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch soloturn-fix-ctrl-crash

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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