Charm implementation#3
Merged
Merged
Conversation
LocalIdentity
approved these changes
Dec 14, 2024
Wires77
pushed a commit
that referenced
this pull request
Jan 19, 2025
Fix Deep Cuts, Deadly Poison and ailment source damage
Soroand
pushed a commit
to rocky-soft/PathOfBuilding-PoE2
that referenced
this pull request
May 15, 2026
Six fixes against the bootstrap JSON-RPC server, all flagged Important by the code reviewer; PathOfBuildingCommunity#4 is a real crash bug. PathOfBuildingCommunity#1 Batch requests (JSON-RPC 2.0 §6): dispatcher.handle now detects array shape at the top and returns -32600 with a clear scope-out message ("batch requests not supported in this server (Phase 1.3a scope)"). Phase 1.3b decides whether to implement batch. Documented in PORT_PLAN.md §4.2. PathOfBuildingCommunity#2 Startup banner moved from stdout to stderr. Emitting a synthetic JSON-RPC response with id=null on stdout violated §5 (id=null is reserved for parse/invalid-request errors). stdout is now reserved for RPC traffic exclusively; the readiness signal joins normal diagnostic logging on stderr. Documented in PORT_PLAN.md §4.1. PathOfBuildingCommunity#3 Dispatcher error-envelope shape: defensive coercion when a handler returns (nil, "bare string") or (nil, nil). Both now produce a spec-conformant {code, message} object instead of a bare string on the wire. PathOfBuildingCommunity#4 (crash bug) rpc/tree.lua:54 nil-deref on ensure_build failure. The convention was inconsistent: ensure_build returned (false, app_error()) but app_error itself already returned (nil, err_table), so the outer tuple was (false, nil) and `err.error` nil-dereffed. Refactored ensure_build to return nil on success / err_table on failure; callers now do `local err = ensure_build(); if err then return nil, err end`. New failing-path test in headless_spec.lua verifies the dispatcher surfaces this as a well-formed JSON-RPC error envelope, not a crash. PathOfBuildingCommunity#5 Removed dead `if os.getenv("CI") == nil then ... end` block from entrypoint.lua — that's Phase 1.3b's Dockerfile concern. PathOfBuildingCommunity#6 Changed `arg = {}` to `_G.arg = arg or {}` in entrypoint.lua — preserves a real arg if the orchestrator ever passes flags, and makes the global-write explicit (matches the file's otherwise- scrupulous use of locals). Plus housekeeping: - tests/README.md: documented stat-value regression, batch/notifications, and ImportFromNodeList allocation-assertion as Phase 1.3b TODOs. - PORT_PLAN.md §1.2: pinned PR #9505 head SHA (948baf20...) so the design-reference doesn't go stale if upstream force-pushes. - PORT_PLAN.md §2.3: clarified the 8 ms / 14 ms numbers are calc-only; full RPC round-trip latency is pending Phase 1.4 measurement. 10/10 busted specs pass under Alpine 3.20 + LuaJIT 2.1 + luautf8 0.1.6-1.
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
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.
Adds Charms to Items.
Belts set the Charm Limit through ("Charm Slots: #") per https://poe2db.tw/Belt