Fixed bugs, added Multi ability line ups#58
Merged
Merged
Conversation
Add LineUp.deepCopy and use deep copies when importing, snapshotting, restoring, and copying lineUps to avoid shared mutable state. Update StrategyPage.copyWith and StrategyProvider to deep-copy lineUps when constructing strategies. Introduce a _hasLoadedStrategy helper and flushPendingAutosaveBeforeExit in StrategyProvider to attempt a silent autosave (respecting autosave settings) before exit; wire this into unsaved_strategy_guard so it will try to save silently and report failures before showing the dialog. Add comprehensive tests (test/unsaved_strategy_guard_test.dart) to cover autosave-enabled/disabled, no-loaded-strategy, and error cases.
- Introduce `LineUpGroup` and `LineUpItem` to support adding multiple items per lineup - Migrate legacy lineup storage and update Hive adapters/registrar - Update ability widgets, painters, and tests for grouped lineup state
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Owner
Author
|
@greptile go ahead and review this bad boy |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors lineup storage and UI to support multiple lineup items per agent (“multi ability lineups”), adds a locked add-item placement mode, and improves exit autosave behavior with new test coverage.
Changes:
- Introduced
LineUpGroup/LineUpItemand migrated lineup persistence/rendering fromLineUpto grouped lineups across providers, widgets, Hive, and JSON. - Added locked “add item to existing group” placement flow (including sidebar dim/disable behavior, context menu actions, and preview agent handling).
- Added autosave flush-on-exit behavior (and comprehensive widget/unit tests for unsaved-exit and lineup interactions).
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unsaved_strategy_guard_test.dart | New tests for autosave flush-before-exit and unsaved-exit dialog behavior. |
| test/role_icon_agent_size_test.dart | Updates test to new framed shell widget type/name. |
| test/lineup_add_item_interaction_test.dart | New widget tests for add-item lineup flow, resizing behavior, and ability bar clearing. |
| test/line_up_provider_switch_sides_test.dart | Adds tests for locked add-item mode; updates harness imports for toast/UI. |
| test/agent_dragable_lineup_state_test.dart | New tests validating agent sidebar dim/disable behavior during lineup placement. |
| test/ability_visibility_widgets_test.dart | Adds tests for lineup context menu behavior on square widgets; adjusts ability info indexing. |
| lib/widgets/sidebar_widgets/agent_dragable.dart | Dims/disables non-active agents in locked add-item mode and reflects selection styling. |
| lib/widgets/save_and_load_button.dart | Passes lineUpGroups into screenshot/export rendering. |
| lib/widgets/mouse_watch.dart | Adds lineUpItemId and updates lineup media/delete behavior to group+item model. |
| lib/widgets/lineup_control_buttons.dart | Enables “agent selected” state when adding an item to an existing group. |
| lib/widgets/line_up_widget.dart | Splits lineup widgets into group-agent and item-ability widgets; adds deprecated wrappers. |
| lib/widgets/line_up_placer.dart | Implements locked add-item placement with preview agent and ability-only drops. |
| lib/widgets/line_up_media_carousel.dart | Updates edit/delete flows to operate on lineup items (transactional delete). |
| lib/widgets/line_up_line_painter.dart | Draws lines from group agent to each item ability (multi-line per group). |
| lib/widgets/draggable_widgets/utilities/role_icon_utility_widget.dart | Switches to FramedAbilityIconShell. |
| lib/widgets/draggable_widgets/shared/framed_ability_icon_shell.dart | Renames shell widget and keeps hover highlighting behavior. |
| lib/widgets/draggable_widgets/placed_widget_builder.dart | Routes lineup placement to startNewGroup / setCurrentAbility; renders groups/items. |
| lib/widgets/draggable_widgets/agents/placed_lineup_agent_widget.dart | New widget for placing/previewing lineup agents with resize awareness. |
| lib/widgets/draggable_widgets/agents/agent_widget.dart | Adds lineup group context menu actions (add item / delete group / create lineup). |
| lib/widgets/draggable_widgets/ability/simple_image_ability_widget.dart | Renames and extends simple image ability widget to carry lineUpItemId. |
| lib/widgets/draggable_widgets/ability/sector_circle_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/rotatable_image_widget.dart | Threads lineUpItemId into MouseWatch. |
| lib/widgets/draggable_widgets/ability/resizable_square_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/deadlock_barrier_mesh_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/custom_square_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/custom_circle_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/center_square_widget.dart | Threads lineUpItemId into nested AbilityWidget. |
| lib/widgets/draggable_widgets/ability/ability_widget.dart | Switches to FramedAbilityIconShell and adds lineUpItemId plumbing. |
| lib/widgets/draggable_widgets/ability/ability_visibility_context_menu.dart | Updates lineup context menu actions to target group+item (transactional delete + visual state update). |
| lib/widgets/dialogs/create_lineup_dialog.dart | Updates create/edit dialog to support editing a lineup item and adding items to a group. |
| lib/widgets/delete_helpers.dart | Updates delete routing from legacy lineup id to group id deletion. |
| lib/widgets/current_line_up_painter.dart | Highlights current lineup agent using either currentAgent or selected group agent. |
| lib/services/unsaved_strategy_guard.dart | Adds autosave flush-before-exit fast path with error reporting and tests. |
| lib/screenshot/screenshot_view.dart | Accepts lineUpGroups (with deprecated lineUps fallback) and loads groups into provider. |
| lib/providers/strategy_provider.dart | Adds flush-before-exit API; integrates lineup group migration and updates load/save paths to lineUpGroups. |
| lib/providers/strategy_page.dart | Stores lineUpGroups (with deprecated lineUps fallback) and updates JSON serialization keys. |
| lib/providers/interaction_state_provider.dart | Clears lineup placement and ability bar when leaving lineup placing mode. |
| lib/providers/canvas_resize_provider.dart | New provider to notify placed widgets to recompute positions on canvas resize. |
| lib/migrations/lineup_group_migration.dart | Migration to normalize lineUpID fields for groups/items. |
| lib/migrations/ability_scale_migration.dart | Updates migrations to operate on lineup groups/items instead of legacy single lineup. |
| lib/interactive_map.dart | Triggers canvasResizeProvider when viewport/play area size changes to reflow placed lineup widgets. |
| lib/hive/hive_registrar.g.dart | Registers new Hive adapters for LineUpGroup and LineUpItem. |
| lib/hive/hive_adapters.g.yaml | Bumps type IDs and updates StrategyPage field indexing for lineUpGroups. |
| lib/hive/hive_adapters.g.dart | Adds Hive adapters for LineUpGroup and LineUpItem; updates StrategyPageAdapter to write lineUpGroups. |
| lib/hive/hive_adapters.dart | Adds adapter specs for LineUpGroup and LineUpItem. |
| lib/const/line_provider.g.dart | Adds JSON codecs for LineUpGroup / LineUpItem. |
| lib/const/line_provider.dart | Introduces LineUpGroup/LineUpItem, locked add-item placement state, and group/item CRUD APIs. |
| lib/const/abilities.dart | Threads lineUpItemId through ability widget factories and renames image ability widget class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
146
to
159
| final effectiveOnTap = widget.onTap ?? | ||
| (widget.lineUpId == null | ||
| ? null | ||
| : () { | ||
| showDialog( | ||
| context: context, | ||
| builder: (context) => LineUpMediaCarousel( | ||
| lineUpId: widget.lineUpId!, | ||
| images: lineUp!.images, | ||
| youtubeLink: lineUp.youtubeLink, | ||
| lineUpGroupId: widget.lineUpId!, | ||
| lineUpItemId: widget.lineUpItemId!, | ||
| images: lineUpItem!.images, | ||
| youtubeLink: lineUpItem.youtubeLink, | ||
| ), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
MouseWatch builds a default onTap handler whenever lineUpId is non-null, but it unconditionally force-unwraps lineUpItemId and lineUpItem (widget.lineUpItemId!, lineUpItem!.images, etc.). This will throw at runtime for lineup group agent widgets (they pass lineUpId but no lineUpItemId). Gate the default onTap on lineUpItemId != null (and lineUpItem != null), or provide a safe fallback (e.g., open the first item / disable tap for group-only widgets).
Comment on lines
93
to
+104
| Widget build(BuildContext context) { | ||
| final LineUp? lineUp = widget.lineUpId == null | ||
| ? null | ||
| : ref.watch( | ||
| lineUpProvider.select((state) { | ||
| for (final lineUp in state.lineUps) { | ||
| if (lineUp.id == widget.lineUpId) { | ||
| return lineUp; | ||
| } | ||
| } | ||
| return null; | ||
| }), | ||
| ); | ||
| final lineUpNotes = lineUp?.notes; | ||
| ref.watch(lineUpProvider); | ||
| final LineUpItem? lineUpItem = | ||
| widget.lineUpId == null || widget.lineUpItemId == null | ||
| ? null | ||
| : ref | ||
| .read(lineUpProvider.notifier) | ||
| .getItemById( | ||
| groupId: widget.lineUpId!, | ||
| itemId: widget.lineUpItemId!, | ||
| ); | ||
| final lineUpNotes = lineUpItem?.notes; |
There was a problem hiding this comment.
ref.watch(lineUpProvider); is currently used only for its side-effect (triggering rebuilds) and its return value is ignored. With flutter_lints, this pattern can trigger unused_result / unused_local_variable style warnings, and it also rebuilds on any lineup state change. Prefer reading the watched value into a local and deriving the needed LineUpItem from it (or use select to watch only the specific item/notes you need).
Owner
Author
|
@cursoragent review :D |
|
Absolutely agreed — and I took exactly that approach.
I also proved recovery is real, not hallucinated:
All these tests passed.   |
Contributor
Greptile SummaryThis PR refactors the lineup system from a flat Key changes:
Confidence Score: 4/5Safe to merge after fixing the missing ability bar update in the 'Create Lineup' context-menu path; all other changes are well-structured with solid backward-compat coverage. One confirmed P1 defect: the 'Create Lineup' shortcut from a strategy agent leaves the ability bar empty/stale, making the new feature non-functional for that entry point. The Hive schema, migration, and backward-compat shims are sound. lib/widgets/draggable_widgets/agents/agent_widget.dart — missing abilityBarProvider.updateData call in the 'Create Lineup' onPressed handler. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent right-click context menu] --> B{lineUpId set?}
B -- Yes --> C["Add Lineup Item\nabilityBarProvider.updateData ✓\ninteractionState → lineUpPlacing\nstartNewItemForGroup"]
B -- No --> D{plainAgent present?}
D -- Yes --> E["Create Lineup\ninteractionState → lineUpPlacing\nstartNewGroup\n⚠️ abilityBarProvider NOT updated"]
D -- No --> F[No lineup menu item]
C --> G[Locked add-item mode\ncurrentGroupId set\nlockedAgentType set]
E --> H[New group mode\ncurrentAgent set\nabilityBar empty/stale]
G --> I[Drop ability on map]
H --> I
I --> J[setCurrentAbility\nvalidates agent type match]
J --> K[CreateLineupDialog\nconfirm → addGroup or addItemToGroup]
Reviews (2): Last reviewed commit: "Preserve legacy lineup field for Hive co..." | Re-trigger Greptile |
SunkenInTime
force-pushed
the
small-patch
branch
from
502fc7e to
088f062
Compare
- keep StrategyPage.lineUps persisted alongside lineUpGroups - update Hive schema and migration versioning - add regression tests for legacy and round-trip deserialization
Owner
Author
|
@greptile I made some changes could you please check again |
- Define a fixed role order for strategy tiles - Return selected agents grouped as duelist, initiator, controller, sentinel
- Replace `AnimatedContainer` with `Container` in pages bar and expanded panel - Update `macos/Podfile.lock` with new plugin entries and checksums
- add saved app preference for the expanded pages bar height - let the pages bar resize via a drag handle and restore the last height
- Clip the sidebar panel to a consistent rounded border - Add hover and active states to the page resize handle - Pass resize state into the expanded pages panel
- Replace content-based minimum height with a fixed two-row minimum - Keep the expanded panel at the configured height instead of growing to fit content
- Track hovered lineup targets by group and item - Add hitbox registration and stack selector for overlapping abilities - Route tap and context-menu actions to the selected lineup item - Update hover, line painting, and widget tests for stacked abilities
SunkenInTime
force-pushed
the
small-patch
branch
from
491187c to
9b58bb1
Compare
- Close anchored context menus before running stacked item actions - Defer hitbox unregister until after frame to avoid stale state - Add regression coverage for stacked lineup delete
…ineup item interactions - Update LineUpGroupAgentWidget and LineUpItemAbilityWidget constructors to use ValueKey for better key management. - Introduce a new test to ensure remaining stacked lineup items remain interactive after deleting a sibling item. - Refactor widget tree in tests to use Consumer for dynamic updates based on provider state.
Owner
Author
|
@cursoragent review |
Findings1) P1 – “Create Lineup” path does not populate ability bar
2) P2 – Lineup migration change detection is identity-based and always “changed”
3) P3 –
|
Owner
Author
|
@cursoragent The reason why create lineup path does not populate the ability bar is because the user has to determine which agent first before we can even pull up the ability bar. That's why it's different with the add lineup branch. We already know what character we're adding the lineup for so that's why it's setting the ability bar. P2 and P3 are still valid tho |
|
Aligned with your clarification, I treated the Create Lineup ability-bar behavior as intentional and only fixed P2 + P3. What I changed
Validation run
All passed. Proactive action taken
Changes are committed and pushed to |
Co-authored-by: Dara Adedeji <SunkenInTime@users.noreply.github.com>
…ects - Integrate strategy settings provider to adjust ability size and content padding dynamically. - Refactor the lineup stack selector to use a dedicated widget for options, improving readability and maintainability. - Implement hover effects for lineup options, enhancing user interaction feedback.
- Increment version number to 4.1.0+62 in pubspec.yaml for release. - Update versionNumber and versionName constants in settings.dart to reflect the new version.
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.
No description provided.