Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Lua chunknames standardized across platforms and formats #6920

Merged
merged 10 commits into from
Aug 31, 2022

Conversation

britzl
Copy link
Contributor

@britzl britzl commented Aug 29, 2022

Engine

The chunk/filename set at runtime is not consistent between Lua modules and script components. The same is true for bytecode when compared to source code.

Examples:

The file game/foo.lua was either given =game.foo or @game/foo.lua depending on wether it was compiled to bytecode or not. The same kind of differences could be observed for script components.

With this change all Lua scripts and modules will use the same naming convention which is @path/to/file.lua or @path/to/file.script. The @ is used so that Lua will show the last 60 of long filenames.

Editor User-facing changes

  • Fixed an issue where certain file paths printed to the Console upon a Lua error were not clickable.
  • Fixed an issue where the Lua debugger Current Line indicator would not show up inside .lua modules.
  • When multiple project resources match a partial file path in the Console, we now present a choice of resources to open.

Editor Technical changes

  • The on-click! callback for a code.view region is now called with an additional MouseEvent argument.

Fixes #6485

This makes things consistent across script components and Lua modules and between bytecode and source code
@@ -330,7 +330,7 @@ local function stack(start)
if not source then break end

local src = source.source
if src:find("[@=]") == 1 then
if src:find("@") == 1 then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can now safely revert this patch since we no longer provide filenames starting with = (see script_module.cpp)

@@ -633,8 +633,8 @@ local function debug_hook(event, line)
-- Unfortunately, there is no reliable/quick way to figure out
-- what is the filename and what is the source code.
-- If the name doesn't start with `@`, assume it's a file name if it's all on one line.
if find(file, "^[@=]") or not find(file, "[\r\n]") then
file = gsub(gsub(file, "^[@=]", ""), "\\", "/")
if find(file, "^@") or not find(file, "[\r\n]") then
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

int LuaLoad(lua_State *L, dmLuaDDF::LuaSource *source)
{
const char *buf;
uint32_t size;
GetLuaSource(source, &buf, &size);
char tmp[DMPATH_MAX_PATH];
return luaL_loadbuffer(L, buf, size, PrefixFilename(FindSuitableChunkname(source->m_Filename), '=', tmp, sizeof(tmp)));
return luaL_loadbuffer(L, buf, size, source->m_Filename);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always use the filename provided by LuaBuilder

@@ -142,7 +102,7 @@ namespace dmScript
return 1;
}

if (!LuaLoadModule(L, module->m_Script, module->m_ScriptSize, name))
if (!LuaLoadModule(L, module->m_Script, module->m_ScriptSize, module->m_Filename))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do the same for Lua modules. Always use what LuaBuilder.java has provided as filename.

JCash
JCash previously approved these changes Aug 30, 2022
dmStrlCpy(&buf[1], input, size-1);
return buf;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

engine/script/src/script_module.cpp Show resolved Hide resolved
@britzl britzl marked this pull request as ready for review August 30, 2022 11:43
JCash
JCash previously approved these changes Aug 30, 2022
@britzl
Copy link
Contributor Author

britzl commented Aug 30, 2022

@matgis discovered issues with hotreload of Lua modules (scripts work fine). We need to look into this first.

Copy link
Contributor

@matgis matgis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works great! The editor parts have been reviewed by Vlad already.

@britzl britzl merged commit e9f674d into dev Aug 31, 2022
@britzl britzl deleted the dev-mobdebug-filename-chunkname-fix branch August 31, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor > Links are no longer clickable in the console (when an error occurs)
3 participants