Add map notes with alignment and sharp text rendering

[alcatron]

Add an Add Note editor action and a dialog for multiline text, left/centre/right alignment, standard colours with a custom picker, fonts, move, clone and delete. New notes default to black Vera Sans at 9 pt and are saved as label-only map nodes with editor hints.

Render note glyphs at twice the density while retaining logical dimensions, coordinates and clickable bounds. Preserve foreground ordering and composite furniture once. Allocate both new canvases before suppressing normal glyphs; if either allocation fails, retain the logical-resolution map and readable note text. Missing or unreadable fonts fall back to a built-in font.

Save configurations through checked atomic replacement under a destination-directory mutex shared with registration and deletion. Compare the exact read revision before replacement so stale edits and repairs cannot overwrite newer content. Create-only writes protect new and cloned map filenames. Missing targets and save failures return errors with neutral retry guidance. Regression tests exercise concurrent creators/readers, stale repairs, raw fragment edits, both canvas allocation failures and process-isolated upgrade stubs.

Validation: affected branch PHP 8.3 and browser regressions pass in isolated Cacti fixtures. The combined changes pass 291 PHP tests (5784 assertions) and 15 browser regression programs. Changed-line coverage passes at 100% of measured production lines; existing controller/entry-point exemptions remain. Syntax, whitespace, translation and manifest checks pass. These revisions have not been deployed to the live installation.

Based on current develop, including the merged icon picker and gallery resize. Shared editor files overlap with the other feature PRs.

[Copilot]

🟡 Changes recommended

Note deletion can silently remove connected links, and save/render consistency issues remain.

6 open findings

• Route note deletion through guarded connected-link confirmation · New
• Propagate configuration write and close failures · New
• Add newly created notes to the render layer index · New
• Add browser regression coverage for the new note UI · New
• Add the standard GPL v2 header to the new test · New
• Use tabs for PHP test method indentation · New

What changed in this PR

Adds editable map notes and high-density text rendering while preserving logical map dimensions.

Changes:

• Adds note creation, styling, movement, cloning, deletion, and validation.
• Renders eligible note text at 2× resolution.
• Adds localization and engine regression coverage.

File	Description
weathermap-cacti-plugin-editor.php	Adds note controls and dialog.
js/​editor.js	Implements note editor interactions.
lib/​editor.notes.php	Validates and persists note data.
lib/​editor.actions.php	Adds note save endpoint and metadata.
lib/​editor.inc.php	Registers note helpers and translations.
lib/​WeatherMap.notes.php	Implements high-density note rendering.
lib/​WeatherMap.class.php	Integrates note composition and thumbnail scaling.
lib/​WeatherMapNode.class.php	Adds multiline alignment and rendering state.
tests/​Unit/​EditorNotesEngineTest.php	Wraps the engine regression test.
tests/​Support/​EditorNotesEngineRegression.php	Tests note persistence, rendering, and validation.
locales/​po/​cacti.pot	Adds translatable note strings.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The built-in live view displays high-density PNGs at twice their logical dimensions, and palette labels are not extractable for translation.

2 open findings

• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

4 resolved since last review

• Route note deletion through guarded connected-link confirmation
• Add newly created notes to the render layer index
• Use tabs for PHP test method indentation
• Add the standard GPL v2 header to the new test

Previously missed (4)

In code that hasn't changed since last review

Constrain live-view images to logical map dimensions

lib/​WeatherMap.class.php:4186

The high-density output changes the PNG's intrinsic size, but the built-in live-view caller renders ?action=liveviewimage without width/height attributes (weathermap-cacti-plugin.php:206). A 600×300 map with a note is therefore displayed at 1200×600 in live view, contrary to the logical-size guarantee. Update that rendering path to constrain the image to the map's logical dimensions (and add a regression for it).

Document function parameters and return value in PHPDoc

lib/​WeatherMap.notes.php:29

This new function's PHPDoc does not follow the repository-required shape because its parameter and return value are undocumented. Add typed @param and @return entries.

This issue also appears on line 60 of the same file.

Indent else-block statement consistently

lib/​WeatherMapNode.class.php:803

The statement inside this else block is not indented one level deeper, unlike the surrounding PHP control structures. Add one tab so the branch structure remains clear.

Use literal gettext calls for translatable palette labels

weathermap-cacti-plugin-editor.php:390

Passing $label dynamically to __esc() prevents gettext extraction. The regenerated POT contains none of these eight plain colour names, so palette labels cannot be translated. Use literal gettext calls for each array value and regenerate locales/po/cacti.pot.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

High-density rendering incorrectly excludes notes beneath non-overlapping higher-layer nodes, and note deletion displays an inaccurate confirmation.

2 open findings

• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

Previously missed (2)

In code that hasn't changed since last review

Use note-specific confirmation for deleting notes

js/​editor.js:587

The note-specific wmEditorText.noteDelete message added by this change is never used. Calling delete_node() shows the generic node confirmation and appends “The Cacti device and graphs will remain,” which is inaccurate for a label-only note. Use a note-aware confirmation (while still warning about any connected links) and omit the device message.

Scan all layers to preserve non-overlapping sharp notes

lib/​WeatherMap.notes.php:43

This only scans the single highest real z-layer. For example, a note at the default layer 600 plus a non-overlapping node at layer 700 starts with an empty $notes list and returns no sharp notes, so the note loses high-density rendering even though the later node does not overlap it. This also contradicts the stated behavior that later nodes block only annotations they overlap. Traverse the sorted layers in draw order so eligible notes are collected before later nodes are checked for overlap, while retaining the conservative handling for links and overlapping objects.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

High-density composition can reorder higher map layers, and localized interfaces may select the wrong default font.

3 open findings

• Reapplying furniture incorrectly changes foreground z-order · New
• Propagate configuration write and close failures
• Add browser regression coverage for the new note UI

Previously missed (6)

In code that hasn't changed since last review

Select bundled fonts using stable identifiers, not localized labels

js/​editor.js:1748

This default-font lookup depends on the English option label. When Vera 9 is already registered, wm_style_font_options() uses its numeric font ID instead of the bundled: value; in a locale that translates the label, neither condition matches and the note silently defaults to font 3 rather than Vera Sans 9 pt. Expose and select a stable bundled-font identifier (for example, a data attribute) instead of matching localized display text.

Use cacti_count() instead of raw count()

lib/​WeatherMap.notes.php:85

Use the repository's cacti_count() wrapper for this new production code instead of raw count(), as required by the plugin's compatibility convention.

Use cacti_count() instead of raw count()

lib/​WeatherMapNode.class.php:371

New or edited Cacti plugin code should use the repository's count wrappers rather than raw count(). Replace this with cacti_count($noteLines) to follow the plugin-wide compatibility convention.

Add required PHPDoc to named helper

tests/​Support/​EditorNotesEngineRegression.php:34

This new named helper is missing the repository-required PHPDoc shape (description, @param entries, and @return). Add a block documenting the assertion value, message, and void return before the function.

This issue also appears on line 152 of the same file.

Use cacti_count() in test code

tests/​Support/​EditorNotesEngineRegression.php:119

Use cacti_count() rather than raw count() in this new test code as well, matching the repository's required Cacti compatibility wrapper convention.

Add required PHPDoc to test method

tests/​Unit/​EditorNotesEngineTest.php:31

The new test method is missing the PHPDoc block required for every PHP function/method in this repository. Add a one-line description and @return void while retaining the process-isolation attributes.

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Layer composition can break z-ordering, and move/delete failures remain silent.

9 open findings

• Reapplying furniture incorrectly changes foreground z-order
• Propagate configuration write and close failures
• Note moves ignore failed configuration writes · New
• Note deletions silently ignore configuration write failures · New
• Refresh only initialized selectmenu widgets · New
• Replace raw count() with cacti_count() · New
• Use cacti_count() in production code · New
• Use cacti_count() in added PHP code · New
• Add browser regression coverage for the new note UI

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Rendering order, repeated compositing, font defaults, and non-atomic failed writes can produce incorrect output or data loss.

12 open findings

• Direct truncating writes can destroy map configuration on failure · New
• Reapplying furniture incorrectly changes foreground z-order
• Propagate configuration write and close failures
• Font option deduplication breaks Vera Sans 9 default selection · New
• Furniture bitmap is composited twice, darkening antialiased edges · New
• Refresh only initialized selectmenu widgets
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Use cacti_count() in added PHP code
• Use cacti_count() in production code
• Replace raw count() with cacti_count()
• Add browser regression coverage for the new note UI

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

High-density rendering composites map furniture twice, and missing note fonts can disrupt editor JavaScript generation.

6 open findings

• Propagate configuration write and close failures
• Fallback when note font definition is unavailable · New
• Font option deduplication breaks Vera Sans 9 default selection
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Add browser regression coverage for the new note UI

7 resolved since last review

• Direct truncating writes can destroy map configuration on failure
• Reapplying furniture incorrectly changes foreground z-order
• Furniture bitmap is composited twice, darkening antialiased edges
• Refresh only initialized selectmenu widgets
• Use cacti_count() in added PHP code
• Use cacti_count() in production code
• Replace raw count() with cacti_count()

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

The new locking scheme can race with configuration deletion and introduces an unrelated plugin-directory dependency for generic config writes.

6 open findings

• Use a shared directory lock for config writes and deletion · New
• Propagate configuration write and close failures
• Derive config locks from destination paths · New
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Add browser regression coverage for the new note UI

2 resolved since last review

• Fallback when note font definition is unavailable
• Font option deduplication breaks Vera Sans 9 default selection

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

Missing-target actions can still report success, and unreadable configured fonts do not fall back as described.

9 open findings

• Use a shared directory lock for config writes and deletion
• Propagate configuration write and close failures
• Return an error when the move target is missing · New
• Fall back when the TrueType font file is unreadable · New
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Document atomic persistence and locking changes · New
• Use relevant messages for move and delete failures · New
• Add browser regression coverage for the new note UI

1 resolved since last review

• Derive config locks from destination paths

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

The combined rendering, filesystem-locking, and atomic-write changes warrant final human integration review.

9 open findings

• Use a shared directory lock for config writes and deletion
• Propagate configuration write and close failures
• Fall back when the TrueType font file is unreadable
• Return an error when the move target is missing
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Use relevant messages for move and delete failures
• Document atomic persistence and locking changes
• Add browser regression coverage for the new note UI

🧠 Review effort: Balanced

[Copilot]

🟡 Changes recommended

High-density allocation failures can abort rendering, and concurrent creation can overwrite a newly created map.

8 open findings

• Prevent concurrent creates from overwriting newly committed maps · New
• Handle high-density allocation failure before image resampling · New
• Use a shared directory lock for config writes and deletion
• Propagate configuration write and close failures
• Note deletions silently ignore configuration write failures
• Note moves ignore failed configuration writes
• Use relevant messages for move and delete failures
• Add browser regression coverage for the new note UI

3 resolved since last review

• Fall back when the TrueType font file is unreadable
• Return an error when the move target is missing
• Document atomic persistence and locking changes

🧠 Review effort: Balanced