Skip to content

Fix sidebar wreck field icon using wrong planet Space Dock level#1495

Merged
Geda173 merged 2 commits into
lanedirt:mainfrom
tiweb442:fix/1397-sidebar-wreck-field-icon
Jul 21, 2026
Merged

Fix sidebar wreck field icon using wrong planet Space Dock level#1495
Geda173 merged 2 commits into
lanedirt:mainfrom
tiweb442:fix/1397-sidebar-wreck-field-icon

Conversation

@tiweb442

Copy link
Copy Markdown
Contributor

Summary

In the sidebar planet list, the wreck field icon used the currently selected planet's Space Dock level instead of the iterated planet's.

Fixes #1397

Test plan

  • Two planets: wreck on A with Space Dock → icon on A when B is selected
  • Wreck on A without Space Dock → no icon on A even if B has Space Dock

Related: tiweb442 #1491, #1492, #1494

Made with Cursor

The planet list loop checked the currently selected planet's Space
Dock level instead of the iterated planet's, hiding or showing the
wreck field icon incorrectly for multi-planet accounts. Fixes lanedirt#1397

Co-authored-by: Cursor <cursoragent@cursor.com>

@Geda173 Geda173 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix looks right. $planet is the @foreach variable from line 1730, and the two lines either side of this one (getPlanetCoordinates() on 1828, isMoon() on 1839) already use it, so this was the odd one out.

Could we get a feature test with it though? This bug is invisible to any single-planet test, because with one planet $planet and planets->current() are the same object. That's why the existing wreck field coverage didn't catch it: 32 tests across FacilitiesWreckFieldTest and WreckFieldTest all pass both with and without this fix. Nothing currently asserts on the sidebar either.

Assert the planet list icon uses each listed planet's Space Dock
level, which single-planet wreck field tests cannot catch.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tiweb442

Copy link
Copy Markdown
Contributor Author

@Geda173 Good call — added SidebarWreckFieldIconTest with two multi-planet cases:

  1. Wreck + Space Dock on A, B selected without dock → icon on A
  2. Wreck on A without dock, B selected with dock → no icon on A

Confirmed both fail against the old planets->current() check and pass with this fix.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Preview deployment

Status URL Commit
❌ Destroyed pr-1495.preview.ogamex.dev -

Preview environment has been cleaned up.

@Geda173

Geda173 commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thank you, merging.

@Geda173
Geda173 merged commit 7318532 into lanedirt:main Jul 21, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Sidebar wreck field icon checks the wrong planet's Space Dock level

2 participants