fix: resolve hotkey unresponsiveness and GUI unstripping errors in IL2CPP#832
Open
sorrowmoil wants to merge 1 commit intobbepis:masterfrom
Open
fix: resolve hotkey unresponsiveness and GUI unstripping errors in IL2CPP#832sorrowmoil wants to merge 1 commit intobbepis:masterfrom
sorrowmoil wants to merge 1 commit intobbepis:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR targets IL2CPP-specific runtime issues by adding an IL2CPP-safe legacy input implementation and hardening parts of the Translation Aggregator UI against stripped IMGUI APIs that can throw NotSupportedException.
Changes:
- Added an
Il2CppLegacyInputSystem(IL2CPP-only) that accessesUnityEngine.Inputvia reflection. - Added a scroll-view “supported/unsupported” fallback in
TranslationAggregatorWindowto avoid unstripping failures. - Reworked
TranslationAggregatorOptionsWindowlayout to avoidGUILayoutusage (not currently IL2CPP-gated).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/XUnity.Common/Utilities/UnityInput.cs | Adds IL2CPP-only legacy input implementation using reflection and wires it into input system selection. |
| src/XUnity.AutoTranslator.Plugin.Core/UI/TranslationAggregatorWindow.cs | Adds guarded scroll view usage to prevent repeated IL2CPP “unstripping failed” exceptions. |
| src/XUnity.AutoTranslator.Plugin.Core/UI/TranslationAggregatorOptionsWindow.cs | Replaces GUILayout layout with absolute GUI positioning to avoid stripped GUILayout API calls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+920
to
+938
| private System.Reflection.MethodInfo _getKeyString; | ||
| public bool GetKey(string name) | ||
| { | ||
| if (_getKeyString == null) _getKeyString = GetInputType().GetMethod("GetKey", new[] { typeof(string) }); | ||
| return _getKeyString != null && (bool)_getKeyString.Invoke(null, new object[] { name }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKey; | ||
| public bool GetKey(KeyCode key) | ||
| { | ||
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | ||
| return _getKey != null && (bool)_getKey.Invoke(null, new object[] { key }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKeyDownString; | ||
| public bool GetKeyDown(string name) | ||
| { | ||
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | ||
| return _getKeyDownString != null && (bool)_getKeyDownString.Invoke(null, new object[] { name }); |
There was a problem hiding this comment.
Il2CppLegacyInputSystem methods call MethodInfo.Invoke/PropertyInfo.GetValue without handling exceptions. In this codebase, any exception in input handling disables hotkeys entirely via AutoTranslationPlugin.HandleInputSafe, so this IL2CPP path should swallow/unwrap reflection exceptions (e.g., TargetInvocationException / NotSupportedException from stripped methods) and return safe defaults instead of letting them propagate.
Suggested change
| private System.Reflection.MethodInfo _getKeyString; | |
| public bool GetKey(string name) | |
| { | |
| if (_getKeyString == null) _getKeyString = GetInputType().GetMethod("GetKey", new[] { typeof(string) }); | |
| return _getKeyString != null && (bool)_getKeyString.Invoke(null, new object[] { name }); | |
| } | |
| private System.Reflection.MethodInfo _getKey; | |
| public bool GetKey(KeyCode key) | |
| { | |
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | |
| return _getKey != null && (bool)_getKey.Invoke(null, new object[] { key }); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDownString; | |
| public bool GetKeyDown(string name) | |
| { | |
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | |
| return _getKeyDownString != null && (bool)_getKeyDownString.Invoke(null, new object[] { name }); | |
| private static bool ShouldSwallowInputReflectionException(Exception ex) | |
| { | |
| if (ex is System.Reflection.TargetInvocationException tie && tie.InnerException != null) | |
| { | |
| return ShouldSwallowInputReflectionException(tie.InnerException); | |
| } | |
| return ex is NotSupportedException | |
| || ex is InvalidOperationException | |
| || ex is MemberAccessException | |
| || ex is System.Reflection.TargetException | |
| || ex is System.Reflection.TargetParameterCountException | |
| || ex is ArgumentException; | |
| } | |
| private bool InvokeBoolOrDefault(System.Reflection.MethodInfo method, object argument) | |
| { | |
| if (method == null) return false; | |
| try | |
| { | |
| return method.Invoke(null, new[] { argument }) is bool result && result; | |
| } | |
| catch (Exception ex) when (ShouldSwallowInputReflectionException(ex)) | |
| { | |
| var actual = ex is System.Reflection.TargetInvocationException tie && tie.InnerException != null | |
| ? tie.InnerException | |
| : ex; | |
| XuaLogger.AutoTranslator.Debug("[UnityInput] Failed to invoke legacy IL2CPP input method '" + method.Name + "' - " + actual); | |
| return false; | |
| } | |
| } | |
| private System.Reflection.MethodInfo _getKeyString; | |
| public bool GetKey(string name) | |
| { | |
| if (_getKeyString == null) _getKeyString = GetInputType().GetMethod("GetKey", new[] { typeof(string) }); | |
| return InvokeBoolOrDefault(_getKeyString, name); | |
| } | |
| private System.Reflection.MethodInfo _getKey; | |
| public bool GetKey(KeyCode key) | |
| { | |
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | |
| return InvokeBoolOrDefault(_getKey, key); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDownString; | |
| public bool GetKeyDown(string name) | |
| { | |
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | |
| return InvokeBoolOrDefault(_getKeyDownString, name); |
Comment on lines
+924
to
+959
| return _getKeyString != null && (bool)_getKeyString.Invoke(null, new object[] { name }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKey; | ||
| public bool GetKey(KeyCode key) | ||
| { | ||
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | ||
| return _getKey != null && (bool)_getKey.Invoke(null, new object[] { key }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKeyDownString; | ||
| public bool GetKeyDown(string name) | ||
| { | ||
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | ||
| return _getKeyDownString != null && (bool)_getKeyDownString.Invoke(null, new object[] { name }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKeyDown; | ||
| public bool GetKeyDown(KeyCode key) | ||
| { | ||
| if (_getKeyDown == null) _getKeyDown = GetInputType().GetMethod("GetKeyDown", new[] { typeof(KeyCode) }); | ||
| return _getKeyDown != null && (bool)_getKeyDown.Invoke(null, new object[] { key }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKeyUpString; | ||
| public bool GetKeyUp(string name) | ||
| { | ||
| if (_getKeyUpString == null) _getKeyUpString = GetInputType().GetMethod("GetKeyUp", new[] { typeof(string) }); | ||
| return _getKeyUpString != null && (bool)_getKeyUpString.Invoke(null, new object[] { name }); | ||
| } | ||
|
|
||
| private System.Reflection.MethodInfo _getKeyUp; | ||
| public bool GetKeyUp(KeyCode key) | ||
| { | ||
| if (_getKeyUp == null) _getKeyUp = GetInputType().GetMethod("GetKeyUp", new[] { typeof(KeyCode) }); | ||
| return _getKeyUp != null && (bool)_getKeyUp.Invoke(null, new object[] { key }); |
There was a problem hiding this comment.
The IL2CPP reflection-based input calls allocate a new object[] on every key query (e.g., in GetKey/GetKeyDown), and these methods are executed every frame in Update. Consider avoiding per-call allocations (e.g., using cached argument arrays, delegates, or another fast-reflection approach) to prevent unnecessary GC pressure and potential stutters.
Suggested change
| return _getKeyString != null && (bool)_getKeyString.Invoke(null, new object[] { name }); | |
| } | |
| private System.Reflection.MethodInfo _getKey; | |
| public bool GetKey(KeyCode key) | |
| { | |
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | |
| return _getKey != null && (bool)_getKey.Invoke(null, new object[] { key }); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDownString; | |
| public bool GetKeyDown(string name) | |
| { | |
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | |
| return _getKeyDownString != null && (bool)_getKeyDownString.Invoke(null, new object[] { name }); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDown; | |
| public bool GetKeyDown(KeyCode key) | |
| { | |
| if (_getKeyDown == null) _getKeyDown = GetInputType().GetMethod("GetKeyDown", new[] { typeof(KeyCode) }); | |
| return _getKeyDown != null && (bool)_getKeyDown.Invoke(null, new object[] { key }); | |
| } | |
| private System.Reflection.MethodInfo _getKeyUpString; | |
| public bool GetKeyUp(string name) | |
| { | |
| if (_getKeyUpString == null) _getKeyUpString = GetInputType().GetMethod("GetKeyUp", new[] { typeof(string) }); | |
| return _getKeyUpString != null && (bool)_getKeyUpString.Invoke(null, new object[] { name }); | |
| } | |
| private System.Reflection.MethodInfo _getKeyUp; | |
| public bool GetKeyUp(KeyCode key) | |
| { | |
| if (_getKeyUp == null) _getKeyUp = GetInputType().GetMethod("GetKeyUp", new[] { typeof(KeyCode) }); | |
| return _getKeyUp != null && (bool)_getKeyUp.Invoke(null, new object[] { key }); | |
| _stringInputArgs[0] = name; | |
| return _getKeyString != null && (bool)_getKeyString.Invoke(null, _stringInputArgs); | |
| } | |
| private readonly object[] _stringInputArgs = new object[1]; | |
| private readonly object[] _keyCodeInputArgs = new object[1]; | |
| private System.Reflection.MethodInfo _getKey; | |
| public bool GetKey(KeyCode key) | |
| { | |
| if (_getKey == null) _getKey = GetInputType().GetMethod("GetKey", new[] { typeof(KeyCode) }); | |
| _keyCodeInputArgs[0] = key; | |
| return _getKey != null && (bool)_getKey.Invoke(null, _keyCodeInputArgs); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDownString; | |
| public bool GetKeyDown(string name) | |
| { | |
| if (_getKeyDownString == null) _getKeyDownString = GetInputType().GetMethod("GetKeyDown", new[] { typeof(string) }); | |
| _stringInputArgs[0] = name; | |
| return _getKeyDownString != null && (bool)_getKeyDownString.Invoke(null, _stringInputArgs); | |
| } | |
| private System.Reflection.MethodInfo _getKeyDown; | |
| public bool GetKeyDown(KeyCode key) | |
| { | |
| if (_getKeyDown == null) _getKeyDown = GetInputType().GetMethod("GetKeyDown", new[] { typeof(KeyCode) }); | |
| _keyCodeInputArgs[0] = key; | |
| return _getKeyDown != null && (bool)_getKeyDown.Invoke(null, _keyCodeInputArgs); | |
| } | |
| private System.Reflection.MethodInfo _getKeyUpString; | |
| public bool GetKeyUp(string name) | |
| { | |
| if (_getKeyUpString == null) _getKeyUpString = GetInputType().GetMethod("GetKeyUp", new[] { typeof(string) }); | |
| _stringInputArgs[0] = name; | |
| return _getKeyUpString != null && (bool)_getKeyUpString.Invoke(null, _stringInputArgs); | |
| } | |
| private System.Reflection.MethodInfo _getKeyUp; | |
| public bool GetKeyUp(KeyCode key) | |
| { | |
| if (_getKeyUp == null) _getKeyUp = GetInputType().GetMethod("GetKeyUp", new[] { typeof(KeyCode) }); | |
| _keyCodeInputArgs[0] = key; | |
| return _getKeyUp != null && (bool)_getKeyUp.Invoke(null, _keyCodeInputArgs); |
Comment on lines
+223
to
243
| try | ||
| { | ||
| positioned.ScrollPosition = GUILayout.BeginScrollView( positioned.ScrollPosition, ArrayHelper.Null<GUILayoutOption>() ); | ||
| _isScrollViewSupported = true; | ||
|
|
||
| foreach( var text in texts ) | ||
| { | ||
| GUILayout.Label( text, GUIUtil.LabelTranslation, ArrayHelper.Null<GUILayoutOption>() ); | ||
| } | ||
|
|
||
| GUILayout.EndScrollView(); | ||
| } | ||
| catch( Exception ) | ||
| { | ||
| _isScrollViewSupported = false; | ||
| foreach( var text in texts ) | ||
| { | ||
| GUILayout.Label( text, GUIUtil.LabelTranslation, ArrayHelper.Null<GUILayoutOption>() ); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
If GUILayout.BeginScrollView succeeds and a later GUILayout call inside the try throws, the catch path skips GUILayout.EndScrollView, which can leave IMGUI layout groups unbalanced and cause additional GUI errors. Track whether the scroll view started and always call EndScrollView in a finally (and consider catching only NotSupportedException for the unsupported-API fallback).
Comment on lines
69
to
83
| float posy = GUIUtil.WindowTitleClearance + GUIUtil.ComponentSpacing; | ||
|
|
||
| if( GUI.Button( GUIUtil.R( WindowWidth - 22, 2, 20, 16 ), "X" ) ) | ||
| { | ||
| IsShown = false; | ||
| } | ||
|
|
||
| GUILayout.Label( "Available Translators" ); | ||
| GUI.Label( GUIUtil.R( GUIUtil.HalfComponentSpacing, posy, WindowWidth - GUIUtil.ComponentSpacing, GUIUtil.LabelHeight ), "Available Translators" ); | ||
| posy += GUIUtil.LabelHeight + GUIUtil.HalfComponentSpacing; | ||
|
|
||
| // GROUP | ||
| _scrollPosition = GUILayout.BeginScrollView( _scrollPosition, GUI.skin.box ); | ||
| GUI.Box( GUIUtil.R( GUIUtil.HalfComponentSpacing, posy, WindowWidth - GUIUtil.ComponentSpacing, _toggles.Count * GUIUtil.RowHeight + GUIUtil.ComponentSpacing ), "" ); | ||
| posy += GUIUtil.HalfComponentSpacing; | ||
|
|
||
| foreach( var vm in _toggles ) | ||
| { |
There was a problem hiding this comment.
This change removes the scroll view and switches the whole layout from GUILayout to absolute-position GUI for all builds, which alters behavior outside IL2CPP (contradicting the PR note that non-IL2CPP behavior is unchanged). If the goal is an IL2CPP fallback, consider keeping the existing GUILayout+ScrollView path for non-IL2CPP (or when supported) and only falling back when the unstripping exception is observed.
Comment on lines
+98
to
+99
| posy += GUIUtil.HalfComponentSpacing + GUIUtil.ComponentSpacing; | ||
|
|
There was a problem hiding this comment.
After removing the scroll view, the private field _scrollPosition is no longer used anywhere in this class. Consider removing it (or reinstating scrolling behind a supported/unsupported fallback) to avoid dead code and warnings.
sorrowmoil
force-pushed
the
fix/input-hotkey
branch
4 times, most recently
from
1186563 to
f3c6dfe
Compare
sorrowmoil
force-pushed
the
fix/input-hotkey
branch
from
f3c6dfe to
fb3b1b7
Compare
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.
This PR is part of a split from a larger PR, as requested in review.
Fixes issues specific to IL2CPP where hotkeys become unresponsive and the UI throws continuous exceptions when opened.
Changes:
Il2CppLegacyInputSystemclass insideUnityInput.cs(wrapped in#if IL2CPP) to directly invokeUnityEngine.Inputmethods via reflection. As requested, this avoids replacing the contents of the wholeLegacyInputSystemand ensures minimal changes outside of the IL2CPP scope.TranslationAggregatorWindowandTranslationAggregatorOptionsWindow. ReplacedGUILayoutmethods (BeginScrollView,BeginHorizontal, etc.) with basicGUIrect calculations or safe fallbacks to preventSystem.NotSupportedException: Method unstripping failedcrashes in heavily stripped IL2CPP games.Notes: