Skip to content

Commit 09cd4e6

Browse files
committed
Simplify Subsplit Editor State
This exposes segment group state separately from flat segment rows in the run editor and removes the extra per-row group metadata that the frontend does not need for rendering or editing. This also updates the C API and TypeScript binding generation for the restructured editor state and covers the group selection behavior in editor tests.
1 parent fd46651 commit 09cd4e6

6 files changed

Lines changed: 124 additions & 85 deletions

File tree

capi/bind_gen/src/typescript.ts

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,8 @@ export interface RunEditorStateJson {
895895
timing_method: TimingMethodJson,
896896
/** The state of all the segments. */
897897
segments: RunEditorRowJson[],
898+
/** The segment groups of the run. */
899+
segment_groups: RunEditorSegmentGroupJson[],
898900
/** The names of all the custom comparisons that exist for this Run. */
899901
comparison_names: string[],
900902
/** Describes which actions are currently available. */
@@ -999,8 +1001,8 @@ export interface RunEditorButtonsJson {
9991001
*/
10001002
can_create_segment_group: boolean,
10011003
/**
1002-
* Describes whether the currently selected segments are exactly one
1003-
* segment group that can be removed.
1004+
* Describes whether the currently selected segments are exactly one or more
1005+
* segment groups that can be removed.
10041006
*/
10051007
can_remove_segment_group: boolean,
10061008
}
@@ -1029,18 +1031,16 @@ export interface RunEditorRowJson {
10291031
comparison_times: string[],
10301032
/** Describes the segment's selection state. */
10311033
selected: "NotSelected" | "Selected" | "Active",
1032-
/** Describes how this segment participates in a segment group. */
1033-
segment_group: RunEditorSegmentGroupStateJson,
1034+
/** The index of the group this segment belongs to, if any. */
1035+
segment_group_index: number | null,
10341036
}
10351037

1036-
/** Describes a segment's role in a segment group. */
1037-
export interface RunEditorSegmentGroupStateJson {
1038-
/** The index of the group this segment belongs to, if any. */
1039-
group_index: number | null,
1040-
/** Whether this segment is a subsplit inside the group. */
1041-
is_subsplit: boolean,
1042-
/** Whether this segment is the major split ending the group. */
1043-
is_major_split: boolean,
1038+
/** Describes a segment group. */
1039+
export interface RunEditorSegmentGroupJson {
1040+
/** The first segment in the group. */
1041+
start: number,
1042+
/** The segment after the last segment in the group. */
1043+
end: number,
10441044
/**
10451045
* The explicit group name. If this is `null`, the major split name is the
10461046
* display name of the group.

capi/src/run_editor.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,7 @@ pub unsafe extern "C" fn RunEditor_create_segment_group_from_selection(
328328
}
329329
}
330330

331-
/// Removes the native segment group containing the active segment, while
332-
/// keeping all segments.
331+
/// Removes the selected native segment groups, while keeping all segments.
333332
#[unsafe(no_mangle)]
334333
pub extern "C" fn RunEditor_remove_active_segment_group(this: &mut RunEditor) -> bool {
335334
this.remove_active_segment_group()

crates/livesplit-hotkey/src/macos/mod.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -456,25 +456,29 @@ unsafe extern "C" fn callback(
456456
let modifier_flags = unsafe { CGEventGetFlags(event) };
457457
let mut modifiers = Modifiers::empty();
458458

459-
if modifier_flags.intersects(EventFlags::SHIFT | EventFlags::LEFT_SHIFT | EventFlags::RIGHT_SHIFT)
459+
if modifier_flags
460+
.intersects(EventFlags::SHIFT | EventFlags::LEFT_SHIFT | EventFlags::RIGHT_SHIFT)
460461
&& !matches!(key_code, KeyCode::ShiftLeft | KeyCode::ShiftRight)
461462
{
462463
modifiers.insert(Modifiers::SHIFT);
463464
}
464465

465-
if modifier_flags.intersects(EventFlags::CONTROL | EventFlags::LEFT_CONTROL | EventFlags::RIGHT_CONTROL)
466+
if modifier_flags
467+
.intersects(EventFlags::CONTROL | EventFlags::LEFT_CONTROL | EventFlags::RIGHT_CONTROL)
466468
&& !matches!(key_code, KeyCode::ControlLeft | KeyCode::ControlRight)
467469
{
468470
modifiers.insert(Modifiers::CONTROL);
469471
}
470472

471-
if modifier_flags.intersects(EventFlags::OPTION | EventFlags::LEFT_OPTION | EventFlags::RIGHT_OPTION)
473+
if modifier_flags
474+
.intersects(EventFlags::OPTION | EventFlags::LEFT_OPTION | EventFlags::RIGHT_OPTION)
472475
&& !matches!(key_code, KeyCode::AltLeft | KeyCode::AltRight)
473476
{
474477
modifiers.insert(Modifiers::ALT);
475478
}
476479

477-
if modifier_flags.intersects(EventFlags::COMMAND | EventFlags::LEFT_COMMAND | EventFlags::RIGHT_COMMAND)
480+
if modifier_flags
481+
.intersects(EventFlags::COMMAND | EventFlags::LEFT_COMMAND | EventFlags::RIGHT_COMMAND)
478482
&& !matches!(key_code, KeyCode::MetaLeft | KeyCode::MetaRight)
479483
{
480484
modifiers.insert(Modifiers::META);

src/run/editor/mod.rs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,12 +1119,30 @@ impl Editor {
11191119
}
11201120

11211121
fn selected_segment_group_index(&self) -> Option<usize> {
1122-
let (start, end) = self.selected_segment_range()?;
1123-
self.run
1124-
.segment_groups()
1125-
.groups()
1126-
.iter()
1127-
.position(|group| group.start() == start && group.end() == end)
1122+
let indices = self.selected_segment_group_indices()?;
1123+
(indices.len() == 1).then_some(indices[0])
1124+
}
1125+
1126+
fn selected_segment_group_indices(&self) -> Option<Vec<usize>> {
1127+
let mut group_indices = Vec::new();
1128+
let mut grouped_segment_count = 0;
1129+
1130+
for (group_index, group) in self.run.segment_groups().groups().iter().enumerate() {
1131+
let selected_in_group = (group.start()..group.end())
1132+
.filter(|index| self.selected_segments.contains(index))
1133+
.count();
1134+
if selected_in_group == 0 {
1135+
continue;
1136+
}
1137+
if selected_in_group != group.end() - group.start() {
1138+
return None;
1139+
}
1140+
group_indices.push(group_index);
1141+
grouped_segment_count += selected_in_group;
1142+
}
1143+
1144+
(!group_indices.is_empty() && grouped_segment_count == self.selected_segments.len())
1145+
.then_some(group_indices)
11281146
}
11291147

11301148
/// Checks if the currently selected segments can be turned into a group.
@@ -1144,9 +1162,10 @@ impl Editor {
11441162
.any(|group| start < group.end() && end > group.start())
11451163
}
11461164

1147-
/// Checks if the currently selected segments are exactly one whole group.
1165+
/// Checks if the currently selected segments are exactly one or more whole
1166+
/// groups.
11481167
pub fn can_remove_active_segment_group(&self) -> bool {
1149-
self.selected_segment_group_index().is_some()
1168+
self.selected_segment_group_indices().is_some()
11501169
}
11511170

11521171
/// Creates a segment group from the currently selected contiguous segment
@@ -1170,16 +1189,18 @@ impl Editor {
11701189
true
11711190
}
11721191

1173-
/// Removes the group containing the active segment, while keeping all
1174-
/// segments.
1192+
/// Removes the selected segment groups, while keeping all segments.
11751193
pub fn remove_active_segment_group(&mut self) -> bool {
1176-
let Some(group_index) = self.selected_segment_group_index() else {
1194+
let Some(mut group_indices) = self.selected_segment_group_indices() else {
11771195
return false;
11781196
};
1179-
self.run
1180-
.segment_groups_mut()
1181-
.groups_mut()
1182-
.remove(group_index);
1197+
group_indices.sort_unstable();
1198+
for group_index in group_indices.into_iter().rev() {
1199+
self.run
1200+
.segment_groups_mut()
1201+
.groups_mut()
1202+
.remove(group_index);
1203+
}
11831204
self.raise_run_edited();
11841205
true
11851206
}

src/run/editor/state.rs

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ pub struct State {
3232
pub timing_method: TimingMethod,
3333
/// The state of all the segments.
3434
pub segments: Vec<Segment>,
35+
/// The native segment groups of the run.
36+
pub segment_groups: Vec<SegmentGroup>,
3537
/// The names of all the custom comparisons that exist for this Run.
3638
pub comparison_names: Vec<String>,
3739
/// Describes which actions are currently available.
@@ -86,19 +88,17 @@ pub struct Segment {
8688
pub comparison_times: Vec<String>,
8789
/// Describes the segment's selection state.
8890
pub selected: SelectionState,
89-
/// Describes how this segment participates in a native segment group.
90-
pub segment_group: SegmentGroupState,
91+
/// The index of the group this segment belongs to, if any.
92+
pub segment_group_index: Option<usize>,
9193
}
9294

93-
/// Describes a segment's role in a native segment group.
95+
/// Describes a native segment group.
9496
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
95-
pub struct SegmentGroupState {
96-
/// The index of the group this segment belongs to, if any.
97-
pub group_index: Option<usize>,
98-
/// Whether this segment is a subsplit inside the group.
99-
pub is_subsplit: bool,
100-
/// Whether this segment is the major split ending the group.
101-
pub is_major_split: bool,
97+
pub struct SegmentGroup {
98+
/// The first segment in the group.
99+
pub start: usize,
100+
/// The segment after the last segment in the group.
101+
pub end: usize,
102102
/// The explicit group name. If this is `None`, the major split name is the
103103
/// display name of the group.
104104
pub name: Option<String>,
@@ -171,6 +171,24 @@ impl Editor {
171171
can_create_segment_group: self.can_create_segment_group_from_selection(),
172172
can_remove_segment_group: self.can_remove_active_segment_group(),
173173
};
174+
let mut segment_groups = Vec::with_capacity(self.run.segment_groups().groups().len());
175+
for group in self.run.segment_groups().groups() {
176+
let (group_icon, has_explicit_icon) = group.icon().map_or(
177+
(self.run.segment(group.major_index()).icon(), false),
178+
|icon| (icon, true),
179+
);
180+
let icon = *image_cache
181+
.cache(group_icon.id(), || group_icon.clone())
182+
.id();
183+
segment_groups.push(SegmentGroup {
184+
start: group.start(),
185+
end: group.end(),
186+
name: group.name().map(str::to_owned),
187+
icon,
188+
has_explicit_icon,
189+
});
190+
}
191+
174192
let mut segments = Vec::with_capacity(self.run.len());
175193

176194
for segment_index in 0..self.run.len() {
@@ -198,36 +216,10 @@ impl Editor {
198216
SelectionState::NotSelected
199217
};
200218

201-
let segment_group = self
219+
let segment_group_index = self
202220
.run
203221
.segment_groups()
204-
.group_index_for_segment(segment_index)
205-
.map(|group_index| {
206-
let group = &self.run.segment_groups().groups()[group_index];
207-
let (group_icon, has_explicit_icon) = group.icon().map_or(
208-
(self.run.segment(group.major_index()).icon(), false),
209-
|icon| (icon, true),
210-
);
211-
let icon = *image_cache
212-
.cache(group_icon.id(), || group_icon.clone())
213-
.id();
214-
SegmentGroupState {
215-
group_index: Some(group_index),
216-
is_subsplit: segment_index < group.major_index(),
217-
is_major_split: segment_index == group.major_index(),
218-
name: group.name().map(str::to_owned),
219-
icon,
220-
has_explicit_icon,
221-
}
222-
})
223-
.unwrap_or(SegmentGroupState {
224-
group_index: None,
225-
is_subsplit: false,
226-
is_major_split: false,
227-
name: None,
228-
icon: *ImageId::EMPTY,
229-
has_explicit_icon: false,
230-
});
222+
.group_index_for_segment(segment_index);
231223

232224
segments.push(Segment {
233225
icon,
@@ -237,7 +229,7 @@ impl Editor {
237229
best_segment_time,
238230
comparison_times,
239231
selected,
240-
segment_group,
232+
segment_group_index,
241233
});
242234
}
243235

@@ -249,6 +241,7 @@ impl Editor {
249241
attempts,
250242
timing_method,
251243
segments,
244+
segment_groups,
252245
comparison_names,
253246
buttons,
254247
metadata: self.run.metadata().clone(),

src/run/editor/tests/mod.rs

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,11 @@ fn creates_renames_and_removes_segment_groups() {
183183

184184
let mut image_cache = ImageCache::new();
185185
let state = editor.state(&mut image_cache, Lang::English);
186-
assert_eq!(state.segments[0].segment_group.group_index, Some(0));
187-
assert!(state.segments[0].segment_group.is_subsplit);
188-
assert!(state.segments[2].segment_group.is_major_split);
189-
assert_eq!(
190-
state.segments[2].segment_group.name.as_deref(),
191-
Some("Renamed")
192-
);
186+
assert_eq!(state.segments[0].segment_group_index, Some(0));
187+
assert_eq!(state.segments[2].segment_group_index, Some(0));
188+
assert_eq!(state.segment_groups[0].start, 0);
189+
assert_eq!(state.segment_groups[0].end, 3);
190+
assert_eq!(state.segment_groups[0].name.as_deref(), Some("Renamed"));
193191

194192
assert!(editor.remove_active_segment_group());
195193
assert!(editor.close().segment_groups().groups().is_empty());
@@ -230,21 +228,21 @@ fn segment_group_icons_can_be_explicit_or_inherited() {
230228

231229
let mut image_cache = ImageCache::new();
232230
let state = editor.state(&mut image_cache, Lang::English);
233-
assert_eq!(state.segments[0].segment_group.icon, *major_icon.id());
234-
assert!(!state.segments[0].segment_group.has_explicit_icon);
231+
assert_eq!(state.segment_groups[0].icon, *major_icon.id());
232+
assert!(!state.segment_groups[0].has_explicit_icon);
235233

236234
let group_icon = Image::new([4, 5, 6].as_slice().into(), Image::ICON);
237235
assert!(editor.set_active_segment_group_icon(group_icon.clone()));
238236

239237
let state = editor.state(&mut image_cache, Lang::English);
240-
assert_eq!(state.segments[0].segment_group.icon, *group_icon.id());
241-
assert!(state.segments[0].segment_group.has_explicit_icon);
238+
assert_eq!(state.segment_groups[0].icon, *group_icon.id());
239+
assert!(state.segment_groups[0].has_explicit_icon);
242240

243241
assert!(editor.remove_active_segment_group_icon());
244242

245243
let state = editor.state(&mut image_cache, Lang::English);
246-
assert_eq!(state.segments[0].segment_group.icon, *major_icon.id());
247-
assert!(!state.segments[0].segment_group.has_explicit_icon);
244+
assert_eq!(state.segment_groups[0].icon, *major_icon.id());
245+
assert!(!state.segment_groups[0].has_explicit_icon);
248246
}
249247

250248
#[test]
@@ -300,6 +298,30 @@ fn mixed_selection_cannot_create_or_remove_segment_group() {
300298
assert!(editor.remove_active_segment_group());
301299
}
302300

301+
#[test]
302+
fn removes_multiple_selected_segment_groups() {
303+
let mut run = Run::new();
304+
for name in ["A", "B", "C", "D", "E", "F"] {
305+
run.push_segment(Segment::new(name));
306+
}
307+
308+
let mut editor = Editor::new(run).unwrap();
309+
editor.select_range(1);
310+
assert!(editor.create_segment_group_from_selection(Some("Group A")));
311+
editor.select_only(2);
312+
editor.select_range(4);
313+
assert!(editor.create_segment_group_from_selection(Some("Group B")));
314+
315+
editor.select_only(0);
316+
editor.select_additionally(1);
317+
editor.select_additionally(2);
318+
editor.select_additionally(3);
319+
editor.select_additionally(4);
320+
assert!(editor.can_remove_active_segment_group());
321+
assert!(editor.remove_active_segment_group());
322+
assert!(editor.run().segment_groups().groups().is_empty());
323+
}
324+
303325
#[test]
304326
fn ungrouped_selection_before_group_can_create_segment_group() {
305327
let mut run = Run::new();

0 commit comments

Comments
 (0)