Wizzard to remap missing device for creation#158
Draft
vicocz wants to merge 10 commits into
Draft
Conversation
vicocz
changed the title
There was a problem hiding this comment.
Pull request overview
This PR adds a UI entry point to remap devices used by an imported creation (intended to help when devices are missing) and refactors “Play” command creation into a shared CreationCommandFactory.
Changes:
- Introduces
ICreationCommandFactoryand moves Play/remap command logic intoCreationCommandFactory. - Adds a “Remap device” toolbar command to the Creation page.
- Adds persistence support for remapping device IDs across controller actions and improves device-id collection (
GetDeviceIdsreturning a set).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| BrickController2/BrickController2/UI/ViewModels/CreationPageViewModel.cs | Switches to ICreationCommandFactory, adds RemapDeviceCommand, removes inline Play logic. |
| BrickController2/BrickController2/UI/ViewModels/CreationListPageViewModel.cs | Switches Play command creation to the command factory. |
| BrickController2/BrickController2/UI/ViewModels/ControllerProfilePageViewModel.cs | Switches Play command creation to the command factory. |
| BrickController2/BrickController2/UI/Pages/CreationPage.xaml | Adds a toolbar item to trigger device remapping. |
| BrickController2/BrickController2/UI/DI/UiModule.cs | Registers CreationCommandFactory as ICreationCommandFactory. |
| BrickController2/BrickController2/UI/Commands/ICreationCommandFactory.cs | New interface extending ICommandFactory<Creation> with Play/remap commands. |
| BrickController2/BrickController2/UI/Commands/CreationCommandFactory.cs | Adds Play/remap command implementations (including remap wizard flow). |
| BrickController2/BrickController2/CreationManagement/ICreationManager.cs | Adds remap API (RemapDevice). |
| BrickController2/BrickController2/CreationManagement/CreationManager.cs | Implements device remap persistence across controller actions. |
| BrickController2/BrickController2/CreationManagement/Creation.cs | Changes GetDeviceIds to return a set and simplifies de-duplication. |
| BrickController2/BrickController2/CreationManagement/ControllerAction.cs | Adds per-action device-id remap helper. |
| BrickController2/BrickController2/BusinessLogic/PlayLogic.cs | Adjusts to GetDeviceIds set semantics; adds GetMissingDevices. |
| BrickController2/BrickController2/BusinessLogic/IPlayLogic.cs | Adds GetMissingDevices to the interface. |
Comments suppressed due to low confidence (2)
BrickController2/BrickController2/UI/ViewModels/CreationPageViewModel.cs:64
HasMultipleControllerProfileswon’t update when controller profiles are added/removed anymore: the previousRaisePropertyChanged(nameof(HasMultipleControllerProfiles))calls were removed and there’s noControllerProfiles.CollectionChangedsubscription. This means the Play swipe item visibility can be stale until the page is recreated. Consider raising the property change after add/delete (and import/paste), or subscribe toCreation.ControllerProfiles.CollectionChangedand raise from there (unsubscribe appropriately).
public Creation Creation { get; }
public bool HasMultipleControllerProfiles => Creation.ControllerProfiles.Count > 1;
BrickController2/BrickController2/UI/ViewModels/CreationListPageViewModel.cs:52
IPlayLogic playLogicis injected and stored in_playLogic, but_playLogicis no longer used in this view model afterPlayAsyncwas removed. Consider removing the unused field/constructor parameter to prevent dead dependencies and warnings.
ICreationManager creationManager,
IDeviceManager deviceManager,
IPlayLogic playLogic,
IDialogService dialogService,
ISharedFileStorageService sharedFileStorageService,
ICreationCommandFactory commandFactory,
IBluetoothPermission bluetoothPermission,
IReadWriteExternalStoragePermission readWriteExternalStoragePermission)
: base(navigationService, translationService)
{
_creationManager = creationManager;
_deviceManager = deviceManager;
_playLogic = playLogic;
_dialogService = dialogService;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
21
to
+34
| private readonly ICreationManager _creationManager; | ||
| private readonly IDialogService _dialogService; | ||
| private readonly IPlayLogic _playLogic; | ||
| private readonly ISharingManager<ControllerProfile> _sharingManagerProfile; | ||
|
|
||
| public CreationPageViewModel( | ||
| INavigationService navigationService, | ||
| ITranslationService translationService, | ||
| ICreationManager creationManager, | ||
| IDialogService dialogService, | ||
| ISharedFileStorageService sharedFileStorageService, | ||
| IPlayLogic playLogic, | ||
| ISharingManager<ControllerProfile> sharingManagerProfile, | ||
| ICommandFactory<Creation> commandFactory, | ||
| ICreationCommandFactory commandFactory, |
Comment on lines
41
to
45
| IPlayLogic playLogic, | ||
| IInputDeviceEventService gameControllerService, | ||
| ICreationCommandFactory commandFactory, | ||
| NavigationParameters parameters) | ||
| : base(navigationService, translationService) |
Comment on lines
+47
to
+48
| public ICommand PlayControllerProfileCommand(PageViewModelBase viewModel) | ||
| => new SafeCommand<ControllerProfile>((profile) => PlayAsync(viewModel, profile.Creation, profile)); |
Comment on lines
+120
to
+137
| // get source device IDs | ||
| var sourceDeviceIds = creation.GetDeviceIds() | ||
| .Select(id => | ||
| { | ||
| DeviceId.TryParse(id, out var deviceType, out var deviceAddress); | ||
| return (DeviceType: deviceType, Address: deviceAddress); | ||
| }) | ||
| .Where(x => x.DeviceType != DeviceType.Unknown && x.Address != null) | ||
| .ToList(); | ||
|
|
||
| // get source types | ||
| var sourceTypes = sourceDeviceIds.Select(x => x.DeviceType).ToHashSet(); | ||
| var addresses = sourceDeviceIds.Select(x => x.Address!).ToHashSet(); | ||
| // source types that have some existing device of such type present, but not used in creation | ||
| var suitableTypes = sourceTypes.Where(x => _deviceManager.Devices | ||
| .Any(d => d.DeviceType == x && !addresses.Contains(d.Address))) | ||
| .ToHashSet(); | ||
|
|
Comment on lines
+169
to
+171
| // choose target device by name but avoid the source one | ||
| var suitableDevices = _deviceManager.Devices | ||
| .Where(d => d.DeviceType == sourceType.Value && d.Address != sourceDeviceAddress) |
| <controls:ToolbarIcon Icon="file_upload" Text="{extensions:Translate ExportCreation}" Command="{Binding ExportCreationCommand}" Order="Secondary" /> | ||
| <controls:ToolbarIcon Icon="copy" Text="{extensions:Translate CopyCreation}" Command="{Binding CopyCreationCommand}" Order="Secondary" /> | ||
| <controls:ToolbarIcon Icon="paste" Text="{extensions:Translate PasteControllerProfile}" Command="{Binding PasteControllerProfileCommand}" Order="Secondary" /> | ||
| <controls:ToolbarIcon Icon="find_replace" Text="{extensions:Translate RemapDevice}" Command="{Binding RemapDeviceCommand}" Order="Secondary" /> |
| Task<Creation> AddCreationAsync(string creationName); | ||
| Task DeleteCreationAsync(Creation creation); | ||
| Task RenameCreationAsync(Creation creation, string newName); | ||
| Task<int> RemapDevice(Creation creation, string sourceDeviceId, string newDeviceId); |
| } | ||
| } | ||
|
|
||
| public async Task<int> RemapDevice(Creation creation, string sourceDeviceId, string newDeviceId) |
Comment on lines
+14
to
+15
| IEnumerable<string> GetMissingDevices(Creation creation); | ||
|
|
|
|
||
| await DialogService.ShowMessageBoxAsync( | ||
| Translate("Information"), | ||
| Translate($"Remapped {count} controller actions from device '{sourceDeviceId}' to '{newDeviceId}'"), |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TBD
Resolves #95