Skip to content

Translate script values back to NBT when an operator expects NBT - #70

Closed
rubensworks wants to merge 1 commit into
master-26-ltsfrom
claude/issue-67-nbt-operator-arguments
Closed

Translate script values back to NBT when an operator expects NBT#70
rubensworks wants to merge 1 commit into
master-26-ltsfrom
claude/issue-67-nbt-operator-arguments

Conversation

@rubensworks

Copy link
Copy Markdown
Member

Fixes the first half of #67: values that came out of an NBT operator could not be passed into another one.

Problem

Only compound tags keep their NBT type when they are translated to a script (they become an NbtCompoundTagProxyObject). Every other tag becomes a plain script value: a list tag or array tag becomes a JS array, primitives become numbers and strings, and an absent NBT value becomes null.

When such a value is passed to an operator again, it is translated back to whatever value type it looks like, so an NBT input receives a list or a number, and OperatorBase.validateTypes rejects it:

const matched = idContext.ops.stringNbtPathMatchFirst('["levels"][?(@ >= 3)]', nbt);
idContext.ops.nbtAsTagList(matched); // operator.integrateddynamics.error.wrong_type [as_tag_list, list, 1, nbt]

A filter expression always matches into a list tag, so this hits every script that post-processes an NBT path match, which is what the reporter ran into. null was worse: no translator handles it, so it failed with valuetype.integratedscripting.error.translation.unknown_from_graal.

Fix

OperatorProxyExecutable now translates each argument against the value type the operator declares for it (CurriedOperator reports the remaining input types, so methods on value proxies work the same way). For an NBT input, a script value that is not NBT is converted with the existing translateToNbt path, and null becomes an absent NBT value. Arguments of every other type keep going through the same translation as before.

Tests

NbtOperatorArgumentsJavaScriptTests covers the list tag round trip, a plain script array, null, and, as controls, a compound tag and a non-NBT argument. Verified that the three NBT tests fail on master-26-lts without the fix:

NbtOperatorArgumentsJavaScriptTests > testListTagMatchPassedBackToNbtOperator() FAILED
    ...error.wrong_type, args=[as_tag_list, ...list..., 1, ...nbt...]
NbtOperatorArgumentsJavaScriptTests > testArrayPassedToNbtOperator() FAILED
    ...error.wrong_type, args=[as_tag_list, ...list..., 1, ...nbt...]
NbtOperatorArgumentsJavaScriptTests > testNullPassedToNbtOperator() FAILED
    ...error.translation.unknown_from_graal, args=[null]
5 tests completed, 3 failed

./gradlew spotlessApply build passes with the fix. I could not run runGameTestServer in this environment.

Note

The reporter's original call, nbtAsIntList on a filter match, no longer errors but still yields an empty list. That part is on the Integrated Dynamics side: ValueTypeListProxyNbtAsListInt only accepts IntArrayTag, and ValueTypeListProxyNbtAsListGeneric swallows the ClassCastException and reports length 0, so a list tag silently yields nothing. nbtAsTagList is the operator that works for these. Worth a separate look there.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JCW1HmWhLT27jh57t9d7B6


Generated by Claude Code

Only compound tags keep their type when they are translated to a script:
all other tags become plain script values, and an absent NBT value becomes
null. Passing such a value to an operator again therefore failed, because
it was translated back to the value type it looked like. A list tag that
came out of an NBT path filter expression came back as a list, so
nbtAsTagList(idContext.ops.stringNbtPathMatchFirst(...)) failed with a
wrong type error instead of listing the matches, and null values did not
translate at all.

Operator arguments are now translated against the value type the operator
expects for them, so script values reaching an NBT input are converted to
NBT, and null becomes an absent NBT value.

Related to #67

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JCW1HmWhLT27jh57t9d7B6
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33627544083

Coverage increased (+0.3%) to 49.913%

Details

  • Coverage increased (+0.3%) from the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • 4 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

4 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
org/cyclops/integratedscripting/evaluate/translation/translator/OperatorProxyExecutable.java 4 85.71%

Coverage Stats

Coverage Status
Relevant Lines: 2582
Covered Lines: 1412
Line Coverage: 54.69%
Relevant Branches: 852
Covered Branches: 302
Branch Coverage: 35.45%
Branches in Coverage %: Yes
Coverage Strength: 19.92 hits per line

💛 - Coveralls

Copy link
Copy Markdown
Member Author

Superseded by #73, which carries the same change against master-1.20-lts so it can be upmerged. The affected code is identical on 1.20, 1.21 and 26, so nothing is lost by taking it there instead. Closing this one.


Generated by Claude Code

@rubensworks rubensworks closed this Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants