Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions main/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,66 @@ import { translate } from "./translation.js";
import { getMetadata } from "meta-png";
import { AUTOSAVE_KEY } from "../config.js";

function collectWorkspaceSnapshot(ws) {
if (!ws) {
return {
blockIds: new Set(),
variableIds: new Set(),
blockCount: 0,
variableCount: 0,
};
}

const blocks = ws.getAllBlocks?.(false) || [];
const variables = ws.getVariableMap?.().getAllVariables?.() || [];

return {
blockIds: new Set(blocks.map((b) => b.id).filter(Boolean)),
variableIds: new Set(variables.map((v) => v.getId?.()).filter(Boolean)),
blockCount: blocks.length,
variableCount: variables.length,
};
}

function collectIncomingSnapshot(json) {
const blockIds = new Set();
const variableIds = new Set();

const walkBlock = (block) => {
if (!block || typeof block !== "object") return;
if (block.id) blockIds.add(block.id);

if (block.inputs && typeof block.inputs === "object") {
Object.values(block.inputs).forEach((input) => {
walkBlock(input?.block);
walkBlock(input?.shadow);
});
}

if (block.next?.block) walkBlock(block.next.block);
};

(json?.blocks?.blocks || []).forEach(walkBlock);
(json?.variables || []).forEach((v) => {
if (v?.id) variableIds.add(v.id);
});

return {
blockIds,
variableIds,
blockCount: blockIds.size,
variableCount: variableIds.size,
};
}

function intersectSets(a, b) {
const result = [];
a.forEach((value) => {
if (b.has(value)) result.push(value);
});
return result;
}

