mirror of
https://github.com/OrcaSlicer/OrcaSlicer.git
synced 2026-10-10 17:21:10 +00:00
Stop duplicate mode names from trapping a plate's mode cycle
The editor only replaced an empty name, so a row could be given a name another row already had. A plate stores its mode by name and find_imex_mode() takes the first row with it, so the second row was unreachable, and the plate's mode list repeated the name: left-click stuck on it, or looped without getting back to Primary. An edited name that another row already carries, or the reserved Primary name in any case, is now replaced when the edit is committed: "copy" becomes "copy 2". The edited row yields, so plates keep resolving to the row they meant, and tabbing through a field without changing it checks nothing. Resetting a row to a saved name that another row has since taken does the same. The plate's mode list comes from imex_plate_mode_choices(), which lists each name once, so a profile that already has duplicates still cycles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
8af5fa45e7
commit
9c5f4ebe47
@@ -651,6 +651,15 @@ std::vector<ImexMode> imex_mode_table(const ConfigBase& cfg)
|
||||
return table;
|
||||
}
|
||||
|
||||
std::vector<std::string> imex_plate_mode_choices(const ConfigBase& cfg)
|
||||
{
|
||||
std::vector<std::string> choices{ kImexPrimaryMode };
|
||||
for (const ImexMode& m : imex_mode_table(cfg))
|
||||
if (!m.name.empty() && std::find(choices.begin(), choices.end(), m.name) == choices.end())
|
||||
choices.push_back(m.name);
|
||||
return choices;
|
||||
}
|
||||
|
||||
bool imex_hull_violates_zones(const std::vector<BoundingBoxf3>& zones, const Polygon& hull)
|
||||
{
|
||||
if (hull.points.empty())
|
||||
|
||||
@@ -40,8 +40,8 @@ inline constexpr const char* kImexPrimaryMode = "primary";
|
||||
// Resolution rule. Applied by find_imex_mode() / imex_mode_table() and NOWHERE else;
|
||||
// every consumer in the tree goes through one of the two:
|
||||
// * `imex_mode_names` is the roster. A mode exists iff a row of that array carries its
|
||||
// name, and the FIRST such row wins. (The modes editor uniquifies names on entry, so
|
||||
// duplicates only reach here from a hand-edited profile; first-match is what a
|
||||
// name, and the FIRST such row wins. (The modes editor makes a name unique when it is
|
||||
// committed, so duplicates reach here from a hand-edited profile; first-match is what a
|
||||
// std::find over the names array already did, and what the editor's own row order
|
||||
// means.)
|
||||
// * A sibling array too short to reach that row yields an EMPTY string for that field
|
||||
@@ -87,6 +87,11 @@ ImexMode find_imex_mode(const ConfigBase& cfg, const std::string& name);
|
||||
// wants every mode (the modes editor, the plate's mode menu) rather than one by name.
|
||||
std::vector<ImexMode> imex_mode_table(const ConfigBase& cfg);
|
||||
|
||||
// The modes a plate can be set to, in the order its mode button cycles them: kImexPrimaryMode,
|
||||
// then each name in the table once. A repeated name is listed at its first row only, the one
|
||||
// find_imex_mode() resolves it to, and empty names and rows named kImexPrimaryMode are left out.
|
||||
std::vector<std::string> imex_plate_mode_choices(const ConfigBase& cfg);
|
||||
|
||||
// =============================================================================
|
||||
// PHYSICAL vs LOGICAL extruder indices — read this before adding a new IMEX call site
|
||||
// =============================================================================
|
||||
|
||||
@@ -5,6 +5,7 @@
|
||||
#include <wx/stattext.h>
|
||||
|
||||
#include <algorithm>
|
||||
#include <boost/algorithm/string/predicate.hpp>
|
||||
#include <utility>
|
||||
|
||||
#include "slic3r/GUI/EditGCodeDialog.hpp"
|
||||
@@ -262,7 +263,7 @@ std::map<int, ImexRole> IMEXModesCtrl::roles_for_mode(const std::string& active_
|
||||
return roles;
|
||||
}
|
||||
|
||||
std::string IMEXModesCtrl::unique_mode_name(const std::vector<std::string>& also_taken) const {
|
||||
std::string IMEXModesCtrl::unique_mode_name(const std::vector<std::string>& also_taken, const std::string& base) const {
|
||||
auto is_taken = [&](const std::string& cand) {
|
||||
if (std::find(also_taken.begin(), also_taken.end(), cand) != also_taken.end())
|
||||
return true;
|
||||
@@ -272,12 +273,35 @@ std::string IMEXModesCtrl::unique_mode_name(const std::vector<std::string>& also
|
||||
return false;
|
||||
};
|
||||
for (int n = 2; ; ++n) {
|
||||
std::string cand = "Mode " + std::to_string(n);
|
||||
std::string cand = base + " " + std::to_string(n);
|
||||
if (!is_taken(cand))
|
||||
return cand;
|
||||
}
|
||||
}
|
||||
|
||||
bool IMEXModesCtrl::fix_row_name(Row& r) {
|
||||
if (r.is_primary || !r.name)
|
||||
return false;
|
||||
const std::string name = into_u8(r.name->GetTextCtrl()->GetValue());
|
||||
// Case-insensitive: a row named "Primary" works, but its menu entry reads the same as the
|
||||
// built-in Primary's.
|
||||
const bool reserved = boost::iequals(name, kImexPrimaryMode);
|
||||
bool taken = false;
|
||||
for (const Row& other : m_rows)
|
||||
if (&other != &r && !other.is_primary && other.name && into_u8(other.name->GetTextCtrl()->GetValue()) == name)
|
||||
taken = true;
|
||||
if (!name.empty() && !reserved && !taken)
|
||||
return false;
|
||||
std::string base = name;
|
||||
// "copy 2" taken becomes "copy 3", not "copy 2 2".
|
||||
if (const size_t sp = base.find_last_of(' '); sp != std::string::npos && sp > 0 && sp + 1 < base.size() &&
|
||||
base.find_first_not_of("0123456789", sp + 1) == std::string::npos)
|
||||
base.erase(sp);
|
||||
// ChangeValue(), not SetValue(): no nested wxEVT_TEXT.
|
||||
r.name->GetTextCtrl()->ChangeValue(from_u8(name.empty() || reserved ? unique_mode_name({}) : unique_mode_name({}, base)));
|
||||
return true;
|
||||
}
|
||||
|
||||
IMEXModesCtrl::RoleStyle IMEXModesCtrl::role_style(std::optional<ImexRole> role) {
|
||||
// Gray / "Inactive" is the no-role answer; every role gets an explicit case so a new
|
||||
// one is a compile-time -Wswitch prompt rather than a tile that silently renders gray.
|
||||
@@ -501,15 +525,24 @@ void IMEXModesCtrl::add_row(const std::string& name,
|
||||
// Restore a name rather than let the row reach get_mode_data() unnamed.
|
||||
// Row is located by panel pointer (stable across add/remove) so a
|
||||
// kill-focus delivered while the rows are being torn down is a no-op.
|
||||
r.name->GetTextCtrl()->Bind(wxEVT_SET_FOCUS, [this, panel = r.panel](wxFocusEvent& e) {
|
||||
e.Skip();
|
||||
for (auto& row_ref : m_rows)
|
||||
if (row_ref.panel == panel && row_ref.name)
|
||||
row_ref.focus_name = into_u8(row_ref.name->GetTextCtrl()->GetValue());
|
||||
});
|
||||
r.name->GetTextCtrl()->Bind(wxEVT_KILL_FOCUS, [this, panel = r.panel](wxFocusEvent& e) {
|
||||
e.Skip();
|
||||
if (m_clearing_rows) return; // focus-out emitted while the rows are being deleted
|
||||
for (auto& row_ref : m_rows) {
|
||||
if (row_ref.panel != panel) continue;
|
||||
if (!row_ref.name || !row_ref.name->GetTextCtrl()->GetValue().empty()) return;
|
||||
// ChangeValue(), not SetValue(): no nested wxEVT_TEXT.
|
||||
row_ref.name->GetTextCtrl()->ChangeValue(from_u8(unique_mode_name({})));
|
||||
notify();
|
||||
// Only an edit is checked. Tabbing through a hand-edited profile's duplicate
|
||||
// leaves it alone rather than renaming the row plates resolve to.
|
||||
const wxString value = row_ref.name ? row_ref.name->GetTextCtrl()->GetValue() : wxString();
|
||||
if (!value.empty() && into_u8(value) == row_ref.focus_name)
|
||||
return;
|
||||
if (fix_row_name(row_ref))
|
||||
notify();
|
||||
return;
|
||||
}
|
||||
});
|
||||
@@ -806,8 +839,11 @@ void IMEXModesCtrl::reset_row_to_parent(wxPanel* panel) {
|
||||
Row& r = m_rows[i];
|
||||
if (r.panel != panel) continue;
|
||||
if (!p_names || i >= p_names->values.size()) return;
|
||||
if (!r.is_primary && r.name)
|
||||
if (!r.is_primary && r.name) {
|
||||
r.name->GetTextCtrl()->ChangeValue(from_u8(p_names->values[i]));
|
||||
// The saved name may since have been given to another row, which keeps it.
|
||||
fix_row_name(r);
|
||||
}
|
||||
if (p_gcodes && i < p_gcodes->values.size())
|
||||
r.gcode->ChangeValue(from_u8(p_gcodes->values[i]));
|
||||
if (p_tools && i < p_tools->values.size()) {
|
||||
|
||||
@@ -121,14 +121,14 @@ private:
|
||||
// outside the currently visible rows × cols without losing data on save.
|
||||
static std::map<int, ImexRole> roles_for_mode(const std::string& active_tools);
|
||||
|
||||
// "Mode N" for the lowest N >= 2 not already used by a row or by `also_taken`
|
||||
// (N == 1 is conceptually the fixed Primary row). Deterministic for a given set
|
||||
// of rows, so get_mode_data() is stable across calls and matches_config() stays
|
||||
// `base` + " N" for the lowest N >= 2 not already used by a row or by `also_taken`
|
||||
// (N == 1 is the fixed Primary row, or the name being made unique). Deterministic for a
|
||||
// given set of rows, so get_mode_data() is stable across calls and matches_config() stays
|
||||
// honest. Intentionally NOT translated: mode names are identifiers — objects
|
||||
// store one in `imex_parallel_mode` and GCode.cpp matches it against
|
||||
// `imex_mode_names` by string — so a locale-dependent name would break a project
|
||||
// opened under a different language.
|
||||
std::string unique_mode_name(const std::vector<std::string>& also_taken) const;
|
||||
std::string unique_mode_name(const std::vector<std::string>& also_taken, const std::string& base = "Mode") const;
|
||||
|
||||
// Everything the editor draws for one role, in ONE place: a new role needs a color and
|
||||
// a legend name here and nowhere else in this file. nullopt is Inactive (gray).
|
||||
@@ -172,6 +172,9 @@ private:
|
||||
// the plates a delete strands. Rebuilding the rows (load_from_config / set_grid_size)
|
||||
// re-seeds it, which is correct: config and rows agree again at that point.
|
||||
std::string orig_name;
|
||||
// The name when the field last took focus, so a commit that leaves it unchanged is not
|
||||
// treated as an edit.
|
||||
std::string focus_name;
|
||||
bool is_primary {false};
|
||||
ScalableButton* reset_btn {nullptr}; // nullptr when row has no parent counterpart
|
||||
bool reset_dirty_cached {false};
|
||||
@@ -209,6 +212,11 @@ private:
|
||||
|
||||
std::string active_tools_string(const Row& r) const;
|
||||
|
||||
// Replaces an edited row's name when it is empty, the reserved Primary name in any case, or
|
||||
// one another row already carries: a plate stores the name and find_imex_mode() takes its
|
||||
// first row, so the edited row yields. Returns whether it changed the name; the caller notifies.
|
||||
bool fix_row_name(Row& r);
|
||||
|
||||
void notify();
|
||||
|
||||
// Update each row's reset bitmap to reflect current dirty state. Cached so we
|
||||
|
||||
@@ -22909,12 +22909,9 @@ int Plater::select_plate_by_hover_id(int hover_id, bool right_click, bool isModi
|
||||
ret = select_plate(plate_index);
|
||||
if (!ret) {
|
||||
PartPlate* curr_plate = p->partplate_list.get_curr_plate();
|
||||
// Build ordered mode list: kImexPrimaryMode first, then all named modes.
|
||||
std::vector<std::string> modes;
|
||||
modes.push_back(kImexPrimaryMode);
|
||||
const DynamicPrintConfig& printer_cfg = wxGetApp().preset_bundle->printers.get_edited_preset().config;
|
||||
for (const ImexMode& m : imex_mode_table(printer_cfg))
|
||||
if (!m.name.empty() && m.name != kImexPrimaryMode) modes.push_back(m.name);
|
||||
// Ordered mode list: kImexPrimaryMode first, then each named mode once.
|
||||
const std::vector<std::string> modes =
|
||||
imex_plate_mode_choices(wxGetApp().preset_bundle->printers.get_edited_preset().config);
|
||||
|
||||
if (right_click) {
|
||||
// Show a popup menu with all modes.
|
||||
|
||||
@@ -1330,6 +1330,14 @@ TEST_CASE("imex_mode_table - every row agrees with find_imex_mode on that name",
|
||||
}
|
||||
}
|
||||
|
||||
TEST_CASE("A plate offers Primary then each mode name once in table order", "[IMEX]") {
|
||||
// A profile can carry the same name twice; a plate stores a mode by name, so only the
|
||||
// first row of a name is reachable, and listing it twice traps the plate's mode cycle.
|
||||
const DynamicPrintConfig cfg = mode_cfg({ "copy", "mirror", "copy", "", "primary", "iq-copy" }, {}, {});
|
||||
REQUIRE(imex_plate_mode_choices(cfg) == std::vector<std::string>{ kImexPrimaryMode, "copy", "mirror", "iq-copy" });
|
||||
REQUIRE(imex_plate_mode_choices(mode_cfg({}, {}, {})) == std::vector<std::string>{ kImexPrimaryMode });
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// imex_resolve_routing — the one derivation shared by the hard block and the warning
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user