Skip to content

Add NBTPath#of(Tag, Tag)#2

Open
Taskeren wants to merge 6 commits into
HMCL-dev:mainfrom
Taskeren:feature-better-path
Open

Add NBTPath#of(Tag, Tag)#2
Taskeren wants to merge 6 commits into
HMCL-dev:mainfrom
Taskeren:feature-better-path

Conversation

@Taskeren

@Taskeren Taskeren commented Jun 7, 2026

Copy link
Copy Markdown
Contributor

Added a util function to create NBTPath from existing tag trees.

by the way, the output of NBTPathImpl#toString is pretty weird. Fixed.

Comment thread src/main/java/org/glavo/nbt/NBTPath.java Outdated
@Taskeren Taskeren requested a review from Glavo June 9, 2026 14:06

@Glavo Glavo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本审查建议由 GPT-5 生成


已将具体审查建议标注在对应代码行。

@SuppressWarnings({"DataFlowIssue", "unchecked"})
@Contract(pure = true)
static @Nullable <T extends Tag> NBTPath<T> of(T tag, @Nullable Tag expectedRoot) throws IllegalArgumentException, IllegalStateException {
ParentTag<?> parentTag = tag.getParentTag();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本审查建议由 GPT-5 生成


[P2] 这里改用 tag.getParentTag() 会把 parent 是 ChunkrootTag 当成“没有 parent”。但 Tag#getParent() 允许 parent 是 ChunkNBTPathImpl.select 也支持从 Chunk 查询。结果是 NBTPath.of(chunk.getRootTag(), null) 返回 null,而 chunk 内部子 tag 也无法自然生成相对 Chunk 可用的 path,调用方必须额外知道并传入 chunk.getRootTag(),和 NBTParent 支持的根类型不一致。

parentTag = parent;
}

if (parentTag != expectedRoot)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

本审查建议由 GPT-5 生成


[P2] expectedRoot == null 会让非根 tag 固定抛 IllegalStateException。例如 root.addTag("x", tag) 后调用 NBTPath.of(tag, null),循环会停在实际 top root,随后这里的 parentTag != expectedRoot 恒为 true。这个 nullable 参数看起来应支持“使用实际 top of tree”,但当前实现只在显式传入 root tag 时可用,测试也没有覆盖 null root。

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.

2 participants