Refactor MC_Bedrock to use dynamic path for SoundVolumeView#26
Refactor MC_Bedrock to use dynamic path for SoundVolumeView#26
Conversation
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Summary of ChangesHello @Ven0m0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the maintainability and robustness of scripts by centralizing common utility functions and eliminating hardcoded paths. By introducing dynamic executable location, the system becomes more adaptable to different user environments and more resilient to missing external tools, enhancing the overall user experience without requiring manual path adjustments. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively refactors the codebase by centralizing FindExe and MustGetExe into a shared library, which is a great move for maintainability. The update to MC_Bedrock.ahk to dynamically locate SoundVolumeView.exe instead of using a hardcoded path is a significant improvement, making the script more robust and portable. My review includes a few suggestions to further improve the code by adopting more modern and idiomatic AutoHotkey v2 syntax, which will enhance readability and maintainability.
| Loop Parse, EnvGet("PATH"), ";" | ||
| { | ||
| p := Trim(A_LoopField) | ||
| if !p | ||
| continue | ||
| cand := p . "\" . name | ||
| if FileExist(cand) | ||
| return cand | ||
| } |
There was a problem hiding this comment.
For better readability and to adhere to modern AutoHotkey v2 style, consider using StrSplit() instead of the legacy Loop Parse. StrSplit() is generally preferred in AHK v2 for its clarity.
for _, p_raw in StrSplit(EnvGet("PATH"), ";")
{
p := Trim(p_raw)
if !p
continue
cand := p . "\" . name
if FileExist(cand)
return cand
}
|
|
||
| MustGetExe(name, fallbacks := []) { | ||
| exe := FindExe(name, fallbacks) | ||
| if exe = "" |
There was a problem hiding this comment.
The check if exe = "" can be simplified. In AutoHotkey v2, you can check for an empty string using if !exe. This is more concise and idiomatic.
if !exe
| RunWait(ToolDir . "\SoundVolumeView\SoundVolumeView.exe /SetDefault `"Razer Audio Controller - Game`"") | ||
| } catch Error as err { | ||
| ; SoundVolumeView not found or error - continue anyway | ||
| if (svv != "") { |
There was a problem hiding this comment.
The check if (svv != "") can be simplified. In AutoHotkey v2, you can directly check the truthiness of the variable. Using if svv is more concise and idiomatic for checking if a string is not empty.
if svv {
There was a problem hiding this comment.
Pull request overview
This PR refactors the MC_Bedrock script to use dynamic path resolution for SoundVolumeView instead of hardcoded user-specific paths, improving portability and maintainability across the codebase.
Changes:
- Moved
FindExeandMustGetExeutility functions fromplaynite-all.ahkto the sharedLib/v2/AHK_Common.ahklibrary for reuse across multiple scripts - Updated
MC_Bedrock.ahkto useFindExewith relative fallback paths instead of hardcoded directory references - Removed obsolete warning comments about path customization requirements
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Lib/v2/AHK_Common.ahk | Added FindExe and MustGetExe functions to provide centralized executable location logic with PATH and fallback support |
| ahk/Minecraft/MC_Bedrock.ahk | Replaced hardcoded ToolDir variable with dynamic FindExe call using relative fallback paths; updated to version 2.1.0 |
| Other/playnite-all.ahk | Removed duplicate FindExe and MustGetExe function definitions now that they're in the shared library |
| if exe = "" | ||
| { |
There was a problem hiding this comment.
The opening brace should be on the same line as the if statement to follow OTBS (One True Brace Style) convention used throughout the codebase. For example, see Lib/v2/WindowManager.ahk:11 where if hasTitle { follows OTBS, and line 72 where if saved.Has(id) { also follows OTBS.
1 participant
FindExeandMustGetExefromplaynite-all.ahktoLib/v2/AHK_Common.ahkfor reusability.MC_Bedrock.ahkto useFindExeto locateSoundVolumeView.exeinstead of a hardcoded user path.playnite-all.ahkto remove duplicate function definitions.SoundVolumeViewis not found.PR created automatically by Jules for task 10530591154500514658 started by @Ven0m0