Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor how map updates are propagated to widgets #1174

Merged
merged 10 commits into from
Aug 9, 2022

Conversation

lmoureaux
Copy link
Contributor

Map changes (e.g. units moving, terrain changing, etc.) are propagated to the map view and the minimap using static functions in mapview_common.cpp. These functions had to (magically) know which widgets were waiting for updates and send them as appropriate. This made the updates hard to track, because the widget code on its own didn't "subscribe" to any notification.

Change this to use the listener<> template. listener<> is meant to handle the case of C-like static code calling into C++-like object-oriented code in a way that respects encapsulation, so it's natural to use it here.

After the change, mapview functions called upon updates no longer need to call into the minimap directly because the minimap receives the updates directly (thus we get separation of purposes).

On top of #1169.

@lmoureaux lmoureaux added refactoring This issue requires code refactoring gui This issue requires changes to the user interface labels Jul 30, 2022
@lmoureaux lmoureaux requested a review from psampathkumar July 30, 2022 22:02
This class will be used to implement the functionality of
refresh_tile|unit|city_mapcanvas and the like. It uses listener<> to provide a
bridge between the old C-style API and the newer object-oriented code.

So far, the class can accumulate a list of updates and restitute them.
Eventually it should be able to notify when a new frame is needed.
This way any updates_handler instance will be populated (indefinitely because
clear() is never called, but so far we have no instance...).
We'll have the updates go through an update handler so we can prove that it
works properly. And then we'll try to migrate stuff away from mapview_common.
It was easy to port. Other updates will come next.
Needed to rewrite unqueue_mapview_updates.
Gone are the old static variables.
Emit a signal whenever an update is ready, and connect it to
unqueue_mapview_updates. Use a queued connection so a few updates can
accumulate before they are processed in the next step of the event loop. This
allows us to remove the "idle callback" completely (a QueuedConnection is
basically the same thing, only in a more elegant way).
Use a second map_updates_handler for minimap (overview) updates. This is mostly
formal, but makes for more modular and better encapsulated code.
It isn't up to the caller to determine when the update should be performed.
We'll always do it at the next opportunity, i.e. whenever the queued connection
fires.
@lmoureaux lmoureaux force-pushed the refactor/map-updates branch from f08b919 to 3e5d61c Compare August 5, 2022 23:51
@lmoureaux lmoureaux marked this pull request as ready for review August 5, 2022 23:51
@lmoureaux
Copy link
Contributor Author

Rebased

@psampathkumar psampathkumar merged commit 55660c2 into longturn:master Aug 9, 2022
@psampathkumar psampathkumar deleted the refactor/map-updates branch August 9, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gui This issue requires changes to the user interface refactoring This issue requires code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants