Skip to content

Commit

Permalink
Rework jobs view & error handling improvements
Browse files Browse the repository at this point in the history 
Job status is now shown in the top menu bar,
with a new Jobs window that can be toggled.

Build and diff errors are now handled more
gracefully.

Fixes #40
  • Loading branch information
@encounter
encounter committed Sep 28, 2024
1 parent bb039a1 commit c7b8551
Show file tree
Hide file tree
Showing 5 changed files with 233 additions and 78 deletions.
10 changes: 8 additions & 2 deletions objdiff-gui/src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use crate::{
frame_history::FrameHistory,
function_diff::function_diff_ui,
graphics::{graphics_window, GraphicsConfig, GraphicsViewState},
jobs::jobs_ui,
jobs::{jobs_menu_ui, jobs_window},
rlwinm::{rlwinm_decode_window, RlwinmDecodeViewState},
symbol_diff::{symbol_diff_ui, DiffViewState, View},
},
Expand All @@ -62,6 +62,7 @@ pub struct ViewState {
pub show_arch_config: bool,
pub show_debug: bool,
pub show_graphics: bool,
pub show_jobs: bool,
}

/// The configuration for a single object file.
Expand Down Expand Up @@ -483,6 +484,7 @@ impl eframe::App for App {
show_arch_config,
show_debug,
show_graphics,
show_jobs,
} = view_state;

frame_history.on_new_frame(ctx.input(|i| i.time), frame.info().cpu_usage);
Expand Down Expand Up @@ -598,6 +600,10 @@ impl eframe::App for App {
state.queue_reload = true;
}
});
ui.separator();
if jobs_menu_ui(ui, jobs, appearance) {
*show_jobs = !*show_jobs;
}
});
});

Expand All @@ -618,7 +624,6 @@ impl eframe::App for App {
egui::SidePanel::left("side_panel").show(ctx, |ui| {
egui::ScrollArea::both().show(ui, |ui| {
config_ui(ui, state, show_project_config, config_state, appearance);
jobs_ui(ui, jobs, appearance);
});
});

Expand All @@ -634,6 +639,7 @@ impl eframe::App for App {
arch_config_window(ctx, state, show_arch_config, appearance);
debug_window(ctx, show_debug, frame_history, appearance);
graphics_window(ctx, show_graphics, frame_history, graphics_state, appearance);
jobs_window(ctx, show_jobs, jobs, appearance);

self.post_update(ctx);
}
Expand Down
12 changes: 7 additions & 5 deletions objdiff-gui/src/jobs/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,15 @@ impl JobQueue {
/// Clears all finished jobs.
pub fn clear_finished(&mut self) {
self.jobs.retain(|job| {
!(job.should_remove
&& job.handle.is_none()
&& job.context.status.read().unwrap().error.is_none())
!(job.handle.is_none() && job.context.status.read().unwrap().error.is_none())
});
}

/// Clears all errored jobs.
pub fn clear_errored(&mut self) {
self.jobs.retain(|job| job.context.status.read().unwrap().error.is_none());
}

/// Removes a job from the queue given its ID.
pub fn remove(&mut self, id: usize) { self.jobs.retain(|job| job.id != id); }
}
Expand All @@ -107,7 +110,6 @@ pub struct JobState {
pub handle: Option<JoinHandle<JobResult>>,
pub context: JobContext,
pub cancel: Sender<()>,
pub should_remove: bool,
}

#[derive(Default)]
Expand Down Expand Up @@ -163,7 +165,7 @@ fn start_job(
});
let id = JOB_ID.fetch_add(1, Ordering::Relaxed);
log::info!("Started job {}", id);
JobState { id, kind, handle: Some(handle), context, cancel: tx, should_remove: true }
JobState { id, kind, handle: Some(handle), context, cancel: tx }
}

fn update_status(
Expand Down
95 changes: 66 additions & 29 deletions objdiff-gui/src/jobs/objdiff.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
sync::mpsc::Receiver,
};

use anyhow::{anyhow, Context, Error, Result};
use anyhow::{anyhow, Error, Result};
use objdiff_core::{
diff::{diff_objs, DiffObjConfig, ObjDiff},
obj::{read, ObjInfo},
Expand Down Expand Up @@ -189,81 +189,118 @@ fn run_build(
None
};

let mut total = 3;
let mut total = 1;
if config.build_target && target_path_rel.is_some() {
total += 1;
}
if config.build_base && base_path_rel.is_some() {
total += 1;
}
let first_status = match target_path_rel {
if target_path_rel.is_some() {
total += 1;
}
if base_path_rel.is_some() {
total += 1;
}

let mut step_idx = 0;
let mut first_status = match target_path_rel {
Some(target_path_rel) if config.build_target => {
update_status(
context,
format!("Building target {}", target_path_rel.display()),
0,
step_idx,
total,
&cancel,
)?;
step_idx += 1;
run_make(&config.build_config, target_path_rel)
}
_ => BuildStatus::default(),
};

let second_status = match base_path_rel {
let mut second_status = match base_path_rel {
Some(base_path_rel) if config.build_base => {
update_status(
context,
format!("Building base {}", base_path_rel.display()),
0,
step_idx,
total,
&cancel,
)?;
step_idx += 1;
run_make(&config.build_config, base_path_rel)
}
_ => BuildStatus::default(),
};

let time = OffsetDateTime::now_utc();

let first_obj =
match &obj_config.target_path {
Some(target_path) if first_status.success => {
update_status(
context,
format!("Loading target {}", target_path_rel.unwrap().display()),
2,
total,
&cancel,
)?;
Some(read::read(target_path, &config.diff_obj_config).with_context(|| {
format!("Failed to read object '{}'", target_path.display())
})?)
let first_obj = match &obj_config.target_path {
Some(target_path) if first_status.success => {
update_status(
context,
format!("Loading target {}", target_path_rel.unwrap().display()),
step_idx,
total,
&cancel,
)?;
step_idx += 1;
match read::read(target_path, &config.diff_obj_config) {
Ok(obj) => Some(obj),
Err(e) => {
first_status = BuildStatus {
success: false,
stdout: format!("Loading object '{}'", target_path.display()),
stderr: format!("{:#}", e),
..Default::default()
};
None
}
}
_ => None,
};
}
Some(_) => {
step_idx += 1;
None
}
_ => None,
};

let second_obj = match &obj_config.base_path {
Some(base_path) if second_status.success => {
update_status(
context,
format!("Loading base {}", base_path_rel.unwrap().display()),
3,
step_idx,
total,
&cancel,
)?;
Some(
read::read(base_path, &config.diff_obj_config)
.with_context(|| format!("Failed to read object '{}'", base_path.display()))?,
)
step_idx += 1;
match read::read(base_path, &config.diff_obj_config) {
Ok(obj) => Some(obj),
Err(e) => {
second_status = BuildStatus {
success: false,
stdout: format!("Loading object '{}'", base_path.display()),
stderr: format!("{:#}", e),
..Default::default()
};
None
}
}
}
Some(_) => {
step_idx += 1;
None
}
_ => None,
};

update_status(context, "Performing diff".to_string(), 4, total, &cancel)?;
update_status(context, "Performing diff".to_string(), step_idx, total, &cancel)?;
step_idx += 1;
let result = diff_objs(&config.diff_obj_config, first_obj.as_ref(), second_obj.as_ref(), None)?;

update_status(context, "Complete".to_string(), total, total, &cancel)?;
update_status(context, "Complete".to_string(), step_idx, total, &cancel)?;
Ok(Box::new(ObjDiffResult {
first_status,
second_status,
Expand All @@ -274,7 +311,7 @@ fn run_build(
}

pub fn start_build(ctx: &egui::Context, config: ObjDiffConfig) -> JobState {
start_job(ctx, "Object diff", Job::ObjDiff, move |context, cancel| {
start_job(ctx, "Build", Job::ObjDiff, move |context, cancel| {
run_build(&context, cancel, config).map(|result| JobResult::ObjDiff(Some(result)))
})
}
Loading

0 comments on commit c7b8551

Please sign in to comment.