Conversation
|
Got feedback from @DoubleKing-Prime and @trevlar Feedback/Reviews: Can you group them by selecting them on the map. Either drag-selecting them or just ctrl-selecting?I was just thinking that I think the way most people will default to trying to do it is multi-selecting, and then looking for a button to group them. Working on a few different options starting with the plan below: Option 1: Canvas "Group Selected" Action Bar
Why this is best:
|
|
Based on the feedback, I would like an additional way to add minions to groups:
The option you suggested option could be added as well, they are not mutually exclusive. Edit: Not sure if your option had this in mind but either way. I don't like it when UI:s aren't clear about what I'm doing, since there is so much stuff on the canvas during combat it should be very clear when you are "in group mode". I know dnd5e has this issue where grouping is very unintuitive and many times we've had monsters being grouped without it being clear and it's also a hassle to remove it. |
Agreed. We could have multiple options to fit different user preferences Edit: I agree, we will get it dialed in so it is both intuitive and clear what it is doing. |
ec93f12 to
6321210
Compare
…haracter cards they own.
d6ce8d6 to
ce8a31f
Compare
|
@7D7D awesome work here! Just a very important note that we are not allowed to use the Nimble logo as per the 3rd party license:
Either remove it or replace it with the compatibility logo found on https://nimblerpg.com/pages/creators |
Fronix
left a comment
There was a problem hiding this comment.
A few things I'd like to see that are a bit codesmelly
The NCS window and related code should probably be named accordingly. As this looks like something we will probably build upon we should try to keep the code and related files named in such as way that's it easier to understand.
src/utils/minionGrouping.ts
Outdated
| export const MINION_GROUP_IDENTITY_COLOR_PALETTE = [ | ||
| '#3B82F6', // A - Blue | ||
| '#F97316', // B - Orange | ||
| '#8B5CF6', // C - Violet | ||
| '#FACC15', // D - Yellow | ||
| '#06B6D4', // E - Cyan | ||
| '#D946EF', // F - Magenta | ||
| ] as const; |
There was a problem hiding this comment.
Any reason to hardcode these values like this? I'd like to se those editable
There was a problem hiding this comment.
I will remove those, they were from the old system, just missed them on the clean up.
I did try to do that. I had to change the old names "Canvas Lite" "Group Mode" etc... but I may have missed some. If you see any let me know. Going forward I am using:
|
|
Changed Logo to the "Nimble Compatibility Logos". I prefer the Nimble logo used on the book (that as a bonus works well in light and dark mode thanks to the black text outline). I am hoping we can get permission from Even to use that Logo instead. I cleaned/removed old combat system code. Improved some of the dice roll value logic for the NCS |
| await this.#syncTurnToCombatant(previousActiveCombatantId, { persist: false }); | ||
| return updated ? [updated] : []; | ||
| } | ||
| } |
There was a problem hiding this comment.
Scream 😱 We've got to start getting a handle on these long, complicated files. I think in the future I'll set limits on file length and function length. Essentially, keeping the total cyclomatic complexity down reduces the likelihood of bugs being introduced.
There was a problem hiding this comment.
Yeah I’m trying to condense it right now. I will switch the PR to draft. I think it may be coming from the previous versions of this feature, I’m trying to make sure all the unnecessary code is actually cleaned out correctly. I’ve done a few passes
trevlar
left a comment
There was a problem hiding this comment.
- Duplicated canCurrentUserReorderCombatant function — Defined in both combat.svelte.ts:25 and CombatTracker.svelte:94. Consider consolidating.
- Test fixture refactoring — Shared fixtures were inlined into individual test files, increasing duplication.














See latest comment for massive UPDATE
🎥 Minion Group Mode Demo
Click the image below to watch:
PR Summary (Updated)
This PR adds full minion-only grouping support to the Combat Tracker so grouped minions share one turn slot, while still preserving per-minion state and management. It is focused on turn order and activation flow only (no grouped attack roll mechanics).
Key functionality delivered
Turn-order and reordering stability fixes
UI/UX additions and refinements
Most recent popover changes (since prior summary)
Group button logic and scenarios