Let Walk run until something stops it - #79
Open
Gaetarra wants to merge 5 commits into
Open
Conversation
Automation runs while the player is tabbed out or AFK, so a graph needs a way to reach someone who is not looking at the screen. Send Message only reaches chat, which nobody sees while a farm runs unattended. Alert has two modes. Play Sound takes any vanilla sound id plus a volume, and plays it through the sound manager with SimpleSoundInstance.forUI so the alert is not attenuated by where the player happens to be standing. Send Webhook POSTs the node's text to an endpoint configured in settings, which is what reaches a phone when nobody is at the machine. Non-obvious calls made here: - The node completes its future as soon as the notification is dispatched rather than awaiting the response. An alert must never stall the graph it is reporting on, so webhook failures are logged rather than propagated. - Webhook sends are rate limited to one per three seconds. An Alert placed inside Forever would otherwise hammer the endpoint, and Discord kills webhooks that are hammered. - Payload shape is chosen by host: Discord needs a JSON body, ntfy takes the raw text. This is a deliberate two-case heuristic rather than another mode. - The URL is validated once in SettingsManager.sanitizeWebhookUrl and https is required. It is a user-supplied string driving an outbound request from the game client, so it is checked at the boundary instead of at each call site. - Alert reuses the multi-line text fields that Send Message and Calculate already have, by joining NodeTextContent, so message text supports runtime variable interpolation with no new UI. - Toast was considered and left out: it only helps someone already watching the screen, which is not the case this node exists for. Not yet wired: the webhook URL has no settings-popup field, so it is set by editing pathmind/settings.json. That control lives in a Stonecutter source file and is left for a follow-up so this change stays reviewable. Verified: Fabric compiles on 26.2 and NeoForge on 26.1.2, and the generated mc26 source is byte-identical to the authored file, so no string transform was needed. The escaping and URL-validation logic was checked separately against the same inputs as NodeAlertTest. Not verified locally: :common:test and the 1.21.x targets, which need a JDK 21 toolchain; CI covers both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NodeMode has getModesForNodeType, not getAvailableModesForNodeType, so the assertion would not have compiled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Alert shipped unusable: it rendered only its message field, so the Sound and Volume parameters were invisible and there was no way to switch to Send Webhook. Its mode names also displayed as raw keys. Two causes, both missed because the editor has no automated coverage: - NodeGraph.rendersInlineParameters decides whether a node draws its parameter strip, and the mode selector is drawn inside that strip. It is true only for parameter nodes and for types tagged RENDER_INLINE_PARAMETERS. Alert was neither, so the strip that would have held both never rendered. - NodeMode.getDisplayName ignores the display strings in the enum constructor and builds "pathmind.node.mode.<name>" as a translation key. Without lang entries the modes rendered as pathmind.node.mode.alert_sound. Verified in a dev client: the mode selector opens, Sound and Volume are editable, and both modes read as English. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Walk could only run for a fixed duration or distance. "Walk until X" had to be built out of Repeat Until, which re-ran the node every iteration and released the forward key between them, so the player stuttered instead of walking, and a second Walk could not extend the first without ending it. Three changes: - WALK gains modes. Walk For is the existing timed/distance behaviour and stays the default. Start Walking holds forward and completes immediately, the way Sprint already does. Stop Walking releases it. Presets saved before this land with no mode, so they take the constructor default of WALK_FOR, whose parameters have the same names and defaults as the old type parameters and restore unchanged. - Both Walk parameter slots stop being hard requirements. A required slot fails the node outright at execution time (NodeExecutionCoordinator), which is why Walk demanded a Direction even though Look already aims the player, and why an unbounded walk was not expressible. Walk For with neither duration nor distance still completes as a no-op, unchanged. - WalkHold owns the forward key for Walk nodes and counts holds. Previously each Walk wrote client.options.keyUp directly, so any walk finishing released the key out from under every other walk still running. Stop-all releases the sustained hold explicitly, since by design it outlives the node that started it. Known limitation, called out in a comment on WalkHold: this arbitrates Walk against Walk only. NavigatorPrimitiveExecutor drives keyUp as a per-tick servo loop and still overwrites a hold while pathfinding is active. That conflict predates this change; making the navigator a hold participant is a larger patch and belongs on its own. Verified: Fabric compiles on 26.2 and NeoForge on 26.1.2, the mc26 source transform leaves WalkHold byte-identical, and the hold counter's semantics were checked standalone. Not verified locally: :common:test and the 1.21.x targets, which need a JDK 21 toolchain; CI covers both. The editor behaviour, in particular the mode selector on Walk and the now-optional slots, has not been clicked through in a dev client. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to the Walk modes, from testing the editor. - Walk no longer has a Direction slot. Look already aims the player, so the second way to do it only added a slot to fill. Presets saved with the old layout are migrated on load: duration/distance moves from slot 1 to slot 0, and whatever sat in the old slot 0 aimed the player and is dropped. The two orientPlayerTowardsRuntimeTarget calls go with it. - The Duration/Distance slot only exists in Walk For. Start and Stop Walking have nothing to time, and Node.getParameterSlotCount now returns 0 for them, so the slot is not drawn. - Walk is tagged RENDER_INLINE_PARAMETERS. Without it NodeGraph.rendersInline Parameters is false, the parameter strip is never drawn, and the mode selector that lives inside that strip is unreachable, which is what made the modes invisible when they were first added. - Mode display names get lang entries. NodeMode.getDisplayName ignores the strings in the enum constructor and builds "pathmind.node.mode.<name>" as a translation key, so the modes rendered as pathmind.node.mode.walk_for. Verified in a dev client: the selector opens, Start/Stop Walking show no slot, Walk For still takes a duration, and walking with an empty Direction works. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What This Changes
Walk could only run for a fixed duration or distance. "Walk until X" had to be
built out of
Repeat Until, which re-ran the node every iteration and releasedthe forward key between them, so the player stuttered instead of walking, and a
second Walk could not extend the first without ending it.
WALKgains modes.Walk Foris the existing timed/distance behaviourand stays the default.
Start Walkingholds forward and completesimmediately, the way
SPRINTalready does.Stop Walkingreleases it.WalkHoldowns the forward key and counts holds. Previously each Walkwrote
client.options.keyUpdirectly, so any walk finishing released the keyout from under every other walk still running.
Directionslot is removed.LOOKalready aims the player, andcarrying a second way to do it only added a slot to fill.
the node outright at execution time (
NodeExecutionCoordinator), which is whyan unbounded walk was not expressible at all.
Verification
./gradlew :common:test -Pmc_version=1.21.11WalkHoldTestis added but has never been executed; the hold counter's semantics were checked standalone instead.verifyCompatibilityManifest verifyCompatibilityStructure verifyBuildGenerationRouting1.21/1.21.8/1.21.10/1.21.1126.1/26.2(-p mc26, both loaders)26.1.2. NeoForge on26.2fails withCould not find net.neoforged:neoforge:unsupported— pre-existing, see #78.Start/Stop Walkingshow no slot,Walk Forstill takes a duration, walking with an empty Direction works.Notes For Review
Backwards compatibility. Two things change under saved presets, and both are
handled:
NodeGraphPersistenceskips
setModeand the node keeps the constructor default ofWALK_FOR,whose parameters have the same names and defaults as the old type parameters.
They restore unchanged.
Directionslot shifts duration/distance from slot 1 to slot 0.Attachments persist by explicit slot index, so old presets would silently lose
their duration node.
NodeGraphPersistenceremaps1 -> 0forWALKon loadand drops whatever was in the old slot 0, which could only have been a
direction. This is the part most worth a second pair of eyes.
Known limitation, deliberately not fixed here.
WalkHoldarbitrates Walkagainst Walk only.
NavigatorPrimitiveExecutordriveskeyUpas a per-tickservo loop at roughly fourteen sites and will still overwrite a hold while
pathfinding is active. That conflict predates this change — a Walk node running
alongside pathfinding already fought over the forward key — and making the
navigator a hold participant is a much larger patch. There is a comment on
WalkHoldsaying so.Semantics deliberately preserved.
Walk Forwith neither duration nordistance still completes as a successful no-op, unchanged, rather than becoming
an unbounded walk. Making empty mean "forever" would have changed the meaning of
existing saved presets silently; the explicit mode avoids that.
Stop-all releases the sustained hold explicitly, since by design it outlives the
node that started it.