// Function to save the current workspace state
export function saveWorkspace(workspace) {
if (workspace && workspace.getAllBlocks) {
Expand Down Expand Up @@ -326,9 +386,50 @@ export function loadWorkspaceAndExecute(json, workspace, executeCallback) {

// Validate JSON before loading into workspace
const validatedJson = validateBlocklyJson(json);
console.log("[workspace-load:entry]", {
workspaceId: workspace?.id ?? null,
topLevelBlocksInJson: validatedJson?.blocks?.blocks?.length ?? 0,
variablesInJson: validatedJson?.variables?.length ?? 0,
});
const before = collectWorkspaceSnapshot(workspace);
const incoming = collectIncomingSnapshot(validatedJson);
const collidingBlockIds = intersectSets(before.blockIds, incoming.blockIds);
const collidingVariableIds = intersectSets(
before.variableIds,
incoming.variableIds,
);

console.log("[workspace-load:before]", {
existingBlocks: before.blockCount,
existingVariables: before.variableCount,
incomingBlocks: incoming.blockCount,
incomingVariables: incoming.variableCount,
});
console.log("[workspace-load:collisions]", {
blockIdCollisions: collidingBlockIds.length,
variableIdCollisions: collidingVariableIds.length,
sampleBlockIds: collidingBlockIds.slice(0, 10),
sampleVariableIds: collidingVariableIds.slice(0, 10),
});

// Load the validated JSON
Blockly.serialization.workspaces.load(validatedJson, workspace);
const after = collectWorkspaceSnapshot(workspace);
const missingIncomingBlockIds = [...incoming.blockIds].filter(
(id) => !after.blockIds.has(id),
);
const missingIncomingVariableIds = [...incoming.variableIds].filter(
(id) => !after.variableIds.has(id),
);

console.log("[workspace-load:after]", {
resultingBlocks: after.blockCount,
resultingVariables: after.variableCount,
missingIncomingBlockIds: missingIncomingBlockIds.length,
missingIncomingVariableIds: missingIncomingVariableIds.length,
sampleMissingBlockIds: missingIncomingBlockIds.slice(0, 10),
sampleMissingVariableIds: missingIncomingVariableIds.slice(0, 10),
});
workspace.scroll(0, 0);
executeCallback();
} catch (error) {
Expand Down Expand Up @@ -726,6 +827,12 @@ function appendSnippetBlocksAtViewport(workspace, blocksJson) {

// Private helper: process a project file (used by file input and drag-and-drop)
function processProjectFileDrop(file, workspace, executeCallback) {
console.log("[workspace-import:start]", {
fileName: file?.name ?? null,
fileSize: file?.size ?? null,
workspaceId: workspace?.id ?? null,
});
Comment on lines +830 to +834
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don't log imported filenames by default.

file.name is user-supplied and can contain personal or project info. Emitting it on every drag-and-drop import exposes that data in normal browser console output; gate this behind a debug flag or omit the name.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@main/files.js` around lines 830 - 834, The console.log call emitting
file.name in the "[workspace-import:start]" log exposes user-supplied filenames;
update the log in main/files.js so it no longer prints file?.name by
default—either remove file?.name from the logged object or wrap the log behind a
debug flag (e.g., check a DEBUG/verbose setting) so filenames are only emitted
when explicitly enabled; keep file?.size and workspace?.id as needed and adjust
the log call that contains the "[workspace-import:start]" tag accordingly.


const maxSize = 5 * 1024 * 1024;
if (file.size > maxSize) {
alert(translate("file_too_large_alert"));
Expand Down
140 changes: 138 additions & 2 deletions ui/blockmesh.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,86 @@ export function getMeshesFromBlockKey(blockKey) {
return set ? [...set] : [];
}

function getUserVariableNameFromBlock(block, fieldName = "ID_VAR") {
if (!block?.workspace || typeof block.getFieldValue !== "function") return null;
const variableId = block.getFieldValue(fieldName);
if (!variableId) return null;
const variableModel = block.workspace.getVariableById?.(variableId);
return variableModel?.name ?? null;
}

function findMeshesByBaseName(baseName) {
if (!baseName || !flock?.scene) return [];
const candidates = [baseName, baseName.replace(/[^a-zA-Z0-9._-]/g, "")];
const seen = new Set();
const matches = [];

for (const name of candidates) {
if (!name || seen.has(name)) continue;
seen.add(name);

const exact = flock.scene.getMeshByName?.(name);
if (exact) matches.push(exact);

for (const mesh of flock.scene.meshes ?? []) {
if (
mesh?.name &&
mesh.name !== name &&
mesh.name.startsWith(`${name}_`) &&
Number.isFinite(Number(mesh.name.slice(name.length + 1)))
) {
matches.push(mesh);
}
}
}

return matches;
}

function rebindMeshesToBlockKey(meshes, blockKey) {
if (!blockKey || !Array.isArray(meshes) || meshes.length === 0) return;
for (const mesh of meshes) {
if (!mesh) continue;
mesh.metadata = mesh.metadata || {};
mesh.metadata.blockKey = blockKey;
const descendants = mesh.getDescendants?.(false) ?? [];
for (const child of descendants) {
child.metadata = child.metadata || {};
child.metadata.blockKey = blockKey;
}
}
_meshIndexDirty = true;
}
Comment on lines +229 to +242
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fallback rebinding is too broad and leaves owner state stale.

Line 235 rewrites every descendant's metadata.blockKey, so separately-owned attached meshes under that hierarchy can get reassigned too. The helper also never updates meshMap / meshBlockIdMap; when blockKey !== block.id, the later getBlockById(blockKey) recovery path cannot repair the mapping after this fallback.

Also applies to: 269-270

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/blockmesh.js` around lines 229 - 242, The fallback rebinding in
rebindMeshesToBlockKey is too aggressive: only set mesh.metadata.blockKey (and
descendants') when they do not already have an explicit owner/blockKey (skip
descendants where child.metadata.blockKey or child.metadata.owner is present and
differs), and whenever you change a mesh's blockKey ensure you also update the
global maps (meshMap and meshBlockIdMap) so lookups can be recovered (handle the
case blockKey !== block.id by moving the mesh entry to the new blockKey in those
maps). Apply the same guarded-update logic to the other rebind site mentioned
(lines ~269-270) so you don't clobber separately-owned attached meshes and
mappings.


function getFallbackMeshesForLoadBlock(block, blockKey = null) {
if (!block) return [];
if (!["load_object", "load_multi_object", "load_character"].includes(block.type))
return [];

const variableName = getUserVariableNameFromBlock(block, "ID_VAR");
if (!variableName) return [];
const matches = findMeshesByBaseName(variableName);

const ws = block.workspace;
const mainWs = Blockly.getMainWorkspace?.();
console.log("[mesh-lookup:fallback]", {
blockType: block.type,
blockId: block.id,
blockKey: blockKey || block.id,
workspaceId: ws?.id ?? null,
isFlyout: !!ws?.isFlyout,
isMainWorkspace: !!(ws && mainWs && ws === mainWs),
variableName,
matchedMeshes: matches.map((m) => ({
name: m?.name,
blockKey: m?.metadata?.blockKey ?? null,
})),
});
Comment on lines +255 to +267
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Keep the new lookup tracing behind flock.meshDebug.

These traces are unconditional, unlike the rest of this file. Because updateOrCreateMeshFromBlock() resolves meshes before the create path, expected create-time misses now emit warnings outside debug sessions and can drown out real diagnostics.

🧰 Suggested guard pattern
-  console.warn("[mesh-lookup:miss]", {
-    blockType: block.type,
-    blockId: block.id,
-    blockKey,
-    workspaceId: ws?.id ?? null,
-    isFlyout: !!ws?.isFlyout,
-    isMainWorkspace: !!(ws && mainWs && ws === mainWs),
-    knownSceneKeys: [...new Set((flock.scene?.meshes ?? [])
-      .map((m) => m?.metadata?.blockKey)
-      .filter(Boolean))].slice(0, 50),
-  });
+  if (flock.meshDebug) {
+    console.warn("[mesh-lookup:miss]", {
+      blockType: block.type,
+      blockId: block.id,
+      blockKey,
+      workspaceId: ws?.id ?? null,
+      isFlyout: !!ws?.isFlyout,
+      isMainWorkspace: !!(ws && mainWs && ws === mainWs),
+      knownSceneKeys: [...new Set((flock.scene?.meshes ?? [])
+        .map((m) => m?.metadata?.blockKey)
+        .filter(Boolean))].slice(0, 50),
+    });
+  }

Also applies to: 324-355, 410-437

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@ui/blockmesh.js` around lines 255 - 267, The unconditional console.log
tracing in ui/blockmesh.js (e.g., inside updateOrCreateMeshFromBlock() fallback
that prints matchedMeshes and related info) should be guarded behind the
flock.meshDebug flag so these messages only appear in debug sessions; wrap those
console.log blocks (including the occurrences around the fallback trace and the
other traces at the ranges you noted) with a check like if (flock.meshDebug) {
... } so normal runs won’t emit create-time miss warnings.


rebindMeshesToBlockKey(matches, blockKey || block.id);
return matches;
}

export function getMeshFromBlock(block) {
if (!block) return null;

Expand Down Expand Up @@ -241,8 +321,38 @@ export function getMeshFromBlock(block) {

const blockKey = getBlockKeyFromBlock(block) || block.id;
if (!blockKey) return null;
const direct = getMeshFromBlockKey(blockKey);
if (direct) {
const ws = block.workspace;
const mainWs = Blockly.getMainWorkspace?.();
console.log("[mesh-lookup:direct]", {
blockType: block.type,
blockId: block.id,
blockKey,
workspaceId: ws?.id ?? null,
isFlyout: !!ws?.isFlyout,
isMainWorkspace: !!(ws && mainWs && ws === mainWs),
meshName: direct.name,
meshBlockKey: direct.metadata?.blockKey ?? null,
});
return direct;
}

const ws = block.workspace;
const mainWs = Blockly.getMainWorkspace?.();
console.warn("[mesh-lookup:miss]", {
blockType: block.type,
blockId: block.id,
blockKey,
workspaceId: ws?.id ?? null,
isFlyout: !!ws?.isFlyout,
isMainWorkspace: !!(ws && mainWs && ws === mainWs),
knownSceneKeys: [...new Set((flock.scene?.meshes ?? [])
.map((m) => m?.metadata?.blockKey)
.filter(Boolean))].slice(0, 50),
});

return getMeshFromBlockKey(blockKey);
return getFallbackMeshesForLoadBlock(block, blockKey)[0] ?? null;
}

export function getMeshesFromBlock(block) {
Expand Down Expand Up @@ -297,8 +407,34 @@ export function getMeshesFromBlock(block) {

const blockKey = getBlockKeyFromBlock(block) || block.id;
if (!blockKey) return [];
const meshes = getMeshesFromBlockKey(blockKey);
if (meshes.length > 0) {
const ws = block.workspace;
const mainWs = Blockly.getMainWorkspace?.();
console.log("[mesh-lookup:direct-many]", {
blockType: block.type,
blockId: block.id,
blockKey,
workspaceId: ws?.id ?? null,
isFlyout: !!ws?.isFlyout,
isMainWorkspace: !!(ws && mainWs && ws === mainWs),
meshNames: meshes.map((m) => m?.name),
});
return meshes;
}

const ws = block.workspace;
const mainWs = Blockly.getMainWorkspace?.();
console.warn("[mesh-lookup:miss-many]", {
blockType: block.type,
blockId: block.id,
blockKey,
workspaceId: ws?.id ?? null,
isFlyout: !!ws?.isFlyout,
isMainWorkspace: !!(ws && mainWs && ws === mainWs),
});

return getMeshesFromBlockKey(blockKey);
return getFallbackMeshesForLoadBlock(block, blockKey);
}

// Safe field getter. Returns null when field is missing or name is invalid.
Expand Down
Loading