search

thebuildcraft / RemotePlayerWaypointsForXaero

Shows player and marker positions from Dynmap, Bluemap, Squaremap or Pl3xMap in game using Xaeros Minimap and Worldmap. Also displays AFK status in the tab list.
GNU General Public License v3.0
10 stars 3 forks source link

[Feature Request] Import waypoints other than players. #17

Closed eatmyvenom closed 1 month ago

eatmyvenom commented 1 month ago

The idea is simple enough: when the map has markers available also put them into the world. An example of this is BlueMap markers, if they were also included in the Xaero waypoint list that would be an excellent feature. It most likely makes sense for this to be a config option.

Implementation seems relatively straight forward (see eatmyvenom/RemoteWaypointsForXaero@4eee80833de1238fa3037854e1b55c7b8408c51d). If you would like I can pull request my branch into this repo. I would do the full implementation but I don't have the ability to do any real testing aside from bluemap, however I can get most of them started if you would like.

thebuildcraft commented 1 month ago

Hi. Thanks for the suggestion and implementation. I just had a look at it, and it seems good, apart from some details I would like to change. For example:

You can make a pull request and will change those few details, add support for the other online maps and merge it later.

eatmyvenom commented 1 month ago

For sure, I'll push what I have so far and PR.

thebuildcraft commented 1 month ago

Thank you. I really appreciate your work. As I already said in the PR, I will merge it tomorrow. And once I have time for it, I will add a config option and implement it for Squaremap and Pl3xMap.

thebuildcraft commented 1 month ago

@eatmyvenom As you might have seen, I finished the implementation (except for the alternative Dynmap methods...). You may also have noticed that I changed quite a lot of your code. That's because it didn't work for all the maps I looked at. Could you please check if it still works with the maps you tested it with? I just realized that you might have a version of the Bluemap / Dynmap I have never seen before and that needs special handling for the markers.

eatmyvenom commented 1 month ago

Just tested it right now, works great! I don't know if you want to leave this issue till the other maps are implemented fully or not so I will leave this up to you to decide, thank you for merging.

eatmyvenom commented 1 month ago

Edit: this appears to be the wrong version of the mod, I am rebuilding and testing from upstream

Actually I found an issue when switching worlds. This causes the timer thread to crash and never restart. Here is my log:

[20:29:07] [Timer-2/ERROR]: Uncaught exception in thread "Timer-2"
java.lang.NullPointerException: Cannot invoke "xaero.common.XaeroMinimapSession.getWaypointsManager()" because the return value of "xaero.common.XaeroMinimapSession.getCurrentSession()" is null
    at de.the_build_craft.remote_player_waypoints_for_xaero.UpdateTask.run(UpdateTask.java:173) ~[remote_waypoints_for_xaero-fabric-2.0.1.jar:?]
    at java.util.TimerThread.mainLoop(Unknown Source) ~[?:?]
    at java.util.TimerThread.run(Unknown Source) ~[?:?]

I didn't notice this prior to this patch but it doesn't seem to be related, if you would like I can create a new issue to track this.

thebuildcraft commented 1 month ago

Hi, thanks for testing. You can close this issue now. The only thing that is not implemented yet is the debug method for dynmap. I don't think I will implement it because the user would have to enter all the marker links individually. For that newly found error. You can open an issue if you want to track it, but I will fix it today as soon as possible. And it should be an easy fix...