Skip to content

Track asset provenance when using the paint editor #229

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

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

georgyangelov
Copy link
Contributor

@georgyangelov georgyangelov commented Apr 10, 2025

Resolves

Something we talked about on the NGP planning

Proposed Changes

Describe what this Pull Request does

Tracks which asset id an asset edited through the paint editor is based on. Tries to only track uploaded (clean) ones. That's what clean means, right?

Requires the change from scratchfoundation/scratch-storage#754

Note on the unused store parameter - in NGP we're not using the webHelper for storing (as it doesn't support async functions), so we can just utilize that parameter there to send to the server together with the rest of the asset.

Reason for Changes

Explain why these changes should be made

@cwillisf was talking about it and I just opened the code to see how hard it would be, and there you have it - this PR 😄 I haven't tested it and there is no test coverage, so it's left in a Draft state.

Test Coverage

Please show how you have added tests to cover your changes

I didn't 😅

@georgyangelov georgyangelov requested a review from cwillisf April 10, 2025 08:59
Copy link
Contributor

@cwillisf cwillisf left a comment

Choose a reason for hiding this comment

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

Wow, nice!
If I make many edits to a costume before a single project save, would this end up setting that the provenance field each time? I think it should only include IDs that made it to the asset server (or to disk, in the apps case)

@georgyangelov
Copy link
Contributor Author

georgyangelov commented Apr 11, 2025

I think it should only include IDs that made it to the asset server (or to disk, in the apps case)

Yep, that's what I was trying to do with...

const provenance = currentAsset.clean ? currentAsset.assetId : currentAsset.provenance;

...so that the provenance is only taken from clean (saved) assets and propagated through unsaved versions.

But as it is untested, there may be some case I haven't considered.

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.

None yet

2 participants