Skip to content

Conversation

@ax0
Copy link
Collaborator

@ax0 ax0 commented Nov 7, 2025

No description provided.

Copy link
Collaborator

@arnaucube arnaucube left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! just a couple of small suggestions


pub async fn destroy_item(_params: &Params, _cfg: &Config, item: &PathBuf) -> anyhow::Result<()> {
// TODO: Nullify
std::fs::remove_file(item)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how would you see, instead of removing file, moving it under the {}/used path, as with the items that have been used?
Since we will nullify the item as with the used items.

// UI for the destruction of items.
pub(crate) fn ui_destroy(&mut self, ctx: &egui::Context, ui: &mut Ui) {
let mut item_to_destroy = self.destruction.item_index;
let all_items = self.all_items();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently in the dropdown menu of items to be destructed, the ones in the used_items list appear too; but those items have been already been nullified.
Maybe here we can just list the non-used items?

Suggested change
let all_items = self.all_items();
let all_items = self.items.clone();

@ed255 ed255 self-requested a review November 10, 2025 14:33
ui.end_row();
});
ui.separator();
egui::ComboBox::from_id_salt("destroyable items")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you replace this ComboBox by a drag-n-drop droppable label so that the UI is consistent with the inputs of Crafting?

}

impl App {
pub fn update_action_ui(&mut self, ctx: &egui::Context, ui: &mut Ui) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to request a minimum width of this right panel so that the predicates don't wrap (too much).

I tried to implement it and wasn't able to get it working reliably so instead I want to request the removal of the Grid which allows the panel to be resizable (so that we can resize it to avoid the wrapping of predicates):

diff --git a/app_gui/src/main.rs b/app_gui/src/main.rs
index bf47292..da9cf02 100644
--- a/app_gui/src/main.rs
+++ b/app_gui/src/main.rs
@@ -167,47 +167,43 @@ impl eframe::App for App {
 
 impl App {
     pub fn update_action_ui(&mut self, ctx: &egui::Context, ui: &mut Ui) {
-        egui::Grid::new("action UI").show(ui, |ui| {
-            ui.set_min_height(32.0);
-            ui.vertical(|ui| {
-                ui.horizontal(|ui| {
-                    if ui
-                        .selectable_label(self.selected_tab == 0, "Mine")
-                        .clicked()
-                    {
-                        self.crafting.selected_recipe = None;
-                        self.selected_tab = 0;
-                    }
-                    if ui
-                        .selectable_label(self.selected_tab == 1, "Craft")
-                        .clicked()
-                    {
-                        self.crafting.selected_recipe = None;
-                        self.selected_tab = 1;
-                    }
-                    if ui
-                        .selectable_label(self.selected_tab == 2, "Destroy")
-                        .clicked()
-                    {
-                        self.destruction.item_index = None;
-                        self.selected_tab = 2;
-                    }
-                    if ui
-                        .selectable_label(self.modal_new_predicates, "+ New Predicate")
-                        .clicked()
-                    {
-                        self.modal_new_predicates = true;
-                    }
-                });
-                ui.separator();
-                match self.selected_tab {
-                    0 => self.ui_produce(ctx, ui, ProductionType::Mine),
-                    1 => self.ui_produce(ctx, ui, ProductionType::Craft),
-                    2 => self.ui_destroy(ctx, ui),
-                    _ => {}
+        ui.vertical(|ui| {
+            ui.horizontal(|ui| {
+                if ui
+                    .selectable_label(self.selected_tab == 0, "Mine")
+                    .clicked()
+                {
+                    self.crafting.selected_recipe = None;
+                    self.selected_tab = 0;
+                }
+                if ui
+                    .selectable_label(self.selected_tab == 1, "Craft")
+                    .clicked()
+                {
+                    self.crafting.selected_recipe = None;
+                    self.selected_tab = 1;
+                }
+                if ui
+                    .selectable_label(self.selected_tab == 2, "Destroy")
+                    .clicked()
+                {
+                    self.destruction.item_index = None;
+                    self.selected_tab = 2;
+                }
+                if ui
+                    .selectable_label(self.modal_new_predicates, "+ New Predicate")
+                    .clicked()
+                {
+                    self.modal_new_predicates = true;
                 }
             });
-            ui.end_row();
+            ui.separator();
+            match self.selected_tab {
+                0 => self.ui_produce(ctx, ui, ProductionType::Mine),
+                1 => self.ui_produce(ctx, ui, ProductionType::Craft),
+                2 => self.ui_destroy(ctx, ui),
+                _ => {}
+            }
         });
     }
 }

@arnaucube arnaucube mentioned this pull request Nov 10, 2025
@ed255 ed255 merged commit 57ed2a4 into 0xPARC:main Nov 11, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants