Skip to content

Use ELK for player save content, restrict node count#474

Merged
tudddorrr merged 1 commit intodevelopfrom
elk-save-content
Feb 23, 2026
Merged

Use ELK for player save content, restrict node count#474
tudddorrr merged 1 commit intodevelopfrom
elk-save-content

Conversation

@tudddorrr
Copy link
Collaborator

No description provided.

@tudddorrr tudddorrr added the enhancement New feature or request label Feb 23, 2026
@claude
Copy link

claude bot commented Feb 23, 2026

PR Review: Use ELK for player save content, restrict node count


Potential Bugs

Stale async layout results (race condition)

Both useNodeGraph and useLinearNodeGraph fire async ELK layout calls with no cancellation in the effect cleanup. If debouncedSearch or debouncedNodeSizes changes while a layout is in-flight, the older call's .then() can resolve after the newer call, overwriting state with stale data and setting committedSearch to the wrong value.

Fix with a cancellation flag:

let cancelled = false
void getLayoutedElements(nodeSet, edgeSet, debouncedNodeSizes).then((layoutedNodes) => {
  if (\!cancelled) {
    setNodes(layoutedNodes)
    setEdges(edgeArray)
    setCommittedSearch(debouncedSearch)
  }
})
return () => { cancelled = true }

Minor Issues

renderArrayRow regex fails for keys containing spaces

The regex /^(\S+)\s+(\[[^\]]+\])/ only captures up to the first whitespace as the key name. If a JSON key contains a space (e.g. "player stats"), the label "player stats [5]" won't match — name falls back to the full raw string and count is '', losing the indigo-colored count display. Parsing from the bracket backward would be more robust:

const bracketIndex = item.lastIndexOf(' [')
const name = bracketIndex \!== -1 ? item.slice(0, bracketIndex) : item

Non-null assertion on matchingNodeCount

In PlayerSaveContent.tsx, matchingNodeCount\!.toLocaleString() uses \! even though the value is typed as number | null. The surrounding ternary makes the null case unreachable at runtime today, but the assertion is fragile against future refactors. Prefer: (matchingNodeCount ?? 0).toLocaleString().

@tudddorrr tudddorrr merged commit e378779 into develop Feb 23, 2026
5 checks passed
@tudddorrr tudddorrr deleted the elk-save-content branch February 23, 2026 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant