fix: resolve module-relative UI paths when loaded from callbacks - #1787
fix: resolve module-relative UI paths when loaded from callbacks#1787mimus-assa wants to merge 1 commit into
Conversation
Any g_ui.loadUI/displayUI/importStyle("barename") call made from a callback
(UI button, protocol packet, scheduled event) resolves to the vfs root and
fails: ResourceManager::resolvePath() relies on g_lua.getCurrentSourcePath(),
which does not yield the module dir in those contexts — and the
isPreDrawing() branch skips script-relative resolution entirely (also the
!scriptPath.empty() guard is dead code: "/" + path is never empty). The same
call works from a module init(), which is why the breakage only shows on
windows opened on demand.
Confirmed broken paths: imbuement shrine window (both designs), game_shop and
game_stash (error on every login), character list on relog, outfit window,
console channels dialog, ignore window, VIP add/edit, extra battle list
instances, store/reward wall subwindows.
Fix (Lua only): wrap the three entry points in corelib and resolve relative
paths against the calling file directory (debug.getinfo), only when the
target .otui actually exists there; every other case is forwarded untouched,
so behavior is otherwise identical. Idempotent; tail calls safely degrade to
the current behavior.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesUI path resolution
Estimated code review effort: 3 (Moderate) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant LuaCaller
participant g_ui
participant resolveCallerPath
participant g_resources
participant OriginalUIImplementation
LuaCaller->>g_ui: loadUI, displayUI, or importStyle(path, ...)
g_ui->>resolveCallerPath: resolveCallerPath(path)
resolveCallerPath->>g_resources: fileExists(caller directory + path)
g_resources-->>resolveCallerPath: existence result
resolveCallerPath-->>g_ui: resolved or original path
g_ui->>OriginalUIImplementation: invoke(path, ...)
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
Verified in-game on macOS against our production 15.25 server (client patched with this exact change):
Linux and Windows ship the same patched Lua; verification pending on those OSes. |
|
Is it still necessary? The main branch has been fixed. |
|
Thanks for the fix. The callback-relative path issue appears to be real, and the new resolver is correctly loaded through corelib's However, wrapping For example, when a module calls: g_ui.importStyle("foo")the wrapper may pass /modules/example/foo to UIManager::importStyle(). That function builds the device-specific filename as: importStyle(deviceName + "." + fileName, false);which produces something like windows./modules/example/foo instead of /modules/example/windows.foo. As a result, OS/device-specific styles can no longer be resolved whenever the base style is rewritten to an absolute path. Please either preserve the original relative argument for importStyle() or update the C++ device-style lookup to preserve the resolved directory. A regression test covering checkDeviceStyles = true with a caller-relative style and a device-specific variant would also be useful. |
The bug
Any
g_ui.loadUI/g_ui.displayUI/g_ui.importStyle('barename')call made from a callback (UI button, protocol packet handler, scheduled event) resolves against the vfs root and fails withunable to open file '/barename.otui': not found. The same call works when made from a moduleinit(), which is why the breakage only shows on windows that open on demand.Confirmed broken on current
maingame_imbuing/new_design/t_imbui.lua:35, also old design) — the shrine does not open at allgame_shopcreate() — Lua error on every logingame_stashimportStyle on game start — stash styles never loadclient_entergame/characterlist.lua— error when returning to the character listgame_outfit/outfit.lua:338)onChannelList) and ignore windowRoot cause
ResourceManager::resolvePath()resolves relative paths viag_lua.getCurrentSourcePath(), but:g_drawPool.isPreDrawing()branch skips script-relative resolution entirely and maps the path to the root; and!scriptPath.empty()guard is dead code —"/" + getCurrentSourcePath()is never empty, so when the source path is unavailable the result is//barename→/barename(root) instead of falling back.The fix (Lua only)
modules/corelib/ui/uipathresolver.luawraps the three entry points and resolves a relative path against the calling file's directory (debug.getinfo), only when the target.otuiactually exists there. In every other case (absolute path, non-string, unresolvable caller, target not found) the argument is forwarded untouched, so behavior is otherwise byte-identical. Idempotent (guarded against double-wrap); tail-called sites safely degrade to the current behavior.Alternative considered: fixing
resolvePath()in C++ — but theisPreDrawing()branch is load-bearing for draw-time image lookups, and the Lua shim fixes all UI load sites without touching that path.Testing
.otui-suffixed names, extra args forwarded, non-string args, callers without file source, return values preserved, double-load idempotence, C frames and tail calls do not error.🤖 Generated with Claude Code
Summary by CodeRabbit