Support Fields Having Pre Select Scoring Zones [AARD-1894]
#1211
Conversation
ryanzhangofficial
requested review from
azaleacolburn,
ariesninjadev,
PepperLola and
BrandonPacewic
ryanzhangofficial
added
ui/ux
| try { | ||
| // Re-encode the assembly with devtool changes | ||
| const updatedBuffer = mirabuf.Assembly.encode(assembly).finish() | ||
|
|
||
| // Update the cached buffer | ||
| const cache = miraType == MiraType.ROBOT ? backUpRobots : backUpFields | ||
| if (cache[id]) { | ||
| cache[id].buffer = updatedBuffer | ||
| } | ||
|
|
||
| // Update OPFS if available | ||
| if (canOPFS) { | ||
| const fileHandle = await ( | ||
| miraType == MiraType.ROBOT ? robotFolderHandle : fieldFolderHandle | ||
| ).getFileHandle(id, { create: false }) | ||
| const writable = await fileHandle.createWritable() | ||
| await writable.write(updatedBuffer) | ||
| await writable.close() | ||
| } | ||
|
|
||
| World.analyticsSystem?.event("Devtool Cache Persist", { | ||
| key: id, | ||
| type: miraType == MiraType.ROBOT ? "robot" : "field", | ||
| assemblyName: assembly.info?.name ?? "unknown", | ||
| fileSize: updatedBuffer.byteLength, | ||
| }) | ||
|
|
||
| return true | ||
| } catch (e) { | ||
| console.error("Failed to persist devtool changes", e) | ||
| World.analyticsSystem?.exception("Failed to persist devtool changes to cache") | ||
| return false | ||
| } |
There was a problem hiding this comment.
We should try to put the try block around as small a section of fallible code as possible. It seems like the fallible function here is the event method call (obviously any file I/O is subject to failure as well, but since we're creating the directory, it shouldn't regularly fail), unless I'm missing something. So maybe we could more the try block to be just around the event method class.
There was a problem hiding this comment.
I would argue that most of the lines in the current try block is fallible. For example, const updatedBuffer = mirabuf.Assembly.encode(assembly).finish() if the assembly is malformed.
rutmanz
changed the title
[AARD-1894]
There was a problem hiding this comment.
I'm trying to test this but it doesn't actually seem to persist the changes? These are the steps I'm taking:
- Import field
- Add scoring zone
- Go to devtool editor and click "save" (I see the toast that says they've been persisted)
- Clear preferences (because scoring zone data is already scored here; I think part of this PR would also be ending the storage of scoring zones in the preferences cache so it can stay with the mirabuf file)
- Reload
- Import same field
- No scoring zones are present
I'm not seeing any console errors
Wouldn't saving data back to the mira file require us to store the mira files for each individual users? The way I understood the PR (I might be wrong) the goal was to allow fields to come with pre-selected scoring zones. Meaning that in the exporter a new tab would be added saving this data to the mira file, that way when the user comes and tries to load a field all the scoring zones are already pre-configured. |
Ah I didn't incorporate the "clear preferences" step on my interpretation of the caching deliverable of this ticket. The changes do persist after reloading if the preferences aren't cleared. However, I don't think it makes sense for the changes to still persist after preferences are cleared especially for default fields. I think your suggestion in |
|
As the current behavior on |
|
The issues should all be addressed. |
Task
Allow users to choose and cache/export where scoring zones are positioned.
AARD-1894
Symptom
Benefits of having direct user editing: bulk edits and reuse, version control and diffs, schema validation
Solution
Developer Tool.UserDataand import them for use cases such as cleared cacheVerification
FieldMiraEditor.test.tsand all passedScoringZonePreferencesare loaded after refresh