BME-171: Waystone Integration #91
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| package com.eerussianguy.blazemap.integration.waystones; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import net.minecraft.resources.ResourceLocation; | ||
| import net.minecraftforge.common.MinecraftForge; | ||
|
|
||
| import com.eerussianguy.blazemap.BlazeMap; | ||
| import com.eerussianguy.blazemap.api.markers.Waypoint; | ||
| import com.eerussianguy.blazemap.feature.waypoints.service.WaypointChannelLocal; | ||
| import com.eerussianguy.blazemap.feature.waypoints.service.WaypointGroup; | ||
| import com.eerussianguy.blazemap.feature.waypoints.service.WaypointServiceClient; | ||
| import com.eerussianguy.blazemap.integration.ModIDs; | ||
| import com.eerussianguy.blazemap.integration.ModIntegration; | ||
| import net.blay09.mods.waystones.api.IWaystone; | ||
| import net.blay09.mods.waystones.api.KnownWaystonesEvent; | ||
|
|
||
| public class WaystonesPlugin extends ModIntegration { | ||
| public static final ResourceLocation WAYSTONE = BlazeMap.resource("textures/waypoints/special/waystone.png"); | ||
| private static final int WHITE = 0xFFFFFFFF; | ||
|
|
||
| public WaystonesPlugin() { | ||
| super(ModIDs.WAYSTONES, ModIDs.BALM); | ||
| } | ||
|
|
||
| @Override | ||
| public void setup() { | ||
| MinecraftForge.EVENT_BUS.addListener(this::onKnownWaystonesEvent); | ||
| } | ||
|
|
||
| /** | ||
| * Event handler for adding waypoints for waystones. The event gets fired on the client | ||
| * every time a waystone is added/updated and contains a list of all the current waystones. | ||
| */ | ||
| private void onKnownWaystonesEvent(KnownWaystonesEvent event) { | ||
| var waypointClient = WaypointServiceClient.instance(); | ||
|
There was a problem hiding this comment. Please avoid using |
||
| var waystones = event.getWaystones(); | ||
|
|
||
| removeDeletedWaystoneWaypoints(waystones); | ||
|
|
||
| for(var waystone : waystones) { | ||
| var dimension = waystone.getDimension(); | ||
| var waypoint = new Waypoint( | ||
| BlazeMap.resource("waypoint/waystone/" + waystone.getWaystoneUid()), | ||
| dimension, | ||
| waystone.getPos(), | ||
| waystone.getName(), | ||
| WAYSTONE, | ||
| WHITE | ||
| ); | ||
| var waypointGroups = waypointClient.getPool(WaypointChannelLocal.PRIVATE_POOL).getGroups(dimension); | ||
| waypointGroups.stream() | ||
| .filter(g -> g.type == WaypointChannelLocal.GROUP_WAYSTONE) | ||
| .findFirst() | ||
| .ifPresentOrElse(g -> updateWaypoint(g, waypoint), () -> { | ||
|
There was a problem hiding this comment. Nit: While short to write, this isn't a particularly performant way of doing this as you're having to:
It also unnecessarily creates a new list, which adds to memory usage and so GC events. While not as fancy looking, it's better speed and memory efficient if you just did a normal for loop to check each element. That way, you can short circuit as soon as you've found a match, meaning fewer elements are checked in the average case, and you're not creating any unnecessary objects that'll need to be GCed later 😊 It's not a big deal here as the numbers we're talking about are very small. But it's a good habit to get into when writing high-performance code, such as code that runs in real time on the main game thread. |
||
| // Add the waystone group if it didn't exist before | ||
| // This is so it doesn't need to be a default group (if waystones isn't installed) and | ||
| // this is the easiest place to add it | ||
|
There was a problem hiding this comment. Do you need to do this check for every waystone present in the event, or can you do it just once before the outer loop an avoid duplicated work? |
||
| var group = WaypointGroup.make(WaypointChannelLocal.GROUP_WAYSTONE); | ||
| waypointGroups.add(group); | ||
| updateWaypoint(group, waypoint); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Helper to add or updates waypoints to a waypoint group. | ||
| * | ||
| * @param group The waypoint group to add to. | ||
| * @param newWaypoint The waypoint to add/update. | ||
| */ | ||
| private void updateWaypoint(WaypointGroup group, Waypoint newWaypoint) { | ||
|
There was a problem hiding this comment. This helper function might make more sense to put on the |
||
| if(!group.has(newWaypoint)) { | ||
| group.add(newWaypoint); | ||
| return; | ||
| } | ||
|
|
||
| // If it already exists update the existing waypoint instead | ||
| group.getAll().stream() | ||
| .filter(waypoint -> waypoint.getID().equals(newWaypoint.getID())) | ||
| .findFirst() | ||
| .ifPresent(waypoint -> { | ||
| waypoint.setName(newWaypoint.getName()); | ||
| // I don't know if these change can change but updating them in case it's possible to move waystones | ||
| waypoint.setDimension(newWaypoint.getDimension()); | ||
| waypoint.setPosition(newWaypoint.getPosition()); | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Removes waystone waypoints that are not in the list of known waystones. | ||
| * | ||
| * @param waystones The list of all known waystones. | ||
| */ | ||
| private void removeDeletedWaystoneWaypoints(List<IWaystone> waystones) { | ||
| var waypointClient = WaypointServiceClient.instance(); | ||
| var waystoneIds = waystones.stream().map(waystone -> waystone.getWaystoneUid().toString()).toList(); | ||
|
|
||
| Map<Waypoint, WaypointGroup> waypointsToRemove = new HashMap<>(); | ||
|
|
||
| waypointClient.getPool(WaypointChannelLocal.PRIVATE_POOL).iterateAll((waypoint, group) -> { | ||
| if(group.type != WaypointChannelLocal.GROUP_WAYSTONE) return; | ||
|
|
||
| var idParts = waypoint.getID().getPath().split("/"); | ||
| var id = idParts[idParts.length - 1]; // UUID of Waystone associated with waypoint | ||
| if(!waystoneIds.contains(id)) { | ||
| waypointsToRemove.put(waypoint, group); | ||
| } | ||
| }); | ||
|
|
||
| waypointsToRemove.forEach((waypoint, group) -> group.remove(waypoint)); | ||
|
There was a problem hiding this comment. Why put them in a map to delete rather than just delete them then and there? It's extra work you don't have to do and an extra data structure that needs to be GCed. If it's because that'd be changing a data structure in place, I believe there are iteration methods that are more forgiving for that. |
||
| } | ||
| } | ||
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong, but I think these should be
compileOnlynotimplementation, same as for Rubidium/Embeddium?(Same for the FTB dependencies above, but that's existing problems so up to you if you wanna fix for everything or just for your change)