fix: allow replacing stale entries left after crash

[standmit]

Summary by CodeRabbit

• Bug Fixes
  • Improved duplicate-apartment handling: duplicates are now allowed when they refer to the same on-panel location (enabling in-place updates) and only flagged as conflicts when an apartment with the same properties exists at a different position.

[coderabbitai]

Walkthrough

Adds a position-aware duplicate check in gui.lua: when storing an apartment, existing entries with the same category and description are only treated as conflicts if their 3D position differs from the panel's position; same-position entries are allowed (treated as the same apartment).

Changes

Cohort / File(s)	Summary
Apartment position validation gui.lua	Modified duplicate-apartment detection to require both matching category/description and differing 3D position before raising an error. Added/used a position equality check to allow updates when an existing apartment is at the same coordinates.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant UI as Panel(UI)
    participant G as gui.lua
    participant S as Store

    UI->>G: submit apartment(category, descr, pos)
    G->>G: find existing by category+descr
    alt existing found
        G->>G: compare existing.pos with pos
        alt positions equal
            G->>S: proceed (treat as same apartment) 
            S-->>G: success
            G-->>UI: success/update
        else positions differ
            G-->>UI: error "duplicate apartment at different position"
        end
    else no existing
        G->>S: create new apartment
        S-->>G: success
        G-->>UI: success
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Check correctness of the 3D position comparison (x, y, z) and floating-point tolerance if applicable
• Verify the duplicate-check logic is applied consistently in all store/update code paths
• Confirm no other validations are skipped by the early-return or branch changes

Poem

🐰 I hop where coordinates align,
Same-home checks now pass just fine,
If category and description meet,
And positions match beneath my feet,
No duplicate cry — just update time! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "fix: allow replacing stale entries left after crash" is directly related to the main change in the changeset. The modification adds a position-aware guard that allows duplicate apartment entries when they refer to the same on-panel location, effectively enabling updates or replacements of existing entries at that location. This matches the intent described in the title of allowing stale entries (entries that may have become outdated or duplicated after a crash) to be replaced. The title is concise, uses clear and specific language, and accurately conveys the primary purpose of the change without unnecessary verbosity or vague terminology.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09a8dc0 and d37f843.

📒 Files selected for processing (1)

• gui.lua (1 hunks)

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

gui.lua (1)

5-7: Consider using Minetest's built-in vector.equals().

The implementation is correct, but Minetest core provides vector.equals(pos1, pos2) for position comparison. Using the built-in function would be more idiomatic and maintainable.

Apply this diff to use the built-in function:

-local function isEqual(pos1, pos2)
-	return (pos1.x == pos2.x) and (pos1.y == pos2.y) and (pos1.z == pos2.z)
-end

Then update line 130:

-						if apartment.apartments[category] and apartment.apartments[category][descr] and (not isEqual(apartment.apartments[category][descr].pos, pos)) then
+						if apartment.apartments[category] and apartment.apartments[category][descr] and (not vector.equals(apartment.apartments[category][descr].pos, pos)) then

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ba28ea3 and 09a8dc0.

📒 Files selected for processing (1)

• gui.lua (2 hunks)

🔇 Additional comments (1)

gui.lua (1)

130-136: The code correctly handles stale entry replacement.

The concern is unfounded. apartment.rent() is always called from gui.lua with oldmetadata = nil (line 148), which ensures the apartment.apartments[category][descr] table entry is updated (init.lua line 178). The position-based check correctly prevents duplicates at different positions while allowing same-position reconfiguration, enabling proper stale entry recovery.