Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perform tilemap upgrade on xml instead of workspace #10368

Merged
merged 2 commits into from
Feb 6, 2025
Merged

perform tilemap upgrade on xml instead of workspace #10368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8560bb
perform tilemap upgrade on xml instead of workspace
riknoll Feb 4, 2025
6780c18
fix tests
riknoll Feb 4, 2025
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
14 changes: 9 additions & 5 deletions pxtblocks/fields/field_tileset.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,14 +193,18 @@ export class FieldTileset extends FieldImages implements FieldCustom {
if (newValue) {
const project = pxt.react.getTilemapProject();
const match = /^\s*assets\s*\.\s*tile\s*`([^`]*)`\s*$/.exec(newValue);
let tile: pxt.Tile;

if (match) {
const tile = project.lookupAssetByName(pxt.AssetType.Tile, match[1]);
tile = project.lookupAssetByName(pxt.AssetType.Tile, match[1]);
}
else if (newValue.startsWith(pxt.sprite.TILE_NAMESPACE)) {
tile = project.lookupAsset(pxt.AssetType.Tile, newValue.trim());
}

if (tile) {
this.localTile = tile;
return newValue;
}
if (tile) {
this.localTile = tile;
return pxt.getTSReferenceForAsset(tile, false);
}
}

Expand Down
67 changes: 39 additions & 28 deletions pxtblocks/fields/field_utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,28 +238,46 @@ export function needsTilemapUpgrade(ws: Blockly.Workspace) {
return !!allTiles.length;
}

export function upgradeTilemapsInWorkspace(ws: Blockly.Workspace, proj: pxt.TilemapProject) {
const allTiles = ws.getVariablesOfType(pxt.sprite.BLOCKLY_TILESET_TYPE).map(model => pxt.sprite.legacy.blocklyVariableToTile(model.name));
if (!allTiles.length) return;

try {
Blockly.Events.disable();
let customMapping: pxt.Tile[] = [];

for (const tile of allTiles) {
if (tile.qualifiedName) {
customMapping[tile.projectId] = proj.resolveTile(tile.qualifiedName);
export function updateTilemapXml(dom: Element, proj: pxt.TilemapProject) {
let needsUpgrade = false;

const upgradedTileMapping: pxt.Map<pxt.Tile> = {};

for (const element of dom.children) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious if iterating over everything in the dom is the most efficient way to do this.. Would this or would filtering for elements with the variable tag and then iterating through those entries be faster?

Copy link
Member Author

Choose a reason for hiding this comment

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

i don't think it would make a huge difference either way. this is only looping over the top-level elements in the xml file so it should only be the variables and any top-level events like on-start

if (element.tagName.toLowerCase() === "variables") {
const toRemove: Element[] = [];

for (const variable of element.children) {
if (variable.getAttribute("type") === pxt.sprite.BLOCKLY_TILESET_TYPE) {
needsUpgrade = true;
const varName = variable.textContent;
const parsed = pxt.sprite.legacy.blocklyVariableToTile(varName);
if (!parsed.qualifiedName) {
const oldId = "myTiles.tile" + parsed.projectId;
const newTile = proj.createNewTile(parsed.data, oldId);
upgradedTileMapping[oldId] = newTile
}
toRemove.push(variable);
}
}
else if (tile.data) {
customMapping[tile.projectId] = proj.createNewTile(tile.data, "myTiles.tile" + tile.projectId);

for (const variable of toRemove) {
variable.remove();
}
deleteTilesetTileIfExists(ws, tile);
}
}

if (!needsUpgrade) return;

const tilemaps = getAllBlocksWithTilemaps(ws);
for (const field of dom.getElementsByTagName("field")) {
const value = field.textContent;

for (const tilemap of tilemaps) {
const legacy = pxt.sprite.legacy.decodeTilemap(tilemap.ref.getInitText(), "typescript");
const trimmed = value.trim();
if (upgradedTileMapping[trimmed]) {
field.textContent = pxt.getTSReferenceForAsset(upgradedTileMapping[trimmed]);
}
else if (trimmed.startsWith(`tiles.createTilemap(`)) {
const legacy = pxt.sprite.legacy.decodeTilemap(value, "typescript");

const mapping: pxt.Tile[] = [];

Expand All @@ -268,7 +286,7 @@ export function upgradeTilemapsInWorkspace(ws: Blockly.Workspace, proj: pxt.Tile
tileWidth: legacy.tileset.tileWidth,
tiles: legacy.tileset.tiles.map((t, index) => {
if (t.projectId != null) {
return customMapping[t.projectId];
return upgradedTileMapping["myTiles.tile" + t.projectId];
}
if (!mapping[index]) {
mapping[index] = proj.resolveTile(t.qualifiedName)
Expand All @@ -280,18 +298,11 @@ export function upgradeTilemapsInWorkspace(ws: Blockly.Workspace, proj: pxt.Tile
legacy.layers
);

tilemap.ref.setValue(pxt.sprite.encodeTilemap(newData, "typescript"));
}

const tilesets = getAllBlocksWithTilesets(ws);
const [id] = proj.createNewTilemapFromData(newData);
const asset = proj.lookupAsset(pxt.AssetType.Tilemap, id);

for (const tileset of tilesets) {
// Force a re-render. getSize() will rerender if necessary
tileset.ref.doValueUpdate_(tileset.ref.getValue());
tileset.ref.getSize();
field.textContent = pxt.getTSReferenceForAsset(asset);
}
} finally {
Blockly.Events.enable();
}
}

Expand Down
7 changes: 6 additions & 1 deletion pxtblocks/importer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import * as Blockly from "blockly";
import { blockSymbol, buildinBlockStatements, hasArrowFunction, initializeAndInject } from "./loader";
import { extensionBlocklyPatch } from "./external";
import { FieldBase } from "./fields";
import { FieldBase, updateTilemapXml } from "./fields";

export interface BlockSnippet {
target: string; // pxt.appTarget.id
Expand Down Expand Up @@ -31,6 +31,11 @@ export function domToWorkspaceNoEvents(dom: Element, workspace: Blockly.Workspac
let newBlockIds: string[] = [];
patchCommentIds(dom);
patchShadows(dom, false);

if (pxt.react.getTilemapProject) {
updateTilemapXml(dom, pxt.react.getTilemapProject());
}

try {
Blockly.Events.disable();
newBlockIds = Blockly.Xml.domToWorkspace(dom, workspace);
Expand Down
2 changes: 0 additions & 2 deletions webapp/src/blocks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,6 @@ export class Editor extends toolboxeditor.ToolboxEditor {
this.cleanXmlForWorkspace(xml);
pxtblockly.domToWorkspaceNoEvents(xml, this.editor);

pxtblockly.upgradeTilemapsInWorkspace(this.editor, pxt.react.getTilemapProject());

this.initLayout(xml);
this.editor.clearUndo();
this.reportDeprecatedBlocks();
Expand Down
1 change: 0 additions & 1 deletion webapp/src/compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,6 @@ function upgradeFromBlocksAsync(): Promise<UpgradeResult> {

const xml = Blockly.utils.xml.textToDom(text);
pxtblockly.domToWorkspaceNoEvents(xml, ws);
pxtblockly.upgradeTilemapsInWorkspace(ws, pxt.react.getTilemapProject());
const upgradedXml = pxtblockly.workspaceToDom(ws);
patchedFiles[pxt.MAIN_BLOCKS] = Blockly.Xml.domToText(upgradedXml);

Expand Down