Skip to content

Commit

Permalink
improve drop down value refresh
Browse files Browse the repository at this point in the history 
update the currently edited drop down list first (so the user doesn't
have to wait for input suggestions), and do so in an intelligent
manner. Works well with the cache.
  • Loading branch information
@Turun
Turun committed Jul 8, 2023
1 parent e581c59 commit fd703bb
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 65 deletions.
12 changes: 8 additions & 4 deletions src/message.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use core::fmt;
use std::sync::Arc;
use std::{collections::HashSet, sync::Arc};

use crate::towns::{Constraint, Town, TownSelection};

Expand Down Expand Up @@ -58,7 +58,7 @@ pub enum MessageToModel {
SetServer(Server, egui::Context),
FetchAll,
FetchGhosts,
FetchTowns(TownSelection),
FetchTowns(TownSelection, HashSet<Constraint>),
}

impl fmt::Display for MessageToModel {
Expand All @@ -67,8 +67,12 @@ impl fmt::Display for MessageToModel {
MessageToModel::SetServer(server, _frame) => {
write!(f, "MessageToMode::SetServer({})", server.id)
}
MessageToModel::FetchTowns(selection) => {
write!(f, "MessageToModel::FetchTowns({selection})")
MessageToModel::FetchTowns(selection, constraints) => {
write!(
f,
"MessageToModel::FetchTowns({selection}, {} Constraints currently edited)",
constraints.len()
)
}
MessageToModel::FetchAll => {
write!(f, "MessageToModel::FetchAll")
Expand Down
107 changes: 68 additions & 39 deletions src/presenter.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,24 +88,19 @@ impl Presenter {
let msg = towns.map(MessageToView::GhostTowns);
self.send_to_view(msg, String::from("Failed to send ghost town list to view"));
}
MessageToModel::FetchTowns(selection) => {
// TODO at the moment, when the user types something into a drop down list, we refetch the
// possible drop down values for all other drop down boxes as well. And since the box the
// user types in changed its content, the ddvlists of the other ddbs isn't even cached!
// Additionally, the ddvlist of the ddb the user is currently typing in is reset with every
// key stroke, making it painfully slow to get the ddv list suggestions. We need to fix that.
// Getting the ddvlist of the ddb the user is currently typing in has the highest priority!
// All other ddvlists can wait.
// For now, the list of filled constraints it reversed, so that - assuming the user add constraints
// at the bottom in the ui - the relevant results are sent back first.
// Ideally in the future we would process the currently edited ddb first (likely cached anyway),
// then we check if the currently edited ddv is a == or <> type. If it is and the user is currently
// changing its value, the db will probably not return any results for the other ddvlists anyway
// ("New Powe" is not an ally name, it needs to be complete before it matches anything in the db).
// Only if it is of type <= or >= does it make sense to fetch new ddvlists for the other ddb as well.
MessageToModel::FetchTowns(selection, constraints_edited) => {
// a list of filled constraints that are not being edited. For each one, filter the ddv list by all _other_ filled, unedited constratins
let constraints_filled_not_edited: Vec<Constraint> = selection
.constraints
.iter()
.rev()
.filter(|c| !c.value.is_empty())
.filter(|c| !constraints_edited.contains(c))
.map(Constraint::partial_clone)
.collect();

// a list of filled constraints. For each one, filter the ddv list by all _other_ filled constratins
let filled_constraints: Vec<Constraint> = selection
let constraints_filled_all: Vec<Constraint> = selection
.constraints
.iter()
.rev()
Expand All @@ -114,37 +109,47 @@ impl Presenter {
.collect();

// a list of empty constraints. Filter the ddv list by all non empty constraints
let empty_constraints: Vec<&Constraint> = selection
let constraints_empty: Vec<&Constraint> = selection
.constraints
.iter()
.filter(|c| c.value.is_empty())
.filter(|c| !constraints_edited.contains(c))
.collect();

let towns = self.model.get_towns_for_constraints(&filled_constraints);
// The drop down values for the constraints currently being edited
for c in constraints_edited {
let towns = self.model.get_names_for_constraint_type_with_constraints(
c.constraint_type,
&constraints_filled_not_edited,
);
let msg = towns.map(|t| {
MessageToView::ValueListForConstraint(
c.partial_clone(),
selection.partial_clone(),
t,
)
});
self.send_to_view(
msg,
String::from("Failed to send town list for currently edited drop down"),
);
}

// Towns of this selection
let towns = self
.model
.get_towns_for_constraints(&constraints_filled_all);
let msg = towns
.map(|t| MessageToView::TownListForSelection(selection.partial_clone(), t));
self.send_to_view(msg, String::from("Failed to send town list to view"));

// filled constraints
if filled_constraints.is_empty() {
// nothing
} else if filled_constraints.len() == 1 {
let c = filled_constraints[0].partial_clone();
let c_towns = self.model.get_names_for_constraint_type(c.constraint_type);
let msg = c_towns.map(|t| {
MessageToView::ValueListForConstraint(c, selection.partial_clone(), t)
});
self.send_to_view(msg, String::from("Failed to send town list to view"));
} else {
// for each constraint, make a list of all other filled constraints and get the ddv list filtered by those
for (i, c) in filled_constraints.iter().enumerate() {
let mut other_constraints = filled_constraints.clone();
let _this_constraint = other_constraints.swap_remove(i);

// drop down values for the empty constraints
if !constraints_empty.is_empty() {
for c in constraints_empty {
let c_towns =
self.model.get_names_for_constraint_type_with_constraints(
c.constraint_type,
&other_constraints,
&constraints_filled_all,
);
let msg = c_towns.map(|t| {
MessageToView::ValueListForConstraint(
Expand All @@ -160,13 +165,37 @@ impl Presenter {
}
}

// empty constraints
if !empty_constraints.is_empty() {
for c in empty_constraints {
// drop down values for the filled constraints
if constraints_filled_not_edited.is_empty() {
// nothing
} else if constraints_filled_not_edited.len() == 1 {
let c = constraints_filled_not_edited[0].partial_clone();
let c_towns = self.model.get_names_for_constraint_type(c.constraint_type);
let msg = c_towns.map(|t| {
MessageToView::ValueListForConstraint(c, selection.partial_clone(), t)
});
self.send_to_view(msg, String::from("Failed to send town list to view"));
} else {
// for each constraint, make a list of all other filled constraints and get the ddv list filtered by those
for (i, c) in constraints_filled_not_edited.iter().enumerate() {
// TODO: only a slight improvement, but we could select the drop down values
// not by constrain with all other filled constraints, but instead by all other
// filled constraints that do not, on their own, reduce the result list to zero.
// What I mean by that is that, when the user has set an alliance name or player
// name field to == and entered a partial name, that constraint will reduce the
// ddvlist of all filled constraints to an empty list. But that is not the drop
// down the user wants to see. They want to see the possible values of ddb xxx
// to show the possible values, given the other useful constraints.
// To implement this we will have to set other_constraints to a list of
// filled_constraints minus the set of filled constraints that give no
// result from the database.
let mut other_constraints = constraints_filled_all.clone();
let _this_constraint = other_constraints.swap_remove(i);

let c_towns =
self.model.get_names_for_constraint_type_with_constraints(
c.constraint_type,
&filled_constraints,
&other_constraints,
);
let msg = c_towns.map(|t| {
MessageToView::ValueListForConstraint(
Expand Down
77 changes: 55 additions & 22 deletions src/view/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod data;
mod dropdownbox;
mod selectable_label;
pub(crate) mod state;
use std::collections::HashSet;
use std::sync::{mpsc, Arc};
use std::time::Duration;
use strum::IntoEnumIterator;
Expand Down Expand Up @@ -163,21 +164,21 @@ impl View {
ui.color_edit_button_srgba(&mut self.ui_data.settings_ghosts.color);
});
ui.separator();
let mut change: Option<Change> = None;
let mut selection_change_action: Option<Change> = None;
for (index, selection) in self.ui_data.selections.iter_mut().enumerate() {
let _first_row_response = ui.horizontal(|ui| {
ui.color_edit_button_srgba(&mut selection.color);
if ui.button("Add Towns").clicked() {
change = Some(Change::Add);
selection_change_action = Some(Change::Add);
}
if ui.button("Remove").clicked() {
change = Some(Change::Remove(index));
selection_change_action = Some(Change::Remove(index));
}
if ui.button("↑").clicked() {
change = Some(Change::MoveUp(index));
selection_change_action = Some(Change::MoveUp(index));
}
if ui.button("↓").clicked() {
change = Some(Change::MoveDown(index));
selection_change_action = Some(Change::MoveDown(index));
}

ui.label(format!("{} Towns", selection.towns.len()));
Expand All @@ -188,8 +189,10 @@ impl View {
});

let num_constraints = selection.constraints.len();
let mut request_update = selection.state == SelectionState::NewlyCreated; // A newly created one must request an update immediately to fill its ddv list
let mut constraint_change = None;
let mut refresh_complete_selection =
selection.state == SelectionState::NewlyCreated;
let mut edited_constraints = HashSet::new();
let mut constraint_change_action = None;
for (cindex, constraint) in selection.constraints.iter_mut().enumerate() {
ui.horizontal(|ui| {
let _inner_response = egui::ComboBox::from_id_source(format!(
Expand All @@ -208,7 +211,7 @@ impl View {
)
.clicked()
{
request_update = true;
edited_constraints.insert(constraint.partial_clone());
}
}
});
Expand All @@ -225,7 +228,7 @@ impl View {
.selectable_value(&mut constraint.comparator, value, text)
.clicked()
{
request_update = true;
edited_constraints.insert(constraint.partial_clone());
}
}
});
Expand All @@ -245,30 +248,30 @@ impl View {
)
.changed()
{
request_update = true;
edited_constraints.insert(constraint.partial_clone());
};
if cindex + 1 == num_constraints {
if ui.button("+").clicked() {
constraint_change = Some(Change::Add);
request_update = true;
constraint_change_action = Some(Change::Add);
refresh_complete_selection = true;
}
} else {
ui.label("and");
}
if ui.button("-").clicked() {
constraint_change = Some(Change::Remove(cindex));
request_update = true;
constraint_change_action = Some(Change::Remove(cindex));
refresh_complete_selection = true;
}
if ui.button("↑").clicked() {
constraint_change = Some(Change::MoveUp(cindex));
constraint_change_action = Some(Change::MoveUp(cindex));
}
if ui.button("↓").clicked() {
constraint_change = Some(Change::MoveDown(cindex));
constraint_change_action = Some(Change::MoveDown(cindex));
}
});
}

if let Some(change) = constraint_change {
if let Some(change) = constraint_change_action {
match change {
Change::MoveUp(index) => {
if index >= 1 {
Expand All @@ -277,6 +280,10 @@ impl View {
}
Change::Remove(index) => {
let _element = selection.constraints.remove(index);
if selection.constraints.is_empty() {
// ensure there is always at least one constraint
selection.constraints.push(Constraint::default());
}
}
Change::MoveDown(index) => {
if index + 1 < selection.constraints.len() {
Expand All @@ -287,23 +294,45 @@ impl View {
}
}

if request_update {
// TODO FetchTowns Message needs an additional argument to tell the backend which constraint is currently edited.
if refresh_complete_selection {
selection.state = SelectionState::Loading;
for constraint in &mut selection.constraints {
constraint.drop_down_values = None;
}

self.channel_presenter_tx
.send(MessageToModel::FetchTowns(selection.partial_clone()))
.send(MessageToModel::FetchTowns(
selection.partial_clone(),
HashSet::new(),
))
.expect(&format!(
"Failed to send Message to Model for Selection {}",
&selection
));
} else if !edited_constraints.is_empty() {
selection.state = SelectionState::Loading;
for constraint in &mut selection.constraints {
for constraint in &mut selection
.constraints
.iter_mut()
.filter(|c| !edited_constraints.contains(c))
{
constraint.drop_down_values = None;
}

self.channel_presenter_tx
.send(MessageToModel::FetchTowns(
selection.partial_clone(),
edited_constraints,
))
.expect(&format!(
"Failed to send Message to Model for Selection {}",
&selection
));
}
ui.separator();
}

if let Some(change_action) = change {
if let Some(change_action) = selection_change_action {
match change_action {
Change::MoveUp(index) => {
if index >= 1 {
Expand All @@ -312,6 +341,10 @@ impl View {
}
Change::Remove(index) => {
let _elem = self.ui_data.selections.remove(index);
if self.ui_data.selections.is_empty() {
// ensure there is always at least one selection
self.ui_data.selections.push(TownSelection::default());
}
}
Change::MoveDown(index) => {
if index + 1 < self.ui_data.selections.len() {
Expand Down

0 comments on commit fd703bb

Please sign in to comment.