Skip to content

Commit 4f2688c

Browse files
CopilotDaanV2
andauthored
Fix structure identifiers to use namespace:identifier format (#454)
Structure identifiers were generated as slash-separated paths (e.g. `"puff/coin1"`) instead of the Minecraft Bedrock `namespace:identifier` format (e.g. `puff:coin1`). This caused correct usage to error and completions to suggest invalid identifiers. ## ID generation (`process.ts`) The first folder segment after `structures/` is now the namespace; the remainder is the identifier. Files directly in `structures/` fall back to the `mystructure` namespace. | File path | Old ID | New ID | |---|---|---| | `structures/house.mcstructure` | `house` | `mystructure:house` | | `structures/puff/coin1.mcstructure` | `"puff/coin1"` | `puff:coin1` | | `structures/stuff/towers/diamond.mcstructure` | `"stuff/towers/diamond"` | `stuff:towers/diamond` | ## Diagnosis (`diagnose.ts`) Removed the slash↔colon translation logic that was converting `namespace:identifier` input into the wrong lookup key, and the spurious "needs quotes" error for slash-containing IDs. Lookup is now a direct match against the stored `namespace:identifier`. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: DaanV2 <2393905+DaanV2@users.noreply.github.com>
1 parent 803a7c1 commit 4f2688c

6 files changed

Lines changed: 70 additions & 60 deletions

File tree

ide/vscode/src/version.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
export const Version = "9.0.30";
1+
export const Version = "9.0.31";

packages/bedrock-diagnoser/src/diagnostics/behavior-pack/structure/diagnose.ts

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { OffsetWord } from 'bc-minecraft-bedrock-shared';
22
import { Errors } from '../..';
3-
import { DiagnosticsBuilder, DiagnosticSeverity } from '../../../types';
3+
import { DiagnosticsBuilder } from '../../../types';
44
import { check_definition_value } from '../../definitions';
55

66
export function diagnose_structure_implementation(
@@ -9,37 +9,20 @@ export function diagnose_structure_implementation(
99
): boolean {
1010
const strId = typeof id === 'string' ? id : id.text;
1111

12-
//If it has a slash it needs ""
13-
if (strId.includes('/')) {
14-
if (strId.startsWith('"') && strId.endsWith('"')) {
15-
// Do nothing
16-
} else {
17-
diagnoser.add(
18-
id,
19-
`A structure id with '/' needs quotes surrounding it: ${strId} => "${strId}"`,
20-
DiagnosticSeverity.error,
21-
'behaviorpack.mcstructure.invalid',
22-
);
23-
}
24-
25-
//Project has structures
26-
const struc = diagnoser.context.getProjectData().behaviors.structures.get(strId, diagnoser.project);
27-
if (struc !== undefined) {
28-
return true;
29-
}
30-
}
31-
3212
const data = diagnoser.context.getProjectData().projectData;
13+
14+
// Check general structures (vanilla etc.)
3315
if (data.general.structures.has(strId)) return true;
3416

35-
//structures can be identified with : or /
36-
if (strId.includes(':')) {
37-
let cid = strId.replace('mystructure:', '').replace(':', '/');
38-
if (!cid.includes('/')) cid = cid.replace(/"/g, '');
39-
if (check_definition_value(diagnoser.project.definitions.structure, cid, diagnoser)) return true;
40-
if (data.behaviorPacks.structures.has(cid)) return true;
41-
if (data.general.structures.has(cid)) return true;
42-
}
17+
// Check project-defined structures
18+
const struc = diagnoser.context.getProjectData().behaviors.structures.get(strId, diagnoser.project);
19+
if (struc !== undefined) return true;
20+
21+
// Check definitions (from project configuration)
22+
if (check_definition_value(diagnoser.project.definitions.structure, strId, diagnoser)) return true;
23+
24+
// Check behavior pack structures collection
25+
if (data.behaviorPacks.structures.has(strId)) return true;
4326

4427
//Nothing then report error
4528
Errors.missing('behaviors', 'structures', strId, diagnoser, id);

packages/bedrock-diagnoser/test/lib/diagnostics/behavior-pack/structure.test.ts

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,43 +12,46 @@ describe("BehaviorPack", () => {
1212
data = diagnoser.context.getProjectData().projectData;
1313
});
1414

15-
it("quotes", () => {
15+
it("simple namespace:name no errors", () => {
1616
data.behaviorPacks.packs[0].structures.set({
17-
id: "test/example",
17+
id: "puff:coin1",
1818
documentation: "",
1919
location: { position: 0, uri: "" },
2020
});
2121

22-
diagnose_structure_implementation({ offset: 0, text: "test/example" }, diagnoser);
23-
diagnose_structure_implementation({ offset: 0, text: "test:example" }, diagnoser);
22+
diagnose_structure_implementation({ offset: 0, text: "puff:coin1" }, diagnoser);
2423

25-
diagnoser.expectAmount(1);
24+
diagnoser.expectEmpty();
2625
});
2726

28-
it("no errors", () => {
27+
it("deep path namespace:name no errors", () => {
2928
data.behaviorPacks.packs[0].structures.set({
30-
id: '"test/example"',
29+
id: "stuff:towers/diamond",
3130
documentation: "",
3231
location: { position: 0, uri: "" },
3332
});
3433

35-
diagnose_structure_implementation({ offset: 0, text: '"test/example"' }, diagnoser);
36-
diagnose_structure_implementation({ offset: 0, text: '"test:example"' }, diagnoser);
34+
diagnose_structure_implementation({ offset: 0, text: "stuff:towers/diamond" }, diagnoser);
3735

3836
diagnoser.expectEmpty();
3937
});
4038

41-
it("missing", () => {
39+
it("mystructure namespace for root structures", () => {
4240
data.behaviorPacks.packs[0].structures.set({
43-
id: '"test/example"',
41+
id: "mystructure:house",
4442
documentation: "",
4543
location: { position: 0, uri: "" },
4644
});
4745

48-
diagnose_structure_implementation({ offset: 0, text: '"t/example"' }, diagnoser);
49-
diagnose_structure_implementation({ offset: 0, text: '"t:example"' }, diagnoser);
46+
diagnose_structure_implementation({ offset: 0, text: "mystructure:house" }, diagnoser);
5047

51-
diagnoser.expectAmount(2);
48+
diagnoser.expectEmpty();
49+
});
50+
51+
it("missing structure reports error", () => {
52+
diagnose_structure_implementation({ offset: 0, text: "puff:coin1" }, diagnoser);
53+
54+
diagnoser.expectAmount(1);
5255
});
5356
});
5457
});
Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,34 +1,45 @@
11
// Jest Snapshot v1, https://jestjs.io/docs/snapshot-testing
22

3-
exports[`Structure "empty/air_1" from F:/Temp2/world/behavior_packs/EW-BP/structures/empty/air_1.mcstructure 1`] = `
3+
exports[`Structure empty:air_1 from F:/Temp2/world/behavior_packs/EW-BP/structures/empty/air_1.mcstructure 1`] = `
44
{
5-
"documentation": "McStructure: "empty/air_1"",
6-
"id": ""empty/air_1"",
5+
"documentation": "McStructure: empty:air_1",
6+
"id": "empty:air_1",
77
"location": {
88
"position": 0,
99
"uri": "F:/Temp2/world/behavior_packs/EW-BP/structures/empty/air_1.mcstructure",
1010
},
1111
}
1212
`;
1313

14-
exports[`Structure "empty/air_1" from F:\\Temp2\\world\\behavior_packs\\EW-BP\\structures\\empty\\air_1.mcstructure 1`] = `
14+
exports[`Structure empty:air_1 from F:\\Temp2\\world\\behavior_packs\\EW-BP\\structures\\empty\\air_1.mcstructure 1`] = `
1515
{
16-
"documentation": "McStructure: "empty/air_1"",
17-
"id": ""empty/air_1"",
16+
"documentation": "McStructure: empty:air_1",
17+
"id": "empty:air_1",
1818
"location": {
1919
"position": 0,
2020
"uri": "F:\\Temp2\\world\\behavior_packs\\EW-BP\\structures\\empty\\air_1.mcstructure",
2121
},
2222
}
2323
`;
2424

25-
exports[`Structure "empty/temp/air_1" from F:/Temp2/world/behavior_packs/EW-BP/structures/empty/temp/air_1.mcstructure 1`] = `
25+
exports[`Structure empty:temp/air_1 from F:/Temp2/world/behavior_packs/EW-BP/structures/empty/temp/air_1.mcstructure 1`] = `
2626
{
27-
"documentation": "McStructure: "empty/temp/air_1"",
28-
"id": ""empty/temp/air_1"",
27+
"documentation": "McStructure: empty:temp/air_1",
28+
"id": "empty:temp/air_1",
2929
"location": {
3030
"position": 0,
3131
"uri": "F:/Temp2/world/behavior_packs/EW-BP/structures/empty/temp/air_1.mcstructure",
3232
},
3333
}
3434
`;
35+
36+
exports[`Structure mystructure:house from F:/Temp2/world/behavior_packs/EW-BP/structures/house.mcstructure 1`] = `
37+
{
38+
"documentation": "McStructure: mystructure:house",
39+
"id": "mystructure:house",
40+
"location": {
41+
"position": 0,
42+
"uri": "F:/Temp2/world/behavior_packs/EW-BP/structures/house.mcstructure",
43+
},
44+
}
45+
`;

packages/bedrock-project/src/project/behavior-pack/structure/process.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,20 @@ export function process(doc: TextDocument): Structure | undefined {
1414
if (index < 0) return undefined;
1515
index += 11;
1616

17-
let id = uri.substring(index, uri.length).replace(/\\/g, '/');
18-
id = id.replace('.mcstructure', '');
17+
let path = uri.substring(index, uri.length).replace(/\\/g, '/');
18+
path = path.replace('.mcstructure', '');
1919

20-
if (id.includes('/')) {
21-
id = '"' + id + '"';
20+
// The first path segment is the namespace; the rest is the identifier.
21+
// Files directly in the structures/ folder use the 'mystructure' namespace.
22+
const slashIndex = path.indexOf('/');
23+
let id: string;
24+
25+
if (slashIndex < 0) {
26+
id = `mystructure:${path}`;
27+
} else {
28+
const namespace = path.substring(0, slashIndex);
29+
const name = path.substring(slashIndex + 1);
30+
id = `${namespace}:${name}`;
2231
}
2332

2433
return {

packages/bedrock-project/src/project/behavior-pack/structure/structure.test.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,19 @@ describe('Structure', () => {
44
const data: { uri: string; result: string }[] = [
55
{
66
uri: 'F:\\Temp2\\world\\behavior_packs\\EW-BP\\structures\\empty\\air_1.mcstructure',
7-
result: '"empty/air_1"',
7+
result: 'empty:air_1',
88
},
99
{
1010
uri: 'F:/Temp2/world/behavior_packs/EW-BP/structures/empty/air_1.mcstructure',
11-
result: '"empty/air_1"',
11+
result: 'empty:air_1',
1212
},
1313
{
1414
uri: 'F:/Temp2/world/behavior_packs/EW-BP/structures/empty/temp/air_1.mcstructure',
15-
result: '"empty/temp/air_1"',
15+
result: 'empty:temp/air_1',
16+
},
17+
{
18+
uri: 'F:/Temp2/world/behavior_packs/EW-BP/structures/house.mcstructure',
19+
result: 'mystructure:house',
1620
},
1721
];
1822

0 commit comments

Comments
 (0)