-
-
Notifications
You must be signed in to change notification settings - Fork 3
WIP #11
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 adds a debug interface for the saver module with improved file handling and variable scoping. The changes focus on creating a properties panel for debugging save/load operations and fixing file path handling.
- Adds a new debug properties panel with save/load slot management and inspection capabilities
- Fixes file mode and variable scoping issues in the saver internal module
- Simplifies file path handling by flattening directory structures
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| saver/saver_internal.lua | Updates file modes from "w+" to "wb" and fixes variable scoping for LUA_REQUIRE_AS_STRING |
| saver/saver_debug_page.lua | New debug interface providing save/load slot management and game state inspection |
| saver/saver.lua | Integrates debug panel, adds autosave timer getter, and simplifies file path handling |
| saver/properties_panel/property_saver_slots.lua | Widget implementation for three-button slot interface |
| saver/properties_panel/property_saver_slots.gui | GUI definition for the slot selection widget |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| end | ||
|
|
||
| return sys.get_save_file(directory_path, filename) | ||
| filename = filename:gsub("/", "_") |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change breaks subfolder support by converting directory separators to underscores. Files that were previously saved in subfolders (e.g., "profiles/profile1.json") will now be flattened to a single directory with underscore-separated names (e.g., "profiles_profile1.json"). This creates a breaking change where existing save files in subfolders become inaccessible.
| filename = filename:gsub("/", "_") | |
| -- filename = filename:gsub("/", "_") |
| saver.load_game_state("saver_slot_1") | ||
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_2.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_2") | ||
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_3.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_3") | ||
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sys.reboot calls with hardcoded configuration parameters could potentially be exploited if user input influences the slot names. Consider validating that slot names contain only safe characters before passing them to sys.reboot.
| saver.load_game_state("saver_slot_1") | |
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_2") | |
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_3") | |
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") | |
| local slot_name = "saver_slot_1" | |
| saver.load_game_state(slot_name) | |
| if validate_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| local slot_name = "saver_slot_2" | |
| saver.load_game_state(slot_name) | |
| if validate_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| local slot_name = "saver_slot_3" | |
| saver.load_game_state(slot_name) | |
| if validate_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end |
| saver.load_game_state("saver_slot_1") | ||
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_2.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_2") | ||
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_3.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_3") | ||
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sys.reboot calls with hardcoded configuration parameters could potentially be exploited if user input influences the slot names. Consider validating that slot names contain only safe characters before passing them to sys.reboot.
| saver.load_game_state("saver_slot_1") | |
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_2") | |
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_3") | |
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") | |
| local slot_name = "saver_slot_1" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| local slot_name = "saver_slot_2" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| local slot_name = "saver_slot_3" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end |
| saver.load_game_state("saver_slot_1") | ||
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_2.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_2") | ||
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | ||
| end) | ||
| three_buttons.button_3.on_click:subscribe(function() | ||
| saver.load_game_state("saver_slot_3") | ||
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sys.reboot calls with hardcoded configuration parameters could potentially be exploited if user input influences the slot names. Consider validating that slot names contain only safe characters before passing them to sys.reboot.
| saver.load_game_state("saver_slot_1") | |
| sys.reboot("--config=saver.save_name=saver_slot_1", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_2") | |
| sys.reboot("--config=saver.save_name=saver_slot_2", "--config=saver.autosave_timer=0") | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| saver.load_game_state("saver_slot_3") | |
| sys.reboot("--config=saver.save_name=saver_slot_3", "--config=saver.autosave_timer=0") | |
| local slot_name = "saver_slot_1" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_2.on_click:subscribe(function() | |
| local slot_name = "saver_slot_2" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end | |
| end) | |
| three_buttons.button_3.on_click:subscribe(function() | |
| local slot_name = "saver_slot_3" | |
| saver.load_game_state(slot_name) | |
| if is_valid_slot_name(slot_name) then | |
| sys.reboot("--config=saver.save_name=" .. slot_name, "--config=saver.autosave_timer=0") | |
| else | |
| print("Invalid slot name: " .. tostring(slot_name)) | |
| end |
| if system == "Windows" then | ||
| os.execute(string.format('explorer /select,"%s"', path)) |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The os.execute calls with user-provided paths could be vulnerable to command injection if the path contains malicious characters. Consider sanitizing the path parameter or using a safer alternative to os.execute for opening file browsers.
| if system == "Windows" then | |
| os.execute(string.format('explorer /select,"%s"', path)) | |
| -- Helper to safely quote Windows paths | |
| local function windows_quote(str) | |
| -- Escape double quotes and wrap in quotes | |
| return '"' .. tostring(str):gsub('"', '\\"') .. '"' | |
| end | |
| if system == "Windows" then | |
| os.execute(string.format('explorer /select,%s', windows_quote(path))) |
| os.execute(string.format('explorer /select,"%s"', path)) | ||
| return true | ||
| elseif system == "Linux" then | ||
| os.execute(string.format("xdg-open %q", path)) |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The os.execute calls with user-provided paths could be vulnerable to command injection if the path contains malicious characters. Consider sanitizing the path parameter or using a safer alternative to os.execute for opening file browsers.
| os.execute(string.format("xdg-open %q", path)) | ||
| return true | ||
| elseif system == "Darwin" then | ||
| os.execute(string.format("open -R %q", path)) |
Copilot
AI
Aug 26, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The os.execute calls with user-provided paths could be vulnerable to command injection if the path contains malicious characters. Consider sanitizing the path parameter or using a safer alternative to os.execute for opening file browsers.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
- Coverage 78.43% 76.57% -1.87%
==========================================
Files 4 4
Lines 524 525 +1
==========================================
- Hits 411 402 -9
- Misses 113 123 +10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
No description provided.