Cleanup: remove private header #95
Conversation
This header was copied in from MapLibre Native to expose a private method. Previously this got out of sync and caused a bug: maplibre#53 Instead we can use the already public delegate method. Note: The deleted header was importing Maplibre/Mapbox.h, but we are using that elsewhere, so I moved that import up a level.
michaelkirk
force-pushed
the
mkirk/remove-private-header
branch
from
ec993cc to
5aa57b5
Compare
There was a problem hiding this comment.
Thanks for looking into this.
If you tested this and everything still works, I am fine with this change. Maybe we give for example @Patrick-Kladek and @hactar a chance to check as well, but other than that, I am fine with merging 👍
|
I'm hoping this PR solves #60 as well any thoughts? |
| override open func mapViewDidFinishRenderingFrameFullyRendered(_ fullyRendered: Bool, frameEncodingTime: Double, frameRenderingTime: Double) { | ||
| super.mapViewDidFinishRenderingFrameFullyRendered(fullyRendered, frameEncodingTime: frameEncodingTime, frameRenderingTime: frameRenderingTime) | ||
|
|
||
| func updateCourseTrackingAfterDidFinishRenderingFrame() { |
There was a problem hiding this comment.
What's the reason for removing the override and then calling this from elsewhere? For me the override function is more logical
There was a problem hiding this comment.
It's so that we don't accidentally break the puck animation again, like what I fixed in #53.
I tried to explain the reasoning in this issue description — the method we were overriding was a private method. I patched it up in #53, but because it's still a private method it could break again. #53 has some more details which might be helpful for context.
There was a problem hiding this comment.
Got it, thanks for linking the PR, helps a lot to understand.
Description
This header was copied in from MapLibre Native to expose a private
method.
Previously this got out of sync and caused a bug:
#53
Instead we can use the already public delegate method.
Note: The deleted header was importing Maplibre/Mapbox.h, but we are
using that elsewhere, so I moved that import up a level.
Open Tasks
Related:
maplibre/maplibre-native#2228
#53
Infos for Reviewer
There should be no change in behavior from this — just cleanup.