Skip to content

refactor: 重构嵌入数据#52

Merged
actboy168 merged 19 commits into
masterfrom
lua_embed
Apr 17, 2026
Merged

refactor: 重构嵌入数据#52
actboy168 merged 19 commits into
masterfrom
lua_embed

Conversation

@actboy168

Copy link
Copy Markdown
Owner

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the lua_embed module to implement a unified, group-based data model. The previous fixed preload and data attributes are replaced by a flexible data table where each key represents a named group, which is then generated as a field in a lua_embed_bundle struct. The generation process now dynamically creates lua_embed_data.h to match the user's configuration. Corresponding updates were made to the bee_glue.c runtime, the Ninja build logic in writer.lua, and the project documentation. Review feedback identified a risk of compilation failure or runtime crashes in bee_glue.c if standard groups like data or preload are omitted, as well as a potential path-handling bug in the generator script when no directory separator is present in the output path.

Comment thread scripts/lua_embed/bee_glue.c
Comment thread scripts/lua_embed/lua_embed_gen.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

此 PR 将 lm:lua_embed 从“preload/data/main + 函数查询”的嵌入模型重构为“按命名 group 组织 + 生成全局结构体访问”的模型,并同步更新 bee.lua 胶水层与文档说明。

Changes:

  • 将嵌入数据改为按 data.<group> 组织,生成 lua_embed_data.h + 全局 lua_embed 结构体供 C/C++ 直接访问
  • 更新 bee 胶水层:约定使用 lua_embed.preload/main/data 三个 group 完成 _PRELOAD 注入、入口脚本与 bee.embed
  • 更新/迁移文档与最佳实践示例,删除旧的 lua_embed.h

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
skills/references/core/advanced_api.md 更新 lm:lua_embed 的 group 规则、bee_glue 约定与新的 C 侧访问方式文档
skills/references/best_practices/bp_dependency.md 将示例迁移到新的 data.<group> 结构
skills/references/best_practices/bp_codegen.md 更新 lm:lua_embed 的推荐用法与分组/bytecode 说明
skills/SKILL.md 更新 lm:lua_embed 简述与示例
scripts/writer.lua 调整 Ninja 产物为 .c + lua_embed_data.h,并更新导出 objdeps 的聚合目标
scripts/lua_embed/lua_embed_gen.lua 重写生成器:按 groups 生成条目数组、lua_embed 全局结构体与 lua_embed_data.h
scripts/lua_embed/lua_embed.h 删除旧 runtime 头文件(函数式查询 API)
scripts/lua_embed/bee_glue.c 适配新数据结构:从 lua_embed.<group> 直接遍历/取 main,并实现 bee.embed
scripts/lua_embed.lua 重写 config 生成与输入收集逻辑,按 data 的 string key 生成有序 groups
.gitignore 忽略 /.claude/
Comments suppressed due to low confidence (1)

scripts/writer.lua:916

  • use_bee 目前用 local_attribute.bee_glue ~= nil 判断会导致 bee_glue = false 也启用 bee_glue.c(与文档的 bool 语义不一致)。建议改为仅在 bee_glue == true 时启用,并在启用时校验 data.preload/data.main/data.data 组存在(否则会因生成的 lua_embed_bundle 缺少字段而编译失败)。
        local use_bee = local_attribute.bee_glue ~= nil


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/SKILL.md Outdated
Comment thread skills/references/core/advanced_api.md Outdated
Comment thread skills/references/best_practices/bp_codegen.md Outdated
Comment thread skills/references/best_practices/bp_codegen.md Outdated
@actboy168

Copy link
Copy Markdown
Owner Author

/gemini review

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

scripts/writer.lua:916

  • bee_glue is now documented/used as a boolean, but lua_embed treats any non-nil value as “enabled” (bee_glue ~= nil). That means an explicit bee_glue = false (or legacy bee_glue = "main.lua") will still enable the glue path and trigger required-group assertions. Consider validating bee_glue's type/value (e.g., require true), and emitting a clearer migration error for the old string form.
        local use_bee = local_attribute.bee_glue ~= nil


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/references/best_practices/bp_dependency.md
Comment thread skills/references/best_practices/bp_codegen.md Outdated
Comment thread scripts/lua_embed/lua_embed_gen.lua

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the lua_embed system to use a flexible, group-based architecture. The previous fixed structure of preload, data, and main has been replaced by a unified lua_embed_bundle struct that supports custom groups defined in the build configuration. Key changes include updates to the generation script to produce a dedicated header file, modifications to the bee_glue layer to interface with the new data layout, and comprehensive documentation updates. Review feedback suggests optimizing the linear search used for data lookups in large sets and improving the robustness of C identifier generation to prevent potential filename collisions.

Comment thread scripts/lua_embed/bee_glue.c
Comment thread scripts/lua_embed/lua_embed_gen.lua Outdated
@actboy168
actboy168 requested a review from Copilot April 17, 2026 13:53
@actboy168

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the lua_embed system to use a flexible, group-based configuration for embedding resources, replacing the previous fixed preload/data structure. It introduces a generated lua_embed_data.h header and a lua_embed_bundle struct for accessing embedded data, while removing the static lua_embed.h. The code generator has been improved with better C identifier collision handling and directory scanning logic that mirrors Lua's package.path priority. Feedback highlights a critical regression in bee_glue.c where the preloading environment is no longer initialized in the main entry point, and suggests a more robust approach for generating unique C identifiers in the generator script.

Comment thread scripts/lua_embed/bee_glue.c
Comment thread scripts/lua_embed/lua_embed_gen.lua Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

scripts/lua_embed/bee_glue.c:115

  • _bee_main() no longer calls _bee_preload_module(). This means running the main script will not register lua_embed.preload into package.preload nor register bee.embed unless callers remember to call _bee_preload_module() separately, which contradicts the documented “bee_glue auto inject” behavior and is a behavioral regression from the prior implementation.
LUA_EMBED_EXPORT int _bee_main(lua_State* L) {
    const lua_embed_entry* m = lua_embed.main;
    if (m == NULL || m->name == NULL) {
        lua_pushstring(L, "lua_embed: no main entry configured");
        return lua_error(L);
    }
    if (luaL_loadbuffer(L, m->data, m->size, m->name) != LUA_OK)
        return lua_error(L);
    if (lua_pcall(L, 0, 0, 0) != LUA_OK)
        return lua_error(L);
    return 0;

scripts/writer.lua:916

  • bee_glue is documented/used as a boolean flag (bee_glue = true), but the implementation enables it whenever the field is merely non-nil. This makes bee_glue = false still link bee_glue.c and enforce bee_glue group constraints, which is surprising and hard to diagnose. Consider treating it as enabled only when bee_glue == true (or explicitly supporting legacy string values and rejecting false).

        local use_bee = local_attribute.bee_glue ~= nil


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/lua_embed.lua Outdated
Comment thread scripts/lua_embed.lua
Comment thread scripts/lua_embed/lua_embed_gen.lua
@actboy168

Copy link
Copy Markdown
Owner Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the lm:lua_embed system to support multiple named data groups, unified file scanning, and automatic generation of both C source and header files. Key changes include a more robust C identifier generation strategy with collision handling in lua_embed_gen.lua, and a simplified integration workflow in writer.lua that removes manual header copying. Feedback identifies a critical regression in bee_glue.c where the automatic module registration call was omitted in _bee_main, and points out the use of local absolute paths in the newly added documentation.

Comment thread scripts/lua_embed/bee_glue.c
Comment thread skills/references/advanced/lua_embed.md Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

scripts/lua_embed/lua_embed_gen.lua:30

  • readfile() uses f:read("a"), which is not a valid Lua file read format in standard Lua I/O (should be "*a" to read the entire file). As written, the generator will fail at runtime when trying to read any embedded input file.
local function readfile(path)
    local f, err = io.open(path, "rb")
    if not f then error("cannot open: " .. path .. " (" .. tostring(err) .. ")") end
    local data = f:read("a")
    f:close()
    return data

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread skills/references/advanced/lua_embed.md
Comment thread skills/references/advanced/lua_embed.md Outdated
Comment thread skills/references/advanced/lua_embed.md
Comment thread scripts/lua_embed/lua_embed_gen.lua
Comment thread scripts/lua_embed/bee_glue.c
@actboy168
actboy168 merged commit ffbaa99 into master Apr 17, 2026
9 of 10 checks passed
@actboy168
actboy168 deleted the lua_embed branch April 17, 2026 14:40
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.

2 participants