pjbroad
commented
5 months ago @lukehorvat its likely far too late to talk about this patch, sorry about that, but I've just taken another look. When I looked at this way back when you submitted it, I was not sure that I agreed with all of it. Then I stopped working on the client for a while and forgot about it. Second time round, its seams like a good compromise. The only thing I'd consider adding is to clear and current input text, not just hide the yellow cross; but I'm not sure. Anyhow, I'm merging the PR now so thanks for providing it and sorry it was left unanswered.
lukehorvat
Hi all. My first patch for the EL client. (Thanks Ben for helping me with building the client on my Mac)
This PR proposes fixes for 3 small bugs in the map window. The fixes are a bit opinionated, so please let me know if you think they should be solved in different ways.
Bug 1
If you start adding a mark and then move to a different map, the temporary (yellow) mark will still show on the new map:
https://user-images.githubusercontent.com/1034878/201548449-46cd687e-069c-4c69-b068-78771606abd6.mp4
This feels a bit buggy to me, because the mark (coords + label) was only relevant to the previous map. It doesn't make much sense for the temporary mark to persist over to the next map imo.
This PR fixes it by clearing the temporary mark on map change:
https://user-images.githubusercontent.com/1034878/201548445-d9a4d5c7-2488-430c-912b-948d30b7f5fc.mp4
Bug 2
If you start adding a mark and then select a different map via the map window, the temporary mark will still show on the new map. But if you then press Enter to save the mark, it will inexplicably disappear. You have to go back to the map where you originally started adding the mark to see that it was actually saved there:
https://user-images.githubusercontent.com/1034878/201548454-19ed68de-1c15-4b1f-a817-bf2cabfd5e7c.mp4
Again, kinda buggy and not intuitive. Considering you can't even right-click on maps your character is not currently on, you should not even be able to see the temporary mark on the new map, let alone save it.
This PR fixes it by clearing the temporary mark when clicking the small map in the map window:
https://user-images.githubusercontent.com/1034878/201548452-654b4756-41a8-408a-9968-0bd0ec36f91b.mp4
Bug 3
If you select a different map via the map window, and then have your character actually move to that same map, the map window does not respond to interaction like you'd expect it to:
https://user-images.githubusercontent.com/1034878/201548461-58d006a3-0caf-4943-b4a1-41d5a9f26436.mp4
This is because the map window goes into a non-interactive state the moment you select a different map, even if your character later moves to that map. You'd have to clear the map selection first by double-clicking the small map to bring back interactivity. Annoying.
This PR fixes it by clearing the map window's currently-selected map on map change:
https://user-images.githubusercontent.com/1034878/201548457-ca6dd73f-f14c-47ab-b8f6-604a87ce31aa.mp4
This fix feels like a bit of a nuclear option, however I think/hope most players will intuitively understand that moving to a different map "resets" the map window...