Skip to content

Consolidate all 22 open PRs: DI/testability, perf, SetTimer, path fixes, regex escaping #109

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.

Sign up for GitHub

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

Merged
Ven0m0 merged 4 commits into from
Jun 3, 2026
Merged

Consolidate all 22 open PRs: DI/testability, perf, SetTimer, path fixes, regex escaping #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7914133
Add new test files and apply SetTimer loop refactor (PRs 79, 84, 92, …
Ven0m0 Jun 3, 2026
d38532e
Apply WindowManager DI fixes, AHK_Common FindExe improvements, Fullsc…
Ven0m0 Jun 3, 2026
51e9bcf
Apply remaining changes: test improvements, perf optimizations, code …
Ven0m0 Jun 3, 2026
df4e4fe
perf: optimize TF_Merge - remove redundant IfExist, split Output conc…
Ven0m0 Jun 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Lib/v2/AHK_Common.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,16 @@ InitScript(requireUIA := true, requireAdmin := false, optimize := true) {
}

FindExe(name, fallbacks := []) {
if (name == "")
return ""
if FileExist(name)
return name
Loop Parse, EnvGet("PATH"), ";"
{
p := Trim(A_LoopField)
p := Trim(A_LoopField, " `t`"")
if !p
continue
p := RTrim(p, "\/")
cand := p . "\" . name
if FileExist(cand)
return cand
Expand Down
24 changes: 12 additions & 12 deletions Lib/v2/WindowManager.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,26 +33,25 @@ MakeFullscreen(winTitle) {
MaximizeWindow(winTitle)
}

WaitForWindow(winTitle, timeout := 30) {
try {
WinWait(winTitle, , timeout)
return true
} catch TimeoutError {
return false
}
WaitForWindow(winTitle, timeout := 30, api := "") {
if !api
api := SystemWindowAPI()

return api.WinWait(winTitle, , timeout) != 0
}
Comment on lines +36 to 41

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

In AutoHotkey v2, the native WinWait function throws a TimeoutError when it times out. By removing the try-catch block, WaitForWindow will propagate this unhandled exception to the caller instead of returning false as expected. We should restore the try-catch block to handle the timeout gracefully.

WaitForWindow(winTitle, timeout := 30, api := "") {
    if !api
        api := SystemWindowAPI()

    try {
        api.WinWait(winTitle, , timeout)
        return true
    } catch TimeoutError {
        return false
    }
}


WaitForProcess(processName, timeout := 30) {
return ProcessWait(processName, timeout) != 0
}

GetMonitorAtPos(x, y, api := "") {
if !api
api := SystemWindowAPI()

count := api.MonitorGetCount()
count := IsObject(api) ? api.MonitorGetCount() : MonitorGetCount()
Loop count {
api.MonitorGet(A_Index, &l, &t, &r, &b)
if IsObject(api)
api.MonitorGet(A_Index, &l, &t, &r, &b)
else
MonitorGet(A_Index, &l, &t, &r, &b)

if (l <= x && x <= r && t <= y && y <= b)
return A_Index
}
Expand Down Expand Up @@ -93,6 +92,7 @@ RestoreWindowBorders(winTitle) {
}

class SystemWindowAPI {
WinWait(winTitle, winText?, timeout?) => WinWait(winTitle, winText?, timeout?)
WinGetStyle(winTitle) => WinGetStyle(winTitle)
WinSetStyle(style, winTitle) => WinSetStyle(style, winTitle)
WinMove(x, y, w, h, winTitle) => WinMove(x, y, w, h, winTitle)
Expand Down
10 changes: 0 additions & 10 deletions Other/7zEmuPrepper/Final.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
; 7zEmuPrepper command builder (for 7zEmuPrepper.ps1)
; Fields -> emits a ready-to-run PowerShell command. Includes a Copy button.

sevenZEmuPrepperPath := ""
sevenZipPath := ""
emulatorPath := ""
arguments := ""
Expand Down Expand Up @@ -46,15 +45,6 @@ gui.Add("Button", "x340 y430 w80 h23", "Exit").OnEvent("Click", (*) => ExitApp()
gui.Show("w480 h470")
return

SelectFileOrDir(btn) {
global
text := btn.Text
if (text = "…") {
; infer based on label order
idx := btn.Hwnd
}
}

; Simplified per-control selection based on Y position
SelectFileOrDir(btn, *) {
y := btn.Pos.Y
Expand Down
33 changes: 20 additions & 13 deletions Other/Citra_mods/Citra_3DS_Manager.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ LoadDestinations(){
DestMap := {}
if !FileExist(DestCsv)
return
Loop, Read, %DestCsv%
FileRead, csvData, %DestCsv%
Loop, Parse, csvData, `n, `r
{
line := Trim(A_LoopReadLine)
line := Trim(A_LoopField)
if (line = "" || !InStr(line, ","))
continue
StringSplit, p, line, `,
Expand All @@ -56,29 +57,35 @@ ScanMods(){
;----------------- Config Helpers -----------------
UpdateConfig(content, updates){
remaining := updates.Clone()
remCount := remaining.Count()

; Pre-allocate buffer for performance.
; content size + estimated size for updates + null terminator
VarSetCapacity(newContent, (StrLen(content) + updates.Count() * 100 + 1) * (A_IsUnicode ? 2 : 1))
newContent := ""
VarSetCapacity(newContent, (StrLen(content) + remCount * 100 + 1) * (A_IsUnicode ? 2 : 1))

Loop, Parse, content, `n, `r
{
line := A_LoopField
pos := InStr(line, "=")
if (pos > 1){
keyCandidate := RTrim(SubStr(line, 1, pos-1))
if (remaining.HasKey(keyCandidate)){
val := remaining[keyCandidate]
line := keyCandidate "=" val
remaining.Delete(keyCandidate)
if (remCount > 0){
pos := InStr(line, "=")
if (pos > 1){
keyCandidate := RTrim(SubStr(line, 1, pos-1))
if (remaining.HasKey(keyCandidate)){
line := keyCandidate "=" remaining[keyCandidate]
remaining.Delete(keyCandidate)
remCount--
}
}
}
newContent .= line "`n"
newContent .= line
newContent .= "`n"
}

for k, v in remaining {
newContent .= k "=" v "`n"
newContent .= k
newContent .= "="
newContent .= v
newContent .= "`n"
}

return SubStr(newContent, 1, -1)
Expand Down
14 changes: 5 additions & 9 deletions Other/Citra_mods/Citra_Mod_Manager.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ root := OneDrive "\Backup\Game\Emul\Citra\nightly-mingw\Mods"

; Read CSV once and cache destinations in an associative array
Destinations := {}
loop Read, % A_ScriptDir "\Destination.csv"
FileRead, csvContent, % A_ScriptDir "\Destination.csv"
loop Parse, csvContent, `n, `r
{
if (InStr(A_LoopReadLine, ","))
if (InStr(A_LoopField, ","))
{
parts := StrSplit(A_LoopReadLine, ",")
parts := StrSplit(A_LoopField, ",")
if (parts.Length() >= 2)
Destinations[parts[1]] := parts[2]
}
Expand Down Expand Up @@ -61,12 +62,7 @@ FileActions(Root, Button)
Fullpath := Zielpfad "\" button.Ziel "\" button.Name
Checkdir := Zielpfad "\" button.Ziel
Quellpfad := button.Path
dirHasItems := false
Loop, Files, %Checkdir%\*, DF ;Loop through items in dir
{
dirHasItems := true ;Mark that we found at least one item
Break ;Optimization: Break after finding the first item
}
dirHasItems := FileExist(Checkdir "\*") != ""

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

This is an AutoHotkey v1 script. In AutoHotkey v1, FileExist does not support wildcards (unlike v2). Calling FileExist(Checkdir "\*") will always return an empty string, meaning dirHasItems will always be false. This breaks the empty directory check and will cause the script to always attempt to copy files, potentially overwriting existing data. We should revert to the original Loop, Files approach.

        dirHasItems := false
        Loop, Files, %Checkdir%\*, DF
        {
            dirHasItems := true
            break
        }

If !dirHasItems ;Is directory empty?
{
FileCopyDir, %Quellpfad%, %Fullpath%
Expand Down
9 changes: 4 additions & 5 deletions Other/Citra_per_game_config/tf.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,12 +889,11 @@ TF_Merge(FileList, Separator = "`n", FileName = "merged.txt")
OW=0
Loop, Parse, FileList, `n, `r
{
Append2File= ; Just make sure it is empty
IfExist, %A_LoopField%
FileRead, Append2File, %A_LoopField%
If not ErrorLevel ; Successfully loaded
{
FileRead, Append2File, %A_LoopField%
If not ErrorLevel ; Successfully loaded
Output .= Append2File Separator
Output .= Append2File
Output .= Separator
}
}

Expand Down
6 changes: 3 additions & 3 deletions Other/Citra_per_game_config/v2/CitraConfigHelpers.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
; Escaped string safe for use in regex patterns
; ============================================================================
RegExEscape(str) {
return RegExReplace(str, "([\\()\[\]{}?*+|^$.])", "\$1")
return RegExReplace(str, "([\\()\[\]{}?*+|^$.})", "\\$1")
}
Comment on lines 20 to 22

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

The closing square bracket ] of the character class was accidentally replaced with } in the regex pattern, leaving the character class unclosed. This will cause RegExReplace to throw a runtime ValueError (invalid regular expression) on any call to RegExEscape. The original character class already matched both { and } correctly.

RegExEscape(str) {
    return RegExReplace(str, '([\\()\[\]{}?*+|^$.])', '\\$1')
}


; ============================================================================
Expand All @@ -36,9 +36,9 @@ RegExEscape(str) {
; Modified configuration content
; ============================================================================
SetKey(content, key, value) {
pat := "m)^(" . RegExEscape(key) . ")\s*=.*$"
pat := "m)^(" . RegExEscape(key) . ")\\s*=.*$"
if RegExMatch(content, pat)
return RegExReplace(content, pat, "$1=" value, , 1)
return RegExReplace(content, pat, "$1=" StrReplace(value, "$", "$$"), , 1)
else
return content "`n" key "=" value
}
Comment on lines 38 to 44

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

In AutoHotkey RegExReplace, backslashes (\) in the replacement string have special meaning (such as backreferences like \1). If value contains backslashes (which is extremely common for file paths in configuration files), they will be interpreted by the regex engine, leading to corrupted values or unexpected behavior. Both backslashes and dollar signs must be escaped in the replacement string.

SetKey(content, key, value) {
    pat := "m)^(" . RegExEscape(key) . ")\\s*=.*$"
    if RegExMatch(content, pat) {
        escapedValue := StrReplace(StrReplace(value, '\'', '\\\\''), '$', '$$')
        return RegExReplace(content, pat, '$1=' . escapedValue, , 1)
    } else {
        return content "`n" key "=" value
    }
}

Expand Down
17 changes: 16 additions & 1 deletion Other/Citra_per_game_config/v2/CitraConfigHelpers_Test.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ TestSetKey() {
content2 := "path\to\file=exists`n"
result := SetKey(content2, "path\to\file", "updated")
AssertEqual(InStr(result, "path\to\file=updated") > 0, true, "SetKey updates key with backslashes")

; Test 6: Empty value
result := SetKey(content, "key1", "")
AssertEqual(InStr(result, "key1=") > 0 && InStr(result, "key1=value1") == 0, true, "SetKey handles empty value")

; Test 7: Value with literal $ sign
result := SetKey(content, "key1", "val$ue")
AssertEqual(InStr(result, "key1=val$ue") > 0, true, "SetKey handles value with literal $ sign")

; Test 8: Value with literal $1
result := SetKey(content, "key2", "val$1ue")
AssertEqual(InStr(result, "key2=val$1ue") > 0, true, "SetKey handles value with literal $1")

; Test 9: Add new key with $ sign
result := SetKey(content, "key4", "new$val")
AssertEqual(InStr(result, "key4=new$val") > 0, true, "SetKey adds new key with $ sign")
}

TestReplaceInFile() {
Expand Down Expand Up @@ -116,7 +132,6 @@ TestReplaceInFile() {
AssertEqual(content, "goodbye world`ngoodbye citra", "ReplaceInFile leaves content unchanged when text not found")

; Test 5: ReplaceInFile handles try/catch error (SaveConfig fails or LoadConfig fails)
; In this case, we can pass a directory to trigger an error
testDir := A_ScriptDir . "\test_replace_dir"
if !DirExist(testDir)
DirCreate(testDir)
Expand Down
38 changes: 23 additions & 15 deletions Other/Lossless_Scaling_Manager.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ EnsureAdmin() {
try Run('*RunAs "' A_AhkPath '" "' A_ScriptFullPath '" ' A_Args.Join(" "))
ExitApp
}
EnsureAdmin()
if (A_LineFile == A_ScriptFullPath) {
EnsureAdmin()
}

MinimizeLS() {
try {
Expand All @@ -35,23 +37,29 @@ StopLS() {
try ProcessClose("LosslessScaling.exe")
}

ToggleLS() {
if ProcessExist("LosslessScaling.exe")
StopLS()
ToggleLS(mockProcessExist := "", mockStopLS := "", mockStartLS := "") {
fnProcessExist := (mockProcessExist != "") ? mockProcessExist : ProcessExist
fnStopLS := (mockStopLS != "") ? mockStopLS : StopLS
fnStartLS := (mockStartLS != "") ? mockStartLS : StartLS

if fnProcessExist("LosslessScaling.exe")
fnStopLS()
else
StartLS()
fnStartLS()
}

; ---------- Dispatch ----------
cmd := (A_Args.Length ? StrLower(A_Args[1]) : "toggle")
try {
switch cmd {
case "start": StartLS()
case "stop", "close": StopLS()
case "toggle": ToggleLS()
default:
MsgBox("Usage: " A_ScriptName " [start|stop|toggle]")
if (A_LineFile == A_ScriptFullPath) {
cmd := (A_Args.Length ? StrLower(A_Args[1]) : "toggle")
try {
switch cmd {
case "start": StartLS()
case "stop", "close": StopLS()
case "toggle": ToggleLS()
default:
MsgBox("Usage: " A_ScriptName " [start|stop|toggle]")
}
} catch Error as err {
MsgBox("Error: " err.Message)
}
} catch Error as err {
MsgBox("Error: " err.Message)
}
3 changes: 1 addition & 2 deletions Other/playnite-all.ahk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ PlayBootVideo() {
vlcPath := MustGetExe("vlc.exe", ["C:\Program Files\VideoLAN\VLC\vlc.exe"])
bootVideo := A_ScriptDir . "\BootVideo.mp4"
vlcArgs := '--fullscreen --video-on-top --play-and-exit --no-video-title -Idummy "' . bootVideo . '"'
RunWait('cmd.exe /c START "" "' . vlcPath . '" ' . vlcArgs, , "hide")
DllCall("kernel32.dll\Sleep", "UInt", 3000)
RunWait('"' . vlcPath . '" ' . vlcArgs, , "hide")

@gemini-code-assist gemini-code-assist Bot Jun 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using RunWait directly on VLC will block the script execution and wait for the entire boot video to finish playing before continuing to launch Playnite. If the intention is to start VLC asynchronously (as it was previously with cmd.exe /c START), you should use Run instead of RunWait.

    Run('"' . vlcPath . '" ' . vlcArgs, , "hide")

}
LaunchPlaynite() {
playniteExe := MustGetExe(
Expand Down
Loading
Loading