-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Block versioning #4
base: main
Are you sure you want to change the base?
Conversation
var isSingleSprite = false; | ||
if (isSingleSpriteOrExtensionManager && isSingleSpriteOrExtensionManager.runtime == runtime) { | ||
extensionManager = isSingleSpriteOrExtensionManager; | ||
} else if (isSingleSpriteOrExtensionManager == true && isSingleSpriteOrExtensionManager == false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This else if
seems contradictory
@@ -974,18 +1248,183 @@ const parseScratchObject = function (object, runtime, extensions, zip, assets) { | |||
sprite.name = object.name; | |||
} | |||
if (Object.prototype.hasOwnProperty.call(object, 'blocks')) { | |||
deserializeBlocks(object.blocks); | |||
deserializeBlocks(object.blocks, extensionManager); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove extensionManager
argument
var { inputs, variables } = gatherInputs(object.blocks, blockJSON); | ||
var fields = gatherFields(blockJSON, blockArgs); | ||
var totalList = addInputsAndFields(inputs, fields, blockArgs); | ||
const newInputs = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI Not sure about this variable specifically, but for a lot of this storage objects, it'd lead to easier to read code to use a Map instead.
And if you later need an object, you can always go: const obj = Object.fromEntries(map);
const regex = /_v\d+/g; // Matches _v followed by one or more digits | ||
blockInfoIndex = blockInfoIndex.replace(regex, ""); // Replaces all matches with an empty string | ||
const menuRegex = /menu_\d+/; | ||
if (!menuRegex.test(blockInfoIndex)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! so we don't apply version to menus, makes sense -- I do wonder what happens when a project is saved with a menu value that disappears in later versions. Is that something that's handled?
} | ||
obj[blockID] = serializeBlock(blocks[blockID], blocks); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we store the version in the opcode
but the ID is kept the same?
Resolves
What Github issue does this resolve (please include link)?
Proposed Changes
Describe what this Pull Request does
Reason for Changes
Explain why these changes should be made
Test Coverage
Please show how you have added tests to cover your changes