Debug scripts with luapanda#251
Conversation
|
I'm marking as draft. Please provide a proper description what it is and whats it for. Also you will have conflicts due to recent ImGUI PR which i accepted, you need to resolve the issues |
|
It's support VS Code Plugin: LuaPanda |
…e (not working), lua files
SaloEater
left a comment
SaloEater
left a comment
There was a problem hiding this comment.
@themrdemonized Hi, I think first iteration is finished and PR is finished and working as release binary. Also, please, look through comments below.
I will try take care of broken AttachDebugger functions on next iteration, just need some time to restore nerves and get some sleep. (I just hope someone will fix it for me)
src/xrEngine/imgui_tools.cpp
Outdated
| { | ||
| if (*luaDebug) | ||
| { | ||
| //AttachDebugger(); //Getting crash in ljtab.c after executing this function: Unhandled exception thrown: read access violation. |
There was a problem hiding this comment.
Wasn't able to make this work, it goes deep into namespaces and lua source files and it's not clear at all what to look for
| <ClCompile Include="..\imgui_helper.cpp"> | ||
| <Filter>Interfaces\ImGui</Filter> | ||
| </ClCompile> | ||
| <ClCompile Include="..\imgui_base.cpp" /> |
There was a problem hiding this comment.
No idea if this is important or no. It's being changed by itself
There was a problem hiding this comment.
.vcxproj / .vcxproj.filters changes should be fine - VS automatically updates those when you modify the solution structure from its UI.
src/xrGame/vs2022/xrGame.vcxproj
Outdated
| <ClCompile> | ||
| <AdditionalIncludeDirectories>vs2022;..;$(SolutionDir);$(SolutionDir)xrServerEntities;$(SolutionDir)3rd party\luajit-2\src\;$(SolutionDir)3rd party\luabind\;$(xrSdkDir)include;$(XRAY_16X_LIBS)OpenAutomate\inc;$(SolutionDir)3rd party\CxImage;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories> | ||
| <PreprocessorDefinitions>WIN32;NDEBUG;_WINDOWS;_USRDLL;XRGAME_EXPORTS;dSINGLE;MSVC;_SECURE_SCL=0;%(PreprocessorDefinitions)</PreprocessorDefinitions> | ||
| <PreprocessorDefinitions>WIN32;NDEBUG;_WINDOWS;_USRDLL;XRGAME_EXPORTS;dSINGLE;MSVC;_SECURE_SCL=0;USE_LUAJIT_ONE;%(PreprocessorDefinitions)</PreprocessorDefinitions> |
There was a problem hiding this comment.
So are there any potential issues with using LuaJIT? While it's the same lua file, there are some #ifdef related to USE_LUAJIT_ONE and I don't really understand how impactful they are
For that time that I had been developing never? had issues with something related to lua
There was a problem hiding this comment.
LuaJIT is the default. I think you actually have to do significant work to make Anomaly build using regular interpreted Lua...
See this comment for more details.
| { | ||
| string_path FileName; | ||
| xr_string FixedFileName = name; | ||
| //FixedFileName = "kernel\\" + FixedFileName; //When this line is enabled all lua files are not being loaded after loading a game |
There was a problem hiding this comment.
When I put lua files into scripts/kernel directory they become unavailable in lua scripts after new/loading a game.
But when I put them just into scripts - those files are loaded into game and I can use them.
I.E. with this line I can run in console run_string debugger_attach() where debugger_attach is a function from global.lua but after I create a new game same command give me function not found error
There was a problem hiding this comment.
Scripts don't load from nested folders on the main branch. #249 lays the groundwork to make that happen, but restricts it to core-code .lua scripts for now to keep the .script ecosystem untouched.
Those changes would probably make this integration a lot simpler, since they start sandboxing Lua away from the engine, and allow its built-in package methods to be used more freely.
| bool bufferLoaded = false; | ||
| if (unlocalPerformed) { | ||
| bufferLoaded = load_buffer(lua(), scriptContents, scriptLength, l_caLuaFileName, caNameSpaceName); | ||
| bufferLoaded = load_buffer(lua(), scriptContents, scriptLength, caNameSpaceName, caNameSpaceName); |
There was a problem hiding this comment.
Before this fix scripts were identified by debugger as script.script instead of it's real name like ui_main_menu.script
There was a problem hiding this comment.
This will have knock-on effects, since load_buffer treats namespace name (short script name) and file name (full file path) as separate quantities.
Best-case, some engine-side logging will become less verbose. Worst-case, it'll go all the way down into Lua's script name metadata, potentially causing crashes in scripts like Weird Task Framework that use the debug API to query their file path at runtime.
It also conflicts with #249, since CScriptStorage has been moved into Lua along with most of its CScriptEngine subclass.
There was a problem hiding this comment.
It also conflicts with #249, since
CScriptStoragehas been moved into Lua along with most of itsCScriptEnginesubclass.
If somehow this PR will be finished, I would also try to do the same after yours will be merged too. Just really crave to make some mods with a debugger and wanted to do that asap.
There was a problem hiding this comment.
Old: "../bin/gamedata/scripts/file.script"
New: "file.script"
Where problem?
There was a problem hiding this comment.
Example from Weird Task Framework:
local function get_game_path()
local info = debug.getinfo(1,'S');
local script_path = info.source:match[[^@?(.*[\/])[^\/]-$]]
local game_path = script_path:match("(.*)gamedata"):gsub("/", "\\")
return game_path
enddebug.getinfo is expected to return @c:\full\path\to\anomaly\bin\../gamedata/scripts/igi_taskdata.script, which is derived from the file name passed to Lua's loadstring as an optional second parameter.
If this change waterfalls down to Lua's internals (which it may well do, as load_buffer invokes Lua's source-loading methods) the call to debug.getinfo will return igi_taskdata.script instead, causing the subsequent match to fail.
In practice, this means WTF will crash the game before it reaches the title screen.
There was a problem hiding this comment.
Example from Weird Task Framework:
Fair enough, still figuring that out
There was a problem hiding this comment.
@ProfLander dealt with this issue, now it's fine. Finishing another round of WTF tests and it's good to go
SaloEater
changed the title
|
Why is I note that you also set As for the define itself, as best I can tell it's a mechanism to switch between the current To my knowledge, that old version hasn't been used in a build for quite some time, so it's worth asking whether we still need the associated code - cleaning up unused blocks would make the codebase easier to read and maintain. However, that sort of change that belongs in its own branch / PR, since it's broader in scope than the implementation of luapanda support; I'm not certain off top of my head, but I think there may be other instances of it elsewhere in the code that would also need to be cleaned up. |
Damn, haha, I just realised that |
|
Other assorted thoughts:
|
Honestly, I just took everything they made in IXRay and put here. I have neither skills neither experience to answer all these questions. I would love to just use luadkm that you suggested in your PR but unable to make it work and had to make this frankenstein |
| //open_kb(L); | ||
| //open_log(L); | ||
|
|
||
| if (IsDebug) |
There was a problem hiding this comment.
Don't sure if there any potential impact on the performance if you turn on these things so locked them behind a flag
Honestly, it gives too much comfort to modders with proper variables scope. And most of these files are just independent ones that can be deleted any moment and nothing will break except debugger itself. |
| -- 把路径中去除后缀部分的.变为/, | ||
| -- @filePath 被替换的路径 | ||
| -- @ext 后缀(后缀前的 . 不会被替换) | ||
| function this.changePotToSep(filePath, ext) |
There was a problem hiding this comment.
@ForserX , the issue was hidden here: the function incorrectly converted test.script into test/script and ultimately transformed it into test/script.script and later cached as script.script.
As you can see from the function's documentation, lmao, for the ext argument, it explicitly states that the dot before extensions shouldn't be converted to a slash. However, they never implemented this logic in the code. I had to add this functionality myself, and now test.script is correctly identified as test.script.
To be more detailed:
getPathmethod -xray-monolith/gamedata/scripts/LuaPanda.lua
Line 1722 in 16b26bc
- we enabled dotted filenames -
xray-monolith/gamedata/scripts/LuaPanda.lua
Line 1746 in 16b26bc
- all our scripts have
.scriptin their filename -xray-monolith/gamedata/scripts/LuaPanda.lua
Line 1750 in 16b26bc
- we use
scriptas a correct extension for the debugger -
|
@ProfLander why I like luapanda more than luadkm is just because luadkm reduces fps by ~5 times, atleast for my current PC.
Do you really think this is important now or it can be done in a next iteration? It definitely would take me a god damn long time to figure it out while modmakers can enjoy miracles of debugging
Debug tools is a part of engine (in this PR) + we would need |
Ideally, nobody should want to delete them :) I can relate to feature enthusiasm (the now-closed #240 stands as testament to my own,) but it's important to consider the means before the end.
Which is fair criticism, in addition to the not-great state of variable inspection when using LuaDkm, but it also levies no maintenance overhead on the engine and avoids introducing a favoured editor for Lua scripts beyond the one we're already stuck with for engine code. I'd be more open to the impact of engine-hosted debugging if it integrated with the Debug Adapter Protocol standard, since that would solve the problem for all compliant editors instead of favouring MS' proprietary offering, thus allowing modders a lower bar to entry by virtue of choice. There is some possibility that luapanda already uses DAP internally since the standard originates from VSC, but being documented in chinese makes it difficult to tell without digging into the code and unpicking it by hand...
The tricky thing with next iterations is that they don't always occur (be it due to burnout, life getting in the way, any number of reasons,) so it's risky to not take big changes at face value.
Slippery slope; if everyone hard-coded their engine mod's UI into the C++ menu, it would end up like the console command code - a boilerplate / global state lava flow that keeps getting bigger, but is too scary for anyone to clean up. Hosting such in Lua and having it get the info it needs from a formal engine interface amortizes the problem by containing it in a simple low-maintenance environment, and also exposes said data to scripts for modder use. (MCM, the debug menu, and to a lesser extent That said, I don't know if the existing ImGui C++ menu is actually set up to be extended from scripts - that'll be a question for @xr-lucy |
|
@ProfLander I appreciate you making a dialog with me! That feels so good, like I'm some cool guy too, lol xd I removed everything else (imgui, level changer) except luapanda debugger as I'm completely fine to run debugger from console, bet others would prefer this to having no debugger at all. And again, I'm more than just willing to make this working again after your PR get merged too! |
| //open_kb(L); | ||
| //open_log(L); | ||
|
|
||
| if (IsDebug) |
There was a problem hiding this comment.
@themrdemonized beside this line PR is ready again.
Should I leave this as is or remove a flag and load those 3 libraries by default?
|
@ProfLander i have set up a script callback to allow the lua side to insert stuff into c++ menus 😊 |
|
Not merged into main yet I've moved lua debug stuff behind The reason: Safe to say that a lot of people actually play with |
|
@themrdemonized Please don't merge this to main yet. The changes to Since that file no longer exists post-refactor, bringing it in now implicitly puts the resulting reintegration on my plate, which doesn't seem fair given that the new scripting backend is almost ready to land following a month of work. It would be more productive to have @SaloEater adapt their changes to the new clean
Collaboration is the name of the game :) I think you're taking it well tbh; I've been on the other side of the open-source code review process before now, and know it sucks to have some smartass barge in and start nitpicking your work.
Awesome, thanks! That'll make gradually subsuming the old debug menu much easier :) |
Damn, we chose perfect time to work on lua together, lmao. Let me dive into your branch and see how to patch it there. |
You'll probably want to start at It's also worth looking at the |
|
I made a PR into #249 here based on @themrdemonized changes - Lander-Modding#1 So I assume this solves the issue with potential conflict. Unfortunately, I am just a script guy and my c++ skills are so bad that I definitely won't be able to do these things you suggested for |
The PR against my repo contains out-of-scope changes to the Moreover, the re-additions to the new script engine - having been done naively - ignore its updated structure, violating the principle that Lua should have full control over its own boot and setup process. If I merge that into my fork, it will both undo unrelated work from mainline, and take the currently-stable-in-testing RC7 release from working-as-designed to needs-refactoring. Hence my suggestion that it be reintegrated with guidance after the new backend is in place, since you lack the knowledge to do that as is, and I'm already spinning a lot of plates.
You're making PRs against an actively-developed C++ codebase; you are by definition no longer just a script guy. (And are also the de-facto owner of this C++ code, as per git!) As for library-source distribution, there's enough existing precedent in |
|
Alright, no more complaints - @SaloEater adapted the changes to the new script engine, so we now have a first consumer of the upcoming Thanks for the hard work 👍 |
Is there anything I can do to add an additional .exe file compiled with luapanda libraries into modexes releases? This way modders would atleast know there is a library that allows to use a debugger. |
|
I'll add Luapanda in the next update, I've made a clean integration commit |
I don't think you understand what you're asking for 😩 Anomaly's That's not justifiable for anything that doesn't absolutely need its own build-time specialization, like the existing static renderers + CPU instruction stuff. Even Optick is kinda awkward as-is, since it's coupled to a DX11 + Release profile when it should be able to run on any of them, but that's a necessary compromise until #264 and its successors land to avoid littering release code with slow profiler instrumentation. Safe to say, feature visibility isn't a good reason to do this. As far as I know the above integration makes it available on all build presets, which is ideal for accessibility. If you want to make people aware, you'll want a combination of good README doco, Discord word-of-mouth, and perhaps a dedicated empty-zip ModDB post to explain it to the broader userbase; 3D Ballistics was literally just a |
окак No pressure at all, didn't know it's THAT complicated, lol |
|
No it's not that complicated, for Lua stuff I think it's acceptable to just add debugger directly into sources, by itself it doesn't weigh and that much, and behind a flag it isn't engaged in any way. With optick profiler the required functions to profile have to be marked in engine and with whole backend running every frame, it can affect performance negatively, so that's why separate configuration was made |
Debug scripts with luapanda
5 participants
gamedata/scriptsfolder in VSCode.vscodefolder from the archive into yourgamedata/scriptsfolder: https://github.com/saloeater/xray-monolith/tree/luapanda/gamedata/scripts/.vscodeLuaPanda.lua,dynamic_callbacks.lua,global.lua,socket.luainto yourgamedata/scriptsfolder: https://github.com/saloeater/xray-monolith/tree/luapanda/gamedata/scriptsRun and Debugsection and start debugging or press F5 key~key and typerun_string debugger_attach(). If you do everything correctly and engine is working properly too, you will get an entry breakpoint atglobal.luafile in VSCode.run_string debugger_attach()in console again.Always latest DX11 Binary - https://drive.google.com/file/d/1k62Xps43dvkdOXHY5AVELQjKqm4bugcR/view?usp=sharing
Huge thanks to @ForserX for initially writing that code and helping to inject it here