Conversation
|
Had to split the output paths pluginFolderWin: binpluginFolderWin: net8.0-windows Location of plugin files on macOS (relative to the plugin base directory).This parameter is required to support Mac.#pluginFolderMac: bin if not working on windows, windows can allso use bin i think |
There was a problem hiding this comment.
Pull request overview
This PR adds macOS support to the Actionly plugin by implementing a companion app integration via URL scheme, while maintaining existing Windows functionality. The Windows build continues to use an embedded WPF UI for user interaction, while the macOS build delegates all UI and AI processing to an external companion app.
Changes:
- Added multi-targeting support for net8.0-windows and net9.0-macos with conditional compilation
- Implemented macOS companion app launcher via actionly:// URL scheme with fallback notifications
- Reorganized project structure to support platform-specific code compilation
Reviewed changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| LoupedeckPackage.yaml | Updated plugin folder paths to point to framework-specific directories (net8.0-windows, net9.0-macos) |
| ActionlyPlugin.csproj | Converted from single-target to multi-target build with conditional package references and platform-specific file exclusions |
| Directory.Build.props | Reformatted XML comment for better readability |
| PromptCommand.cs | Added platform-specific UI launch logic with conditional compilation directives |
| PromptWithStopCommand.cs | Added platform-specific UI launch logic with conditional compilation directives |
| WindowsUI.cs | Added System.Windows.Forms using statement and removed BOM |
| ScreenshotHelper.cs | Added System.Windows.Forms using statement |
| CommandExecutor.cs | Added System.Threading using statement and removed BOM |
| ApplicationSwitcher.cs | Added System.Collections.Generic using statement |
| AIResponse.cs | Added System using statement and removed BOM |
| GeminiClient.cs | New file implementing Gemini API client for Windows builds |
| AIResponseStore.cs | New file implementing thread-safe response storage for Windows builds |
| Prompt.cs | New file containing AI prompt template as a constant |
| Prompt.txt | New text file with AI prompt template (appears to be duplicate of Prompt.cs content) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { | ||
| using System; | ||
|
|
||
| #if NET8_0_WINDOWS |
There was a problem hiding this comment.
Missing using directive for System.Threading. The code uses 'Thread' on line 62 but the using directive is only conditionally included in the Windows block. This will cause a compilation error on Windows builds. Add 'using System.Threading;' to the Windows-specific using block.
| #if NET8_0_WINDOWS | |
| #if NET8_0_WINDOWS | |
| using System.Threading; |
| <Target Name="PostBuild" AfterTargets="PostBuildEvent"> | ||
| <!-- Write the path to the Debug folder (not the framework subfolder) so plugin can find metadata --> | ||
| <Exec Condition="$(OS) == 'Windows_NT'" Command="echo $(OutputPath)..\ > "$(PluginDir)$(ProjectName).link"" /> | ||
| <Exec Condition="$(OS) != 'Windows_NT'" Command="echo $(OutputPath)../ > $(PluginDir)$(ProjectName).link" /> | ||
| <Message Text="🔨 Created plugin link at: $(PluginDir)$(ProjectName).link" Importance="High" /> | ||
| <Message Text="📂 Plugin folder: $(OutputPath)../" Importance="High" /> | ||
|
|
||
| <Message Text="🔄 Sending plugin reload command for $(PluginShortName) to Logi Plugin Service" Importance="High" /> | ||
| <Exec Condition="$(OS) == 'Windows_NT'" Command="start loupedeck:plugin/$(PluginShortName)/reload" ContinueOnError="true" /> | ||
| <Exec Condition="$(OS) != 'Windows_NT'" Command="open loupedeck:plugin/$(PluginShortName)/reload" ContinueOnError="true" /> | ||
| <Message Condition="'$(MSBuildLastTaskResult)' == 'false'" Text="❌ Couldn't reload the plugin, please restart the Logi Plugin Service manually" Importance="High" /> | ||
| </Target> |
There was a problem hiding this comment.
The PostBuild target writes "$(OutputPath).." to the plugin link file, but with the new multi-targeting setup where OutputPath is "bin\Debug\net8.0-windows" or "bin\Debug\net9.0-macos", this will point to "bin\Debug" which is correct for finding the package metadata. However, this build target runs for both target frameworks, potentially creating a race condition where both Windows and macOS builds try to write to the same .link file. Consider adding a condition to only run this for the active target framework, or ensure the path resolution is consistent across both frameworks.
| <Nullable>disable</Nullable> | ||
| <RootNamespace>Loupedeck.ActionlyPlugin</RootNamespace> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>net8.0-windows;net9.0-macos</TargetFrameworks> |
There was a problem hiding this comment.
Using .NET 9 for macOS (net9.0-macos) while Windows uses .NET 8 (net8.0-windows) creates an asymmetry in the target frameworks. This could lead to behavioral differences and maintenance challenges. Consider using the same .NET version for both platforms (e.g., net8.0-macos and net8.0-windows) unless there's a specific requirement for .NET 9 features on macOS. If .NET 9 is required on macOS, document the reasoning.
| <!-- Windows-only NuGet packages --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net8.0-windows'"> | ||
| <PackageReference Include="Google.GenAI" Version="0.6.0" /> | ||
| <PackageReference Include="System.Drawing.Common" Version="8.0.0" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Reference Include="PluginApi"> | ||
| <HintPath>$(PluginApiDir)PluginApi.dll</HintPath> | ||
| </Reference> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Windows-specific resources --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net8.0-windows'"> | ||
| <Resource Include="Platforms\Windows\Views\actionly-logo.png"> | ||
| <CopyToOutputDirectory>Always</CopyToOutputDirectory> | ||
| </Resource> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <Folder Include="Helpers\Models\" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Windows-only resources --> | ||
| <ItemGroup Condition="'$(TargetFramework)' == 'net8.0-windows'"> | ||
| <None Update="Helpers\Windows\Prompt.txt"> | ||
| <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory> | ||
| </None> | ||
| </ItemGroup> | ||
|
|
||
| <!-- Exclude platform-specific files from wrong builds --> | ||
| <ItemGroup> | ||
| <Compile Remove="Platforms\Windows\**\*" Condition="'$(TargetFramework)' != 'net8.0-windows'" /> | ||
| <Compile Remove="Platforms\MacOS\**\*" Condition="'$(TargetFramework)' != 'net9.0-macos'" /> | ||
| <Compile Remove="Helpers\Windows\**\*" Condition="'$(TargetFramework)' != 'net8.0-windows'" /> | ||
| <Compile Remove="Helpers\MacOS\**\*" Condition="'$(TargetFramework)' != 'net9.0-macos'" /> | ||
| <None Remove="Platforms\Windows\**\*" Condition="'$(TargetFramework)' != 'net8.0-windows'" /> | ||
| <None Remove="Platforms\MacOS\**\*" Condition="'$(TargetFramework)' != 'net9.0-macos'" /> | ||
| <Page Remove="Platforms\Windows\**\*" Condition="'$(TargetFramework)' != 'net8.0-windows'" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
The embedded resources for robot.png and stop.png have been removed from the project file. These images are still referenced in PromptWithStopCommand.cs using PluginResources.FindFile(), which expects these files to be embedded resources. Without the EmbeddedResource entries, the plugin will fail to load these images at runtime. Add the following to an ItemGroup (can be platform-agnostic since both Windows and macOS builds need the images):
| using Loupedeck.ActionlyPlugin.Helpers; | ||
| using Loupedeck.ActionlyPlugin.Helpers.Windows; | ||
| using Loupedeck.ActionlyPlugin.Helpers.Models; | ||
| using Loupedeck.ActionlyPlugin.Helpers.Windows; |
There was a problem hiding this comment.
Missing using directive for System.Threading. The code uses 'Thread' on line 43 but the using directive is only conditionally included in the Windows block. This will cause a compilation error on Windows builds. Add 'using System.Threading;' to the Windows-specific using block.
| using Loupedeck.ActionlyPlugin.Helpers.Windows; | |
| using Loupedeck.ActionlyPlugin.Helpers.Windows; | |
| using System.Threading; |
| try | ||
| { | ||
| var process = Process.Start(new ProcessStartInfo | ||
| { | ||
| FileName = "open", | ||
| Arguments = "actionly://", | ||
| UseShellExecute = false, | ||
| RedirectStandardError = true | ||
| }); | ||
|
|
||
| process?.WaitForExit(3000); | ||
|
|
||
| // If exit code != 0, the URL scheme is not registered (app not installed) | ||
| if (process != null && process.ExitCode != 0) | ||
| { | ||
| PluginLog.Warning("Companion app not installed - showing notification"); | ||
| Process.Start("osascript", "-e 'display notification \"Please install the Actionly companion app to use this plugin.\" with title \"Actionly\" sound name \"Basso\"'"); | ||
| } | ||
| else | ||
| { | ||
| PluginLog.Info("Successfully opened companion app"); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| PluginLog.Error($"Failed to launch companion app: {ex.Message}"); | ||
| try | ||
| { | ||
| Process.Start("osascript", "-e 'display notification \"Please install the Actionly companion app to use this plugin.\" with title \"Actionly\" sound name \"Basso\"'"); | ||
| } | ||
| catch | ||
| { | ||
| // Notification failed, silently continue | ||
| } | ||
| } |
There was a problem hiding this comment.
The macOS companion app launch code is duplicated between PromptCommand.cs (lines 78-112) and PromptWithStopCommand.cs (lines 81-106). This code handles the URL scheme launch, exit code checking, and notification display. Consider extracting this into a shared helper method to improve maintainability and ensure consistent behavior across both commands.
| Process.Start("osascript", "-e 'display notification \"Please install the Actionly companion app to use this plugin.\" with title \"Actionly\" sound name \"Basso\"'"); | ||
| } | ||
| else | ||
| { | ||
| PluginLog.Info("Successfully opened companion app"); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| PluginLog.Error($"Failed to launch companion app: {ex.Message}"); | ||
| } |
There was a problem hiding this comment.
The Process.Start call for showing notifications is not properly awaited or checked. If the osascript command fails (line 96), there's no logging or error handling beyond the try-catch that silently continues. While silent failure may be acceptable for notifications, consider adding at least a debug log message to aid in troubleshooting.
This PR adds triggers the actionly companion app, when on macos.
If not installed, the user gets a system notification, that he needs to install the app
On windows, there should be no change!