NPCTag.can_navigate_to Tag & NPCTag Modernization#2811
Conversation
| list.add(anchor.getName()); | ||
| } | ||
| return list; | ||
| return new ListTag(object.getCitizen().getOrAddTrait(Anchors.class).getAnchors(), anchor -> new ElementTag(anchor.getName())); |
There was a problem hiding this comment.
these can be plain elements
| } | ||
| tagProcessor.registerTag(LocationTag.class, ElementTag.class, "pose", (attribute, object, param) -> { | ||
| Pose pose = object.getCitizen().getOrAddTrait(Poses.class).getPose(param.asString()); | ||
| return new LocationTag(org.bukkit.Bukkit.getWorlds().get(0), 0, 0, 0, pose.getYaw(), pose.getPitch()); |
There was a problem hiding this comment.
Unless there's some specific input conflict, can probably clean this FQN up
| // Returns whether an NPC can navigate to a specified location. | ||
| // --> | ||
| tagProcessor.registerTag(ElementTag.class, LocationTag.class, "can_navigate_to", (attribute, object, param) -> { | ||
| return new ElementTag(object.npc.getNavigator().canNavigateTo(param)); |
There was a problem hiding this comment.
Probably #getCitizen like the rest of the tags/mechs?
| } | ||
| else { | ||
| mechanism.echoError("Must set waypoint_provider to 'wander' before setting wander_xrange!"); | ||
| if (mechanism.requireInteger()) { |
There was a problem hiding this comment.
Why undo the early return? Usually less nesting is nicer to read and follow
| else { | ||
| BukkitImplDeprecations.assignmentRemove.warn(mechanism.context); | ||
| object.getCitizen().getOrAddTrait(AssignmentTrait.class).clearAssignments(null); | ||
| object.npc.removeTrait(AssignmentTrait.class); |
There was a problem hiding this comment.
Maybe do #getCitizen for these ones as well, so that they're not randomly different from themselves/the rest of the tags/mechs
| HologramTrait hologram = object.getCitizen().getOrAddTrait(HologramTrait.class); | ||
| hologram.setLineHeight(input.asDouble()); |
| } | ||
| } | ||
| tagProcessor.registerMechanism("owner", false, PlayerTag.class, (object, mechanism, input) -> { | ||
| object.getCitizen().getOrAddTrait(Owner.class).setOwner(input.getPlayerEntity()); |
There was a problem hiding this comment.
What about the alternative String behavior this had? Are we sure it's safe to remove?
| Material mat = item.getMaterial().getMaterial(); | ||
| Entity npcEntity = getEntity(); | ||
| tagProcessor.registerMechanism("item_type", false, ItemTag.class, (object, mechanism, input) -> { | ||
| Material mat = input.getMaterial().getMaterial(); |
| if (mechanism.requireBoolean()) { | ||
| if (!object.getCitizen().hasTrait(SneakingTrait.class)) { | ||
| object.getCitizen().addTrait(SneakingTrait.class); | ||
| } |
There was a problem hiding this comment.
Early return would be nicer here, also that hasTrait -> addTrait is just legacy code from before getOrAddTrait I believe, so can be removed.
| final LocationTag loc = mechanism.valueAsType(LocationTag.class); | ||
| getNavigator().getLocalParameters().lookAtFunction((n) -> loc); | ||
| object.getNavigator().getLocalParameters().lookAtFunction((n) -> mechanism.valueAsType(LocationTag.class)); |
There was a problem hiding this comment.
This would have it parse the location repeatedly every time the lookAt function is called
| if (input.asBoolean()) { | ||
| object.getNavigator().getDefaultParameters().stuckAction(TeleportStuckAction.INSTANCE); | ||
| } | ||
| else { | ||
| object.getNavigator().getDefaultParameters().stuckAction(null); | ||
| } |
| } | ||
| tagProcessor.registerTag(LocationTag.class, ElementTag.class, "pose", (attribute, object, param) -> { | ||
| Pose pose = object.getCitizen().getOrAddTrait(Poses.class).getPose(param.asString()); | ||
| return new LocationTag(org.bukkit.Bukkit.getWorlds().get(0), 0, 0, 0, pose.getYaw(), pose.getPitch()); |
| } | ||
| else { | ||
| mechanism.echoError("Must set waypoint_provider to 'wander' before setting wander_yrange!"); | ||
| if (mechanism.requireInteger()) { |
| } | ||
| } | ||
| tagProcessor.registerMechanism("owner", false, (object, mechanism) -> { | ||
| object.getCitizen().getOrAddTrait(Owner.class).setOwner(mechanism.getValue().canBeType(PlayerTag.class) ? mechanism.valueAsType(PlayerTag.class).getPlayerEntity() : null); |
There was a problem hiding this comment.
It looks like the owner can also be an OfflinePlayer looking at Citizens? No reason to limit it to getPlayerEntity here if that's the case.
| } | ||
| SneakingTrait trait = getCitizen().getOrAddTrait(SneakingTrait.class); | ||
| SneakingTrait trait = object.getCitizen().getOrAddTrait(SneakingTrait.class); | ||
| if (trait.isSneaking() && !mechanism.getValue().asBoolean()) { |
| // @input ElementTag | ||
| // @description | ||
| // Sets the NPC's nickname. | ||
| // Sets the NPC's nickname. Leave blank to remove the nickname. |
There was a problem hiding this comment.
Honestly on second thought I'm not sure if I like this, if the entire mechanism was just being added sure, but seeing as this is already present in an existing feature (that isn't being deprecated or anything) adding duplicate functionality just for the sake of it seems a bit messy.
Not forbidding this if you really want it or anything, but just my opinion.
4 participants
NPCTag.can_navigate_to[<location>]tagtagProcessor