Skip to content

Conversation

@Alvsch
Copy link
Contributor

@Alvsch Alvsch commented Jun 7, 2025

Description

Moved log, server and events lua tables into structs implementing mlua::UserData

@vyPal
Copy link
Member

vyPal commented Jun 11, 2025

Looks pretty good imo, thanks for the work! (I'm also kinda interested in how copilot reviews work, so Imma just request a review from copilot rq, first time trying it)

@vyPal vyPal requested a review from Copilot June 11, 2025 08:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the Lua API integration by moving the log, server, and events tables into their own structs implementing mlua::UserData for better separation of concerns.

  • Introduces separate Lua userdata structs for log, server, and events
  • Updates the runtime to reference these new structs instead of inlined function definitions
  • Adjusts module exports accordingly in the lua module

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/lua/structs/server.rs Implements the Server userdata with an async function
src/lua/structs/log.rs Implements the Log userdata with logging functions
src/lua/structs/events.rs Implements the Events userdata with listener management
src/lua/runtime.rs Updates pumpkin global to use new structs and events setup
src/lua/structs/mod.rs Exposes new structs through the module
src/lua/mod.rs Registers the structs module

})?,
)?;

events_table.set("", structs::Events)?;
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

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

The events table is being set using an empty key, which is likely unintended. Consider using a meaningful key (e.g., "events") or verifying this is the intended behavior.

Suggested change
events_table.set("", structs::Events)?;
events_table.set("default", structs::Events)?;

Copilot uses AI. Check for mistakes.
@vyPal
Copy link
Member

vyPal commented Jun 11, 2025

Well, interesting result, but I'm gonna merge it anyway. Thanks again for the work!

@vyPal vyPal merged commit f8ddc6f into PumpkinPlugins:master Jun 11, 2025
3 checks passed
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