From 203c537e53350e33f548772e8842933a93b192b8 Mon Sep 17 00:00:00 2001 From: DanIsOk Date: Thu, 16 Apr 2026 15:02:06 -0400 Subject: [PATCH 1/5] fix offset maps --- .../controllers/SimulationController.java | 41 ++++++++----------- 1 file changed, 17 insertions(+), 24 deletions(-) diff --git a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java index 653103d..aff6011 100644 --- a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java +++ b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java @@ -485,22 +485,15 @@ private void initialize() { System.out.println("Error saving map to database: " + e.getMessage()); } - // Expand the canvas just enough to fit loaded entities while respecting the setup minimum. - if (!loadedMap.getEntities().isEmpty()) { - int maxTileX = 0, maxTileY = 0; - for (MapEntity entity : loadedMap.getEntities()) { - maxTileX = Math.max(maxTileX, (int) entity.getPosition().getX()); - maxTileY = Math.max(maxTileY, (int) entity.getPosition().getY()); - } - int entitySpanX = maxTileX + 1; - int entitySpanY = maxTileY + 1; - canvasWidthTiles = Math.max(AppState.getCanvasWidthTiles(), entitySpanX + 2); - canvasHeightTiles = Math.max(AppState.getCanvasHeightTiles(), entitySpanY + 2); - entityOffsetTileX = (canvasWidthTiles - entitySpanX) / 2; - entityOffsetTileY = (canvasHeightTiles - entitySpanY) / 2; - } - if (canvasSizeLabel != null) + // buildBuiltinMapInCanvas and loaded configs already sit at their correct absolute tile positions within the map. Adding a centering offset on top + // Use map dimensions directly and set offsets to zero. + canvasWidthTiles = loadedMap.getWidth(); + canvasHeightTiles = loadedMap.getHeight(); + entityOffsetTileX = 0; + entityOffsetTileY = 0; + if (canvasSizeLabel != null) { canvasSizeLabel.setText("Canvas Size: " + canvasWidthTiles + "×" + canvasHeightTiles + " Tiles"); + } refreshIntersectionObjectTileVisibility(); populateOutliner(); @@ -1614,8 +1607,8 @@ private void renderRackDropoffArray(Rack rack, VBox container) { Label headerLabel = new Label("Valid Dropoff Points"); headerLabel.setStyle("-fx-font-weight: bold;"); Tooltip headerTip = new Tooltip( - "Boxes are distributed across the valid dropoff points using round-robin. " + - "Null slots are ignored. At least one non-null slot is required to play."); + "Boxes are distributed across the valid dropoff points using round-robin. " + + "Null slots are ignored. At least one non-null slot is required to play."); Tooltip.install(headerLabel, headerTip); Button addBtn = new Button("+"); addBtn.setOnAction(ev -> { @@ -1652,7 +1645,7 @@ private void renderRackDropoffArray(Rack rack, VBox container) { DeliveryStation ds = stationById.get(id); display = ds == null ? "(deleted station)" : ds.getName() + " (" + (int)ds.getPosition().getX() - + ", " + (int)ds.getPosition().getY() + ")"; + + ", " + (int)ds.getPosition().getY() + ")"; } Label displayLabel = new Label(display); if (id == null || stationById.get(id) == null) { @@ -1681,7 +1674,7 @@ private void beginDropoffPicking(Rack rack, int slotIndex) { if (tipLabel != null) { // Reuse the main viewport click handler for the actual station assignment. tipLabel.setText("TIP: Click a delivery station to assign it to slot #" - + slotIndex + ". Click elsewhere to cancel."); + + slotIndex + ". Click elsewhere to cancel."); } viewportStack.setCursor(javafx.scene.Cursor.CROSSHAIR); drawViewport(); @@ -1899,7 +1892,7 @@ private void onPlay() { for (MapEntity me : engine.getMap().getEntities()) { if (me instanceof Rack r && r.isManualDropoffAssignment()) { boolean hasValid = r.getValidDropoffIds().stream() - .anyMatch(uid -> uid != null && stationIds.contains(uid)); + .anyMatch(uid -> uid != null && stationIds.contains(uid)); if (!hasValid) offenders.add(r.getName()); } } @@ -1917,9 +1910,9 @@ private void onPlay() { alert.setTitle("Cannot start simulation"); alert.setHeaderText("Manual-mode racks have no valid dropoff points"); alert.setContentText( - "The following racks use Manual Dropoff Assignment but their pool is empty or all-null:\n\n \u2022 " - + String.join("\n \u2022 ", offenders) - + "\n\nAssign at least one delivery station per rack, or disable Manual Dropoff Assignment."); + "The following racks use Manual Dropoff Assignment but their pool is empty or all-null:\n\n \u2022 " + + String.join("\n \u2022 ", offenders) + + "\n\nAssign at least one delivery station per rack, or disable Manual Dropoff Assignment."); alert.showAndWait(); log("\u26a0 Play aborted: " + offenders.size() + " rack(s) have an empty manual dropoff pool."); return; @@ -2614,4 +2607,4 @@ public void shutdown() { Logger.flushRobotEvents(); } -} +} \ No newline at end of file From a002da53423438bbd2a5ed2c6aba690a51815d7e Mon Sep 17 00:00:00 2001 From: DanIsOk Date: Thu, 16 Apr 2026 16:57:34 -0400 Subject: [PATCH 2/5] impl data validation in setup screen --- .../controllers/SetupController.java | 137 +++++++++++++++++- 1 file changed, 130 insertions(+), 7 deletions(-) diff --git a/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java b/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java index 2920358..19f0bec 100644 --- a/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java +++ b/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java @@ -205,6 +205,17 @@ private void initialize() { } }); + // Apply input restrictions (prevent negative signs and letters) + makeIntegerOnly(maxTasksField); + makeIntegerOnly(maxTicksField); + makeIntegerOnly(workloadSeedField); + makeIntegerOnly(randomSeedField); + makeFloatOnly(batteryCapacityField); + makeFloatOnly(lowBatteryField); + makeFloatOnly(chargePerTickField); + makeFloatOnly(energyPerMoveField); + makeSpinnerIntegerOnly(reservationKSpinner); + String initialMap = mapCombo.getValue(); if (initialMap != null) autoSetCanvasForMap(initialMap); } @@ -954,21 +965,133 @@ private float parseFloatSafe(String text, float fallback) { // Validation // Validates the minimum required inputs before starting the simulation. + // ------------------------------------------------------------------ // + // Validation + // ------------------------------------------------------------------ // + private boolean validate() { - // Loaded configs intentionally clear the map combo, so skip that check once an engine exists. + // Map Check if (!AppState.hasEngine() && mapCombo.getValue() == null) { - statusLabel.setText("\u26a0 Please select a map or load a config file."); + statusLabel.setText("⚠ Please select a map or load a config file."); return false; } + + if ("RESERVATION_K".equals(policyCombo.getValue())) { + if (!checkSpinnerBound(reservationKSpinner, "Reservation K", 1, 50)) return false; + } + + // Integer Bounds + if (!checkIntBound(maxTasksField, "Max Tasks", 1, 500)) return false; + if (!checkIntBound(maxTicksField, "Max Ticks", 1, 1000000)) return false; + + // Float Bounds + if (batteryCapacityField != null && batteryCapacityField.getScene() != null) { + if (!checkFloatBound(batteryCapacityField, "Battery Capacity", 1.0f, 1000.0f)) return false; + if (!checkFloatBound(lowBatteryField, "Low Battery", 0.0f, 1000.0f)) return false; + if (!checkFloatBound(chargePerTickField, "Charge Per Tick", 0.0f, 100.0f)) return false; + if (!checkFloatBound(energyPerMoveField, "Energy Per Move", 0.0f, 100.0f)) return false; + + float cap = Float.parseFloat(batteryCapacityField.getText().trim()); + float low = Float.parseFloat(lowBatteryField.getText().trim()); + if (low >= cap) { + statusLabel.setText("⚠ Low battery threshold must be lower than total capacity."); + return false; + } + } + + statusLabel.setText(""); // Clear errors + return true; + } + + private boolean checkIntBound(TextField field, String fieldName, int min, int max) { + if (field == null) return true; try { - int ticks = Integer.parseInt(maxTicksField.getText().trim()); - if (ticks <= 0) throw new NumberFormatException(); + int value = Integer.parseInt(field.getText().trim()); + if (value < min || value > max) { + statusLabel.setText("⚠ " + fieldName + " must be between " + min + " and " + max + "."); + return false; + } + return true; } catch (NumberFormatException e) { - statusLabel.setText("\u26a0 Max ticks must be a positive integer."); + statusLabel.setText("⚠ " + fieldName + " requires a valid number."); return false; } - statusLabel.setText(""); - return true; + } + + private boolean checkFloatBound(TextField field, String fieldName, float min, float max) { + if (field == null) return true; + try { + float value = Float.parseFloat(field.getText().trim()); + if (value < min || value > max) { + statusLabel.setText("⚠ " + fieldName + " must be between " + min + " and " + max + "."); + return false; + } + return true; + } catch (NumberFormatException e) { + statusLabel.setText("⚠ " + fieldName + " requires a valid number."); + return false; + } + } + + private boolean checkSpinnerBound(Spinner spinner, String fieldName, int min, int max) { + if (spinner == null || spinner.isDisabled()) return true; + try { + // Check the editor text because Spinner.getValue() might not have changed yet + String text = spinner.getEditor().getText().trim(); + if (text.isEmpty()) { + statusLabel.setText("⚠ " + fieldName + " cannot be empty."); + return false; + } + + int value = Integer.parseInt(text); + if (value < min || value > max) { + statusLabel.setText("⚠ " + fieldName + " must be between " + min + " and " + max + "."); + return false; + } + + // Force the spinner to adopt the typed value internally + spinner.getValueFactory().setValue(value); + return true; + } catch (NumberFormatException e) { + statusLabel.setText("⚠ " + fieldName + " requires a valid number."); + return false; + } + } + + /** Restricts a TextField to only accept positive integers */ + private void makeIntegerOnly(TextField field) { + if (field == null) return; + field.setTextFormatter(new TextFormatter<>(change -> { + // Allows empty string (while deleting) or digits only + if (change.getControlNewText().matches("\\d*")) { + return change; + } + return null; + })); + } + + /** Restricts a TextField to accept positive floats (decimals) */ + private void makeFloatOnly(TextField field) { + if (field == null) return; + field.setTextFormatter(new TextFormatter<>(change -> { + // Allows empty, digits, or digits with a single decimal point + if (change.getControlNewText().matches("\\d*(\\.\\d*)?")) { + return change; + } + return null; + })); + } + + /** Restricts an editable Spinner to only accept positive integers */ + private void makeSpinnerIntegerOnly(Spinner spinner) { + if (spinner == null || !spinner.isEditable()) return; + TextField editor = spinner.getEditor(); + editor.setTextFormatter(new TextFormatter<>(change -> { + if (change.getControlNewText().matches("\\d*")) { + return change; + } + return null; // Reject the keystroke + })); } // Navigation From de63c9294dcec238a55cab297e1f8db3db2accbf Mon Sep 17 00:00:00 2001 From: DanIsOk Date: Thu, 16 Apr 2026 17:23:42 -0400 Subject: [PATCH 3/5] fix robot battery property validation --- open-robotics/configs/..valid.json | 6 ++-- .../controllers/SimulationController.java | 32 ++++++++++++++++--- .../controllers/SetupControllerTest.java | 26 ++++++++++++--- .../controllers/SimulationControllerTest.java | 2 +- 4 files changed, 54 insertions(+), 12 deletions(-) diff --git a/open-robotics/configs/..valid.json b/open-robotics/configs/..valid.json index 2c5928f..ca7b53f 100644 --- a/open-robotics/configs/..valid.json +++ b/open-robotics/configs/..valid.json @@ -1,6 +1,6 @@ { "config" : { - "runId" : "641949f5-3dbe-43c8-ad7b-cfeb1b222f18", + "runId" : "ebc72ee6-96dc-4bb3-a878-6f8e7df4cf02", "runName" : "default_run", "tickMs" : 100, "maxTicks" : 5000, @@ -15,14 +15,14 @@ "manualTaskAssignment" : false }, "map" : { - "mapId" : "5b307bf9-e784-4863-8250-7367bf3d6560", + "mapId" : "4ef3d4b7-a31b-4aab-8064-b16b7de126b6", "width" : 8, "height" : 8, "tiles" : [ ] }, "entities" : { "robots" : [ { - "id" : "cef66328-ce04-4d1c-8217-57ab1a8c4f93", + "id" : "bb6e0a02-c19d-4200-9c79-8c2f0feb95e3", "name" : "R1", "position" : { "x" : 1, diff --git a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java index aff6011..c1c5252 100644 --- a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java +++ b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java @@ -1525,21 +1525,45 @@ private void showPropertiesFor(MapEntity entity) { HBox batteryBox = new HBox(8); batteryBox.setAlignment(javafx.geometry.Pos.CENTER_LEFT); float maxBattery = robot.getConfig().batteryCapacity; + Spinner batterySpinner = new Spinner<>( - new SpinnerValueFactory.DoubleSpinnerValueFactory(0, maxBattery, robot.getBattery(), 1.0)); + new SpinnerValueFactory.DoubleSpinnerValueFactory(0.0, (double) maxBattery, (double) robot.getBattery(), 1.0)); batterySpinner.setPrefWidth(90); batterySpinner.setEditable(true); + + // Restrict input to positive digits and a single decimal point + batterySpinner.getEditor().setTextFormatter(new TextFormatter<>(change -> { + String newText = change.getControlNewText(); + if (newText.matches("\\d*(\\.\\d*)?")) { + return change; + } + return null; // Reject the change + })); + + // Force spinner to commit text value when the user clicks away + batterySpinner.getEditor().focusedProperty().addListener((obs, wasFocused, isFocused) -> { + if (!isFocused) { + batterySpinner.increment(0); + } + }); + batterySpinner.valueProperty().addListener((obs, oldVal, newVal) -> { if (newVal != null && oldVal != null && !newVal.equals(oldVal)) { if (guardEditor("change battery")) { - batterySpinner.getValueFactory().setValue(oldVal); + // Revert spinner if editor is locked + Platform.runLater(() -> batterySpinner.getValueFactory().setValue(oldVal)); return; } - robot.setBattery(newVal.floatValue()); + + float newBat = newVal.floatValue(); + float oldBat = oldVal.floatValue(); + + robot.setBattery(newBat); drawViewport(); - pushAction(new BatteryChangeAction(robot, oldVal.floatValue(), newVal.floatValue())); + pushAction(new BatteryChangeAction(robot, oldBat, newBat)); } }); + batteryBox.getChildren().addAll(new Label("Battery:"), batterySpinner); propertiesPanel.getChildren().add(batteryBox); diff --git a/open-robotics/src/test/java/com/openrobotics/controllers/SetupControllerTest.java b/open-robotics/src/test/java/com/openrobotics/controllers/SetupControllerTest.java index e1da651..1bf230d 100644 --- a/open-robotics/src/test/java/com/openrobotics/controllers/SetupControllerTest.java +++ b/open-robotics/src/test/java/com/openrobotics/controllers/SetupControllerTest.java @@ -271,19 +271,37 @@ void validate_rejects_missing_map_only_when_no_engine_is_loaded() { */ @Test void validate_rejects_bad_max_tick_values_and_accepts_positive_integer() { + // Select a map so the validation doesn't fail on the Map check early + interact(() -> { + ComboBox maps = lookup("#mapCombo").queryComboBox(); + maps.getSelectionModel().selectFirst(); + }); + TextField maxTicks = textField("maxTicksField"); - interact(() -> maxTicks.setText("abc")); + // Test non-numeric characters + interact(() -> { + maxTicks.clear(); + maxTicks.setText("abc"); + }); + assertEquals("", maxTicks.getText()); assertFalse((boolean) invokePrivate("validate", new Class[0])); - assertEquals("⚠ Max ticks must be a positive integer.", statusLabel().getText()); + assertEquals("\u26a0 Max Ticks requires a valid number.", statusLabel().getText()); + // Test zero interact(() -> maxTicks.setText("0")); assertFalse((boolean) invokePrivate("validate", new Class[0])); + assertEquals("\u26a0 Max Ticks must be between 1 and 1000000.", statusLabel().getText()); - interact(() -> maxTicks.setText("-5")); + // Test negative + interact(() -> { + maxTicks.clear(); + maxTicks.setText("-5"); + }); assertFalse((boolean) invokePrivate("validate", new Class[0])); - interact(() -> maxTicks.setText("1")); + // Test valid positive integer + interact(() -> maxTicks.setText("30000")); assertTrue((boolean) invokePrivate("validate", new Class[0])); assertEquals("", statusLabel().getText()); } diff --git a/open-robotics/src/test/java/com/openrobotics/controllers/SimulationControllerTest.java b/open-robotics/src/test/java/com/openrobotics/controllers/SimulationControllerTest.java index 61846de..a66fd15 100644 --- a/open-robotics/src/test/java/com/openrobotics/controllers/SimulationControllerTest.java +++ b/open-robotics/src/test/java/com/openrobotics/controllers/SimulationControllerTest.java @@ -185,7 +185,7 @@ void loading_engine_populates_canvas_outliner_stats_and_console() { assertAll( () -> assertSame(fixture.engine, AppState.getEngine()), - () -> assertEquals("Canvas Size: 6×5 Tiles", field("canvasSizeLabel", Label.class).getText()), + () -> assertEquals("Canvas Size: 5×5 Tiles", field("canvasSizeLabel", Label.class).getText()), () -> assertEquals("Loaded 5 objects", field("viewportStatusLabel", Label.class).getText()), () -> assertEquals("Objects: 6", field("objsLabel", Label.class).getText()), () -> assertEquals(6, outliner.getItems().size()), From c4c73c4fb54245005c294bcaf57ac09184369035 Mon Sep 17 00:00:00 2001 From: DanIsOk Date: Thu, 16 Apr 2026 17:36:19 -0400 Subject: [PATCH 4/5] fix comments --- .../java/com/openrobotics/controllers/SetupController.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java b/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java index 19f0bec..e3c5aca 100644 --- a/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java +++ b/open-robotics/src/main/java/com/openrobotics/controllers/SetupController.java @@ -964,11 +964,8 @@ private float parseFloatSafe(String text, float fallback) { } // Validation - // Validates the minimum required inputs before starting the simulation. - // ------------------------------------------------------------------ // - // Validation - // ------------------------------------------------------------------ // + // Validates the minimum required inputs before starting the simulation. private boolean validate() { // Map Check if (!AppState.hasEngine() && mapCombo.getValue() == null) { From fbcc6833feb214d18912d873e7f48c0e05484008 Mon Sep 17 00:00:00 2001 From: DanIsOk Date: Thu, 16 Apr 2026 17:57:34 -0400 Subject: [PATCH 5/5] fix new icon for delivery station --- .../controllers/SimulationController.java | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java index f6c7697..002109f 100644 --- a/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java +++ b/open-robotics/src/main/java/com/openrobotics/controllers/SimulationController.java @@ -1818,11 +1818,12 @@ private void populateOutliner() { List newBacking = new ArrayList<>(); // Keep entity rows first so editor selections stay predictable. for (MapEntity e : engine.getMap().getEntities()) { - String icon = (e instanceof Robot) ? "\ud83e\udd16 " // robot - : (e instanceof Rack) ? "\ud83d\udce6 " // rack/shelf - : (e instanceof Station) ? "\u26a1 " // station - : (e instanceof Obstacle) ? "\ud83e\uddf1 " // wall/obstacle - : "\u25ab "; // generic entity + String icon = (e instanceof Robot) ? "\ud83e\udd16 " // robot + : (e instanceof Rack) ? "\ud83d\udce6 " // rack/shelf + : (e instanceof ChargingStation) ? "\u26a1 " // charging station + : (e instanceof DeliveryStation) ? "\uD83C\uDFC1 " // delivery station + : (e instanceof Obstacle) ? "\ud83e\uddf1 " // wall/obstacle + : "\u25ab "; // generic entity String entry = icon + e.getName() + " " + e.getPosition(); if (filter.isEmpty() || entry.toLowerCase().contains(filter)) { newItems.add(entry); @@ -1855,11 +1856,6 @@ private void populateOutliner() { } } - @FXML - private void onResetStringProp() { - if (strPropField != null) strPropField.setText("Hello"); - } - // Playback Controls @FXML private void onPlay() {