Skip to content

Fix GridRenderer hygiene#6000

Open
kwvanderlinde wants to merge 6 commits into
RPTools:developfrom
kwvanderlinde:bugfix/5581-grid-renderer
Open

Fix GridRenderer hygiene#6000
kwvanderlinde wants to merge 6 commits into
RPTools:developfrom
kwvanderlinde:bugfix/5581-grid-renderer

Conversation

@kwvanderlinde

@kwvanderlinde kwvanderlinde commented Jul 4, 2026

Copy link
Copy Markdown
Collaborator

Identify the Bug or Feature request

Fixes issue with PR #5581
Progresses #3691
Fixes #5758

Description of the Change

Fixes a bug where modifying a grid in one zone forces all other zones to start rendering the modified zone's grid. This is done by not reassigning GridRenderer#zone in response to GridChanged events.

GridRenderer no longer uses static state to track changing variables. This avoids the need to invoke GridRenderer and Grid methods in specific orders just to get the state to be correct.

In order to allow gridColours to not be static anymore, the GridRenderer no directly draws the grid and coordinates rather than delegating to Grid#draw() (which would then delegate back to GridRenderer). Lots of unused code was removed from Grid classes, some related to this change some that was just sitting around.

Possible Drawbacks

Should be none.

Documentation Notes

N/A

Release Notes

N/A


This change is Reviewable

@kwvanderlinde kwvanderlinde self-assigned this Jul 4, 2026
@kwvanderlinde kwvanderlinde moved this from Todo to Awaiting-Review in MapTool 1.19 Jul 4, 2026
@Baaaaaz

Baaaaaz commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Sounds like this will also fix: #5758

@kwvanderlinde

Copy link
Copy Markdown
Collaborator Author

Sounds like this will also fix: #5758

Thanks @Baaaaaz, I somehow missed that issue when I searched. This PR does indeed fix that issue.

Aside from `#gridColours`, static state is no longer used `GridRenderer`. This prevents one `GridRenderer` for one
`Zone` from affecting another `GridRenderer` for another `Zone`.

In `#onGridChanged()`, do not reassign `#zone`, but only action the event if the zone matches. Before this, changing a
grid on one zone would result in the GridRenderer state being invalid for its original zone.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/5581-grid-renderer branch from 5e67bcf to 3acecf9 Compare July 6, 2026 14:25
`Grid#draw()` and `Grid#drawCoordinatesOverlay()` are no longer used and have been removed (along with much other unused
Grid functionality).
It is not costly to recreate the colours. This avoids the event bus in the renderer and means there is no chance of
keeping the wrong state around.
@kwvanderlinde kwvanderlinde force-pushed the bugfix/5581-grid-renderer branch from 3acecf9 to cc367b8 Compare July 6, 2026 14:38

@bubblobill bubblobill 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.

Apart from my personal dislike for final classes, this all appears sensible.

@github-project-automation github-project-automation Bot moved this from Awaiting-Review to To-Be-Merged in MapTool 1.19 Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: To-Be-Merged

Development

Successfully merging this pull request may close these issues.

[Bug]: Wrong map grids are sometimes shown

3 participants