`TaskTableModel` optimizations #7756

psikomonkie · 2025-10-18T17:57:51Z

Flame graph before changes:

Flame graph after changes:

When changes happen to the table, and the user hasn't selected a Tech, it will now store a "temp tech" for that panel so it doesn't need to repeatedly get the entire list of techs and filter it down for a temp tech.

Also stops row height from repeatedly adjusting itself.

psikomonkie · 2025-10-18T17:58:20Z

MekHQ/src/mekhq/gui/model/TaskTableModel.java

+                    if (tech == null) {
+                        tech = panel.getTempTech();
+                    }
                    if (null == tech) {


Format these the same

codecov · 2025-10-18T18:04:49Z

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 12.12%. Comparing base (c3ca6b4) to head (b7636b9).
⚠️ Report is 14 commits behind head on main.

Files with missing lines	Patch %	Lines
MekHQ/src/mekhq/gui/model/TaskTableModel.java	0.00%	6 Missing ⚠️
MekHQ/src/mekhq/gui/RepairTab.java	0.00%	4 Missing ⚠️
MekHQ/src/mekhq/gui/WarehouseTab.java	0.00%	3 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #7756      +/-   ##
============================================
- Coverage     12.13%   12.12%   -0.01%     
+ Complexity     7313     7307       -6     
============================================
  Files          1210     1210              
  Lines        152774   152803      +29     
  Branches      23123    23127       +4     
============================================
- Hits          18540    18534       -6     
- Misses       132318   132347      +29     
- Partials       1916     1922       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull Request Overview

Optimize TaskTableModel rendering and tech selection to reduce repeated computation and UI churn

Only adjust table row height when necessary to avoid repeated resets during rendering
Cache a "temp tech" in panels when no tech is selected to avoid repeated tech list scans
Extend ITechWorkPanel with temp tech getters/setters and implement in RepairTab and WarehouseTab

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
MekHQ/src/mekhq/gui/model/TaskTableModel.java	Optimize row height setting and introduce use of cached temp tech when no tech is selected
MekHQ/src/mekhq/gui/WarehouseTab.java	Add tempTech storage and implement ITechWorkPanel’s new getters/setters
MekHQ/src/mekhq/gui/RepairTab.java	Add tempTech storage, implement ITechWorkPanel’s new getters/setters, and initialize new field
MekHQ/src/mekhq/gui/ITechWorkPanel.java	Extend interface with temp tech accessors

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-19T01:39:14Z

MekHQ/src/mekhq/gui/model/TaskTableModel.java

-            table.setRowHeight(UIUtil.scaleForGUI(100));
+            int newRowHeight = UIUtil.scaleForGUI(100);
+            if (table.getRowHeight(row) != newRowHeight) {
+                table.setRowHeight(newRowHeight);


This compares a per-row height (getRowHeight(row)) but updates the table-wide default height (setRowHeight(rowHeight)), which is inconsistent and can lead to unnecessary updates or overriding per-row custom heights. Compare against the default with table.getRowHeight() or set the specific row height using table.setRowHeight(row, newRowHeight). Example fix: if (table.getRowHeight() != newRowHeight) { table.setRowHeight(newRowHeight); }

Suggested change

table.setRowHeight(newRowHeight);

table.setRowHeight(row, newRowHeight);

Copilot · 2025-10-19T01:39:15Z

MekHQ/src/mekhq/gui/model/TaskTableModel.java

                    Person tech = panel.getSelectedTech();

+                    if (tech == null) {
+                        tech = panel.getTempTech();


A cached temp tech is used without verifying it can service the current entity, which can produce incorrect behavior when switching between parts or units with different tech requirements. Guard the reuse with a capability check and fall back to the search if incompatible, e.g.: if (tech == null) { Person cached = panel.getTempTech(); if (cached != null && cached.canTech(part.getUnit().getEntity())) { tech = cached; } }

Suggested change

tech = panel.getTempTech();

Person cachedTech = panel.getTempTech();

if (cachedTech != null && part.getUnit() != null && cachedTech.canTech(part.getUnit().getEntity())) {

tech = cachedTech;

}

Copilot · 2025-10-19T01:39:15Z

MekHQ/src/mekhq/gui/RepairTab.java

    private int selectedLocation = -1;
    private Unit selectedUnit = null;
    private Person selectedTech = getSelectedTech();
+    private Person tempTech = getTempTech();


[nitpick] Initializing a field by calling its own getter during field initialization is confusing and unnecessary; it will just assign the default null via an instance method before construction completes. Prefer a plain declaration: private Person tempTech; and let it be managed via the getter/setter.

Suggested change

private Person tempTech = getTempTech();

private Person tempTech;

Copilot · 2025-10-19T01:39:15Z

MekHQ/src/mekhq/gui/ITechWorkPanel.java

    IPartWork getSelectedTask();

    Person getSelectedTech();
+
+    Person getTempTech();
+
+    void setTempTech(Person tempTech);
 }


New API methods lack Javadoc/comments describing intended semantics (e.g., when tempTech should be set/cleared, thread-safety expectations, and that the cached tech must be compatible with the current part/unit). Please add brief Javadoc to guide implementers.

Suggested change

IPartWork getSelectedTask();

Person getSelectedTech();

Person getTempTech();

void setTempTech(Person tempTech);

}

/**

* Returns the currently selected task for tech work.

* Implementers should ensure the returned task is compatible with the current part/unit.

*

* @return the selected IPartWork task, or null if none is selected

*/

IPartWork getSelectedTask();

/**

* Returns the currently selected technician for the task.

* The returned Person should be valid for the current part/unit.

*

* @return the selected Person as technician, or null if none is selected

*/

Person getSelectedTech();

/**

* Returns the temporary technician assigned to the current task.

* This is typically used for previewing or staging a tech assignment before confirmation.

* Implementers should clear tempTech when the assignment is finalized or cancelled.

* Thread-safety: If this panel is accessed from multiple threads, ensure proper synchronization.

*

* @return the temporary Person assigned as technician, or null if none is set

*/

Person getTempTech();

/**

* Sets the temporary technician for the current task.

* Use this to stage a tech assignment before confirmation.

* The provided Person must be compatible with the current part/unit.

* Implementers should clear tempTech when the assignment is finalized or cancelled.

* Thread-safety: If this panel is accessed from multiple threads, ensure proper synchronization.

*

* @param tempTech the Person to set as temporary technician, or null to clear

*/

void setTempTech(Person tempTech);

Unless the setter or getter does anything special it doesn't need to be javadoc. Even I don't javadoc setters and getters and I javadoc everything.

Don't waste your time on this PR, doesn't quite work right anyway ;) I'll un-draft it / ask for input when it's less bad. But something's gotta be done, it looks like it just constantly recalculates values when you're sitting on the tab. Something I'll keep messing with...

That actually matches what I was finding last time I was in this corner of the code. It's not just an issue restricted to here.

The campaign summary section, on the command center tab (the bit where it shows your unit rating, etc). Used to be constantly refreshing itself. I think I changed that to only refresh on new day. But, obviously, that won't work here.

TaskTableModel optimizations

b7636b9

psikomonkie commented Oct 18, 2025

View reviewed changes

HammerGS requested a review from Copilot October 19, 2025 01:37

Copilot AI reviewed Oct 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`TaskTableModel` optimizations #7756

`TaskTableModel` optimizations #7756

Uh oh!

psikomonkie commented Oct 18, 2025

Uh oh!

psikomonkie Oct 18, 2025

Uh oh!

codecov bot commented Oct 18, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 19, 2025

Uh oh!

Copilot AI Oct 19, 2025

Uh oh!

Copilot AI Oct 19, 2025

Uh oh!

Copilot AI Oct 19, 2025

Uh oh!

IllianiBird Oct 19, 2025

Uh oh!

psikomonkie Oct 19, 2025

Uh oh!

IllianiBird Oct 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	table.setRowHeight(newRowHeight);
	table.setRowHeight(row, newRowHeight);

-                        tech = panel.getTempTech();
+                        Person cachedTech = panel.getTempTech();
+                        if (cachedTech != null && part.getUnit() != null && cachedTech.canTech(part.getUnit().getEntity())) {
+                            tech = cachedTech;
+                        }

	private Person tempTech = getTempTech();
	private Person tempTech;

-    IPartWork getSelectedTask();
-    Person getSelectedTech();
-    Person getTempTech();
-    void setTempTech(Person tempTech);
-}
+    /**
+     * Returns the currently selected task for tech work.
+     * Implementers should ensure the returned task is compatible with the current part/unit.
+     *
+     * @return the selected IPartWork task, or null if none is selected
+     */
+    IPartWork getSelectedTask();
+    /**
+     * Returns the currently selected technician for the task.
+     * The returned Person should be valid for the current part/unit.
+     *
+     * @return the selected Person as technician, or null if none is selected
+     */
+    Person getSelectedTech();
+    /**
+     * Returns the temporary technician assigned to the current task.
+     * This is typically used for previewing or staging a tech assignment before confirmation.
+     * Implementers should clear tempTech when the assignment is finalized or cancelled.
+     * Thread-safety: If this panel is accessed from multiple threads, ensure proper synchronization.
+     *
+     * @return the temporary Person assigned as technician, or null if none is set
+     */
+    Person getTempTech();
+    /**
+     * Sets the temporary technician for the current task.
+     * Use this to stage a tech assignment before confirmation.
+     * The provided Person must be compatible with the current part/unit.
+     * Implementers should clear tempTech when the assignment is finalized or cancelled.
+     * Thread-safety: If this panel is accessed from multiple threads, ensure proper synchronization.
+     *
+     * @param tempTech the Person to set as temporary technician, or null to clear
+     */
+    void setTempTech(Person tempTech);

TaskTableModel optimizations #7756

Are you sure you want to change the base?

TaskTableModel optimizations #7756

Uh oh!

Conversation

psikomonkie commented Oct 18, 2025

Uh oh!

psikomonkie Oct 18, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

IllianiBird Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

psikomonkie Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

IllianiBird Oct 19, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

`TaskTableModel` optimizations #7756

`TaskTableModel` optimizations #7756

codecov bot commented Oct 18, 2025 •

edited

Loading