Asset Manager: an editor for every asset kind, and an asset you can change your mind about - #87
Merged
Merged
Conversation
The library was a wall of picture-only thumbnails in whatever order the asset database's hash table happened to return, which is not the same order twice. The name reached a person only as a tooltip, and AssetCategory and AssetDescription -- both editable in the inspector -- were read by nothing at all. AssetLibraryWindow is now the class on libWindow and owns the pinned toolbar, the scroller, the five groups and the three pieces of state they share. The toolbar stays put while the groups scroll under it, measured into place rather than positioned against a guessed title-bar height. Filtering matches name, description and category as you type, across every group at once; a group whose matches are all gone keeps its header and reads "Images (0)", so the shape of the library does not move under the person typing. Sorting reorders the tiles that are already there rather than rebuilding them, which keeps the selection, the running animations and the asset acquisitions intact. Three decisions worth recording. Rows mode is MaxColCount = 1, not the control palette's trick of asking for a cell wider than the pane. With CellModeX variable the single column takes the whole width, and unlike a width-derived CellSizeX it survives a resize with nothing recomputing it. The search could not use the engine's queries. findAssetName's partial mode is a case-insensitive prefix match rather than a substring one, findAssetCategory is exact and case-sensitive, and there is no findAssetDescription at all -- so each tile lowercases its own key once and the filter walks those. GuiEditorChoiceRow and GuiEditorToggleIcon become EditorChoiceRow and EditorToggleIcon in EditorCore. editor/main.cs loads AssetAdmin before GuiEditor, so nothing the Asset Manager builds at create time can come out of a module loaded after it; neither file was ever Gui Editor specific. EditorPreferences is the editor's first memory between runs -- dynamic fields on a ScriptObject written as TAML to getPrefsPath, holding the view mode and the sort field. Deliberately not $pref:: globals: script has no setVariable(), so writing one by name would need eval(). The PlanetX assets carry categories now, so the sort has something to sort by. Covered by tests/smoke/assetLibrary.cs, 81 checks, and a shot harness alongside it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
GuiControl::getLineList pushes the line it has been building at the end of every paragraph. When the last word was too wide to fit, that word had already been emitted as a line of its own and the buffer left empty -- so the push added a second, empty line, and a caption of one unbreakable word came out two lines tall. It reads as a bottom-alignment bug, because that is where it shows: the empty line takes the bottom slot and the word climbs out of it. It is not. The line count becomes blockHeight, which is what getTextVerticalOffset positions from, what mTextExtend sizes a control from, and what renderText compares against the room available to decide the text does not fit at all. A whole line of movement under BottomVAlign, half a line under MiddleVAlign, none under TopVAlign, and the wrong answer about fitting under all three. The push is now guarded on the buffer having something in it -- or on the paragraph having produced no lines at all, which is what keeps an empty paragraph yielding the one empty line that blank lines and an empty text box's caret both depend on. The word-fitting half comes out as GuiControl::wrapParagraph, taking a width-measuring callback instead of a GFont so that it can be tested with no GL context: asking a profile for a font registers a texture, and TextureManager asserts without one -- which in a debug build is a modal box, so the failure would arrive as a hang. getTextVerticalOffset is public and static for the same reason. Eight tests in guiTextWrapTests.cc cover the ordinary wrapping, the unbreakable word first, last and doubled, the empty paragraph, and which alignments the stray line moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Two things the harness left lying around. The log. Every engine started from the repo root writes console.log, so a test run and a hand-started editor share one file: they interleave, and in log mode 2, where the file is held open, whichever starts second produces a log nobody can read. That happened twice in one afternoon and both times looked like a broken test. run.ps1 now gives each suite tests/logs/<name>.log and run-unit.ps1 takes tests/logs/unit.log. The log of a suite that failed also survives the rest of the run now, instead of being truncated by the next one. The engine side is a feature that was started and never finished: console.cc has carried a logFileName static, initialised to NULL and read by nothing, next to a hardcoded "console.log". Both open sites now go through it, and Con::setLogFileName is bound to script as setLogFileName(). It has to be called before setLogMode, and mode 2 is handled by closing the held file and reopening under the new name. The leftovers. Tests build project folders, and createTheme copies the stock cursor art into <project>/themes/cursors/<name> the moment it is called -- before any save, and deleteTheme does not take it back. Suites working in a throwaway project lost it with the folder; the four that open PlanetX had been quietly accumulating art inside real content. Remove-TestArtifacts reads each test's own source for setProjectFolder and createTheme and removes what they make. It still runs before each test, which is the guarantee -- a killed test never gets to tidy up, so the next run cannot assume it did -- and now again over every test that ran, once the run is done. -Keep skips the sweep for picking over a failure. A suite cannot do this itself: script has no deleteDirectory binding and both artifacts are directories. Reading the source rather than watching the filesystem means the sweep can only ever remove a name a test itself names, so a run cannot eat work that happened to be in the tree -- but it does mean the name has to be spelled out. The runner warns beside any test that passes either call a variable, rather than silently skipping it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
The Inspector tab was the stock C++ GuiInspector, which reflects every registered field into flat alphabetical groups. For an image asset that meant the eight cell values arrived split into "X Values" and "Y Values", so a cell's width never sat beside its height; an Asset Name box that silently did nothing, because AssetBase::setAssetName is a no-op once the manager owns the asset; an absolute path where the portable one belongs; and nothing at all about the picture -- its size, its frame count, or the fact that the cell values were being ignored. Image assets now get a pane of their own. The rest keep the inspector, which is why the plumbing is a superclass the next asset kind inherits. AssetInspectorPane grids, panels, rows, bind/refresh/commit AssetImageInspectorPane the layout, and what an ImageAsset holds AssetImageCellGrid the eight cell values as one X/Y table Four blocks of roughly equal size in one grid -- identity, frames, settings, description -- so the same pane is 1x4 in a tall narrow frame, 2x2 at the size the inspector opens at, and 4x1 across the foot of a wide screen. A readout under the cell table says what actually loaded and how it cut up, and a warning line surfaces three things that were previously a line in a console log and nothing on screen: an image that did not load, explicit frame mode being on, and a cell layout that does not fit. Absent on purpose: AssetInternal and AssetPrivate exist to keep an asset OUT of the editor; the asset id and file only restate what is on show; and Asset Name is read-only until renaming is done properly, since it changes the asset id and every file that refers to it. BlendColor is absent too, and moved rather than dropped. It tints the base layer that an asset's layers are composed onto, and does nothing at all when there are none -- ImageAsset::setBlendColor warns to the console and returns before the redraw and before the save. So it now lives in row 0 of the Image Layers tab, beside the thing it tints, where that tab's colors are pickers instead of four decimals in a text box. Row 0 wears a padlock while it is the only row: greying a swatch is invisible on its own, since GuiColorPopupCtrl fills its face with the color in every state. EditorFieldRow moves from the Gui Editor to EditorCore, since AssetAdmin depends only on EditorCore and is loaded before the Gui Editor. Two things it hardcoded now come from its owner: the color popup's class, which named a Gui Editor class EditorCore cannot see, and what a Find button measures a path against -- an asset's loose file is relative to the asset's folder, not the game root. Two clipping bugs found on the way. A scroller sized with VertSizing "height" keeps the gap it had to each edge, so a tab page resized twice on the way up left it 12 pixels taller than the page and the page ate the scroll bar's down arrow; "fill" recomputes and cannot drift. And a frame set moves its divider whatever the window in it thinks, so the inspector window's 500-pixel minimum meant 106 pixels of it hung off the right edge, clipped, taking the Find button with it. tests/smoke/assetImageInspector.cs is 97 checks, including the reflow at three widths, both clipping cases, and the layer color column end to end. tests/shots/assetImageInspector.cs takes seven pictures, one of them at 1600x900 because the test canvas is too narrow to show the 4x1 case. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
AnimationAsset::onAssetRefresh called nothing but its empty parent, so the validated frame list was only ever built when the animation itself changed. A refresh reaches it from two directions, though: the asset manager walks the depended-on list, so re-cutting the image underneath an animation refreshes the animation too -- and that pass did nothing. The list then still held indices from the old cut, and getImageFrameArea clamps an out-of-range index to the last frame rather than failing. So an animation whose image had been cut into fewer cells went on playing, silently, out of the wrong frames. Nothing in the log, nothing in the editor. One call to validateFrames() puts it right, and it is cheap: a pass over a list of tens of integers, on an asset's own refresh. The test needs a real image asset with a real texture behind it, which rules out a unit test -- loading a font or a texture with no GL context trips a modal assert that arrives as a hang. So it is a smoke suite, on the barbarian death animation: 25 frames drawn from the last five rows of a 10 x 10 sheet, which halving the cut puts entirely out of range. It fails two checks without the fix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
GuiTreeViewCtrl::drawIconFrame is three lines that have nothing to do with trees: given an image asset and a frame number, stretch that frame into a rect. GuiEditorExplorerTree already reached up into its base class for it to draw the eye and padlock icons, and the animation editor's frame palette and timeline will both want the same thing while being no relation to a tree at all. So it moves to guiDefaultControlRender as renderImageAssetFrame, beside the two functions a reader would otherwise find first and misuse. The comment explaining why this is not renderStretchedImageAsset moves with it and now sits directly under that function, where the contrast is visible: one reads its sheet off a profile and can only draw what a control is wearing, the other is handed the asset; one clears the bitmap modulation and would throw away a row's ink, the other leaves it alone; and one takes the frame as a U8, so it quietly cannot reach past frame 255 of a sheet that may have a thousand -- which the palette, pointed at whatever image the user chose, certainly can. No behavior change. treeIcons (11 checks) and explorerGutter (26) cover both call sites and are green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
PlatformFileIOTests.PathCopyAndRename has been failing, and it was failing on its first line, which hid two real bugs behind a third thing that was the test's own fault. The test's fault first: unitTestWriteFile opens a file for writing in a folder that does not exist yet -- the scratch root is deleted at the top of the case, deliberately, because pathCopy making its own destination is part of what is being tested. File::open does not make a path, it just fails. So the very first write failed and every assertion after it was unreachable. With that out of the way, two genuine gaps in the Win32 layer: pathCopy would not create the folder its destination sits in. The directory branch makes folders as it walks, so a tree copy worked; a single file copied into a new folder handed the path straight to ::CopyFile, which fails. The two halves of one function disagreed about whose job the path was. createPath has to run before the backslash conversion, because it splits on forward slashes only and would find nothing to make afterwards. And it would copy a directory into itself, recursing until the path outgrew MAX_PATH. The UNIX build has refused that since pathCopy was implemented there; Windows never got the guard. Ported across, comparing without case as a Windows path must and accepting either separator, since both names are backslashed by the time the directory branch sees them. Found while adding an unrelated test file, which is the only reason the run was looked at closely. All 230 unit tests pass now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
The animation editor needs to draw the same thing twice: the palette of every frame an image offers, and the timeline of the frames an animation plays. This is what they will share -- an image asset to draw from, and where the cells go. It has to be C++. Script cannot ask an image where a frame IS: in implicit cell mode getFrameSize is the only per-frame question with an answer, and a grid needs the source rect, which ImageAsset::getImageFrameArea has and no binding exposes. A grid of GuiSpriteCtrls was never an option. The layout is entirely static functions taking everything they use, so the renderer and the hit test call the same function with the same numbers and cannot drift apart -- the discipline GuiEditorExplorerTree's gutter uses, and for the same reason: a disagreement between where a cell is drawn and where it is clicked is experienced as clicking the wrong frame, which is a maddening bug to be told about and an easy one to test away. So there are 22 of them. The ones worth naming, because each is a mistake that would otherwise ship: getColumnsFor asks its question of a width one pad wider than the real one, because n cells span n advances LESS the gap the last one does not need. The naive width/advance loses a column at exactly the width that fits it. It also never answers zero -- a pane dragged narrower than one cell is ordinary, and zero is what the row arithmetic divides by. cellAt returns -1 for the gaps and for the empty tail of a short last row. A click on nothing must not become a click on the nearest something, and on the timeline the gap is where the insertion caret lives -- a different question with a different answer. getContentExtent leaves no trailing gap. It is what the scroller is told, and a pad of overshoot there is a scroll bar for a gap. Each subclass says which axis it grows along by overriding getDesiredExtent, one line each: a palette is as wide as its scroller and as tall as its rows, a timeline the other way about. Inferring it from the column count was tried first and read as a riddle. Named GuiEdit... so both copies of the palette-refusal rule refuse it by prefix and no icon table needs an entry; palette (152 checks) and inspectorSpec (114) confirm it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Two grids over the shared arithmetic, and almost nothing else in common -- which is why they are two classes rather than one with a mode flag. The palette derives its cells from the image and wraps them into rows; the timeline holds an editable list in one scrolling line. One appends on a click, the other selects and scrubs. Only one has a keyboard. A flag would have made every method an if. The palette's whole job is the fork between a click and a drag, five pixels of slop apart, which is the same fork GuiEditorControlTile makes. Both ends report to script rather than acting: what a dropped frame MEANS is the timeline's business. It also has to guard the double-fire -- a release that followed a drag is that drag ending, not a click, or a dragged frame would be both dropped where it was let go and appended to the end. The timeline holds a COPY of the list and never writes the asset, reporting one onFramesChanged per completed gesture. That matters more than the usual separation would suggest: AnimationAsset::setAnimationFrames has no equality guard, so every call rewrites the .animation.taml, and a per-drag-tick write would be hundreds of file writes for one reorder. Two things in it are worth reading twice. Repeats of one frame are drawn joined across the gap, because the asset format has no per-frame duration -- every frame gets AnimationTime divided by the count -- so naming a frame twice is the ONLY way to hold a pose, and a run of duplicates has to read as one held frame rather than as somebody's mistake. And insertionAt counts cell CENTRES, not edges, so the caret flips halfway across a cell where a person expects "before this one" to become "after it"; measuring from the edge makes the caret lag the pointer by half a frame. The drop and the caret call it with the same numbers, so what was shown is what happens. The playhead is read in onPreRender, which recurses from the canvas every frame -- the cheapest correct poll and the documented place to mark yourself dirty. It reads getAnimationFrame, the slot, not getCurrentAnimationFrame, the image frame: one image frame can fill several slots and the marker has to be on the one actually playing. Selection and playhead are drawn differently on purpose, an outline against a bar, because they are usually the same cell and scrubbing sets one by reading the other. Ten more layout tests, 32 in all. palette (152 checks) confirms all three classes are refused from the Gui Editor by their prefix. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
The animation editor needs the preview area divided into the art, the frames available and the timeline, with dividers a user can drag. This puts the frame set that will do it in place and changes nothing else -- the whole point of the commit is that the Asset Manager looks and behaves exactly as it did. That it can be invisible is what makes "build the nest always, split on demand" safe. GuiFrameSetCtrl::resize hands its one frame its own extent with no insets, so unsplit it is a pass-through; and splitting later never reparents anything, because splitFrame only rewrites which frame holds a control and removing one collapses the frame and hoists its twin. The background sprite, the SceneWindow, the scene and the audio overlay all stay exactly where they are for the life of the editor, whatever the stage does around them. previewHost is the layer that looks like a pointless wrapper. GuiWindowCtrl finds its dock target by casting its parent's FIRST child to GuiFrameSetCtrl; today that child is the background sprite, the cast fails, and window docking is quietly off in the Asset Manager. Making the frame set child zero would switch docking on by accident, aimed at the animation split -- so the Asset Inspector would offer to dock into frames the stage deletes out from under it. One plain control in between keeps the answer no. It is commented in place, because the next reader will otherwise remove it. assetLibrary (81), assetImageInspector (97) and assetPicker (63) are green, and the inspector's seven screenshots are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Choosing an animation asset now splits the preview three ways: the art playing on the left, every frame the image offers on the right, and the frames the animation actually plays along the bottom. Choosing anything else puts it back. AssetAnimationStage owns that. It is a ScriptObject rather than a control because what it manages is a shape -- two panes and the frame set they live in -- and because it gives the five asset kinds that have never heard of animation a single line to ignore: retainFor, called once above the selection chain, which keeps the split up for the asset it already shows and takes it down otherwise. Frames get into the timeline by dragging or by clicking, and both end in the same appendFrame. A click is a drop that never moved, and giving it its own path into the list would mean two places to remember to commit. Three things cost real time and are worth writing down. setFrameSize is the only thing that lays a frame set out, and a layout can only size the controls already in their frames. Sizing the frames before adding the panes produced a split with all the right frames and a palette still 100 x 100, parked behind the preview where nothing whatever could be seen of it. Both sizes now come last, after both panes are in. The two panes take fill on the axis their scroller cannot scroll and let the scroller own the other. That is legal exactly because the bar is alwaysOff there, and it is what lets the palette learn its real width -- which it must have before it can work out how many columns fit. It had been using "width", which preserves the gap it was built with, and the gap was wrong. And moving a divider resizes the SceneWindow, whose onExtentChange answers by re-clicking the selected tile -- which lands back in the stage. So building and collapsing are both shut for the duration. Without that, deleting the first pane re-entered select() while built was still true and the second call reached for a pane that was already half gone. Ids are cleared on teardown too, not just deleted: a field still holding a freed id will answer isObject() about whatever took that id next, and the symptom is a call into a live object that has never heard of the method. 30 checks in tests/smoke/assetAnimationTimeline.cs, including that the split collapses to exactly one frame for any other asset and that the preview window and its scene are the same objects throughout. Four screenshots, and the log is clean of script errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Every asset setter ends in refreshAsset, and the Asset Manager's answer to a refreshed asset has always been to re-click the selected tile -- which clears the preview scene and builds a new sprite. For an image or a font that is exactly right: the picture is a pure function of the asset. For an animation being edited it means dragging one frame restarts playback from the beginning, which makes the timeline useless for the thing it exists to do. So AssetBase::onRefresh now asks AssetAdmin::refreshPreview, which gives the animation stage first refusal. The stage takes the refresh only for the asset it is actually showing, keeps the scene and the sprite it has, and re-reads the values that moved. Everything else falls through to the old path untouched. Two engine behaviours make that harder than it sounds, and both are handled by the same short function. The engine has already restarted playback by the time script hears about it. AssetManager::refreshAsset notifies every AssetPtr pointing at the asset before firing the script onRefresh -- and for a sprite that notification IS playAnimation, from slot zero. So the slot is captured before the write rather than read after it, when the sprite has already forgotten. And playAnimation opens by clearing the pause, so a paused preview comes back playing and has to be paused again. The playhead is restored by SLOT, not by image frame. A slot's meaning shifts when something is inserted before it, so the preview can appear to skip a frame -- but tracking the image frame instead breaks the moment a frame appears twice, which is exactly what a hold is. A resize gets the same treatment: a divider moving used to rebuild the whole preview through onExtentChange, so the animation restarted every time the palette was widened. The sprite is already there and only its size is wrong. Also fixes the stage never learning about its sprite on a first selection: the tile displays before it selects, so displayAnimationAsset announced the new sprite while there was still no stage built to hear it. select() now asks for it rather than waiting to be told, which covers both orders. 38 checks, including that the sprite is the same object across an edit, that the playhead stays where it was, and that a change raised from outside the editor still reloads the strip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
The transport sits over the preview as an overlay, where the audio play button already sits and proof that one there receives clicks over the SceneWindow. It costs no layout and takes no room from the art. Five buttons, and three of them have to show a state, which is why the row is assembled from EditorToggleIcons by hand rather than from an EditorButtonBar of momentary buttons. Stop pauses rather than stopping, and that is not a shortcut. SpriteBase's stopAnimation sets the finished flag, updateAnimation returns immediately on it, and setAnimationFrame goes through updateAnimation -- so a preview stopped that way can never be scrubbed again. Pausing halts it just as visibly, leaves the playhead where you stopped to look at it, and keeps every other gesture alive. armPreview is the way back for a preview that finished on its own, and every path that moves the playhead goes through it. The suite asserts that directly: stop the animation the engine's way, then scrub, and the scrub still works. Loop is the asset's AnimationCycle, so it writes the file like any other edit. Keep-frame-rate is not: it decides what the EDITOR does on the user's behalf, so it is a preference and it is remembered. Uniform timing means AnimationTime is shared out over however many frames there are, so adding one makes every frame play faster and the animation stops lasting as long. Which of those a person wants depends on whether they are lengthening a walk cycle or dropping in a hold, so it is a switch, off by default, and both numbers are always on show. The range builder is a plain object with no dialog attached, because it is the part worth checking and the dialog's feedback line is its own answer read back rather than a second description that could drift. Three stages, and the order is the design: the stepped run, then the ping-pong reverse MINUS both shared end frames, and only then the hold. Hold last is what makes "shared" mean one frame rather than N slots -- keep the ends and the turn at each end lasts twice as long as everything else, which reads as a stutter. Eight rows of table assert it. Two script-only stumbles worth recording: getMax and mClampF do not exist in TorqueScript. The first is a link error you see; the second silently returns nothing, which setAnimationTime then wrote as zero -- and a zero animation time divides by zero in the playback integrator. It is spelled out by hand now, with a note that mClamp would round a sub-second animation down to nothing. 61 checks, and six screenshots including the dialog with a ping-pong read back before it is applied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Animation assets got the generic GuiInspector: General, SimBase, Namespace Linking, Dynamic Fields, and somewhere among them the things that actually matter. They now get the same treatment image assets got -- three blocks that reflow, only the fields worth showing, and a line saying what the numbers add up to. The most important thing about the pane is a field that is not in it. AnimationFrames is the timeline, in the editor above; a box of space-separated numbers beside a timeline editing the same list is two sources of truth, and the box is the one that cannot say which frame 67 is. Named frames are absent for a harder reason: the engine's named-frame API does not round-trip through its own file, so an asset in that mode keeps the generic inspector rather than being offered a pane that would quietly lose work. The info line is the part the separate fields cannot say: "25 frames, 2.08 s, 12.0 per second. Frames are 96 x 96, from ToyAssets:TD_Barbarian_CompSprite (1024 x 1024, 100 frames)." Uniform timing means the rate is a consequence of two other fields, so it has to be shown rather than worked out. Four warnings, and the one worth reading twice compares the specified frame list against the validated one. That is the only comparison script can make and it is exactly the right one, because the engine CLAMPS an out-of-range frame to the last one instead of dropping it -- so the animation goes on playing and shows the wrong art with nothing said. It needs the validateFrames fix from earlier in this branch to have anything to compare against. chooseInspector was a boolean with two literal isVisible() tests reading it back from the far side of the file. It is a key registry now: registerPane names a pane, chooseInspector shows one and unbinds the rest, and activePaneObject is the single accessor the other two went through. imageScroller and imagePane stay as named handles onto it -- assetImageInspector names them in eight assertions, and that suite passing unchanged at 97 checks is the proof this refactor preserved behavior. The five copies of the same four addHiddenField lines are now one inspectStock. 48 checks. Two things needed a second look and are worth knowing: addFieldRow takes the label and kind as arguments rather than asking labelFor/kindFor, so rows built without them come out captionless; and makeInfoLabel gives one line of 20 pixels, so a sentence needs textWrap and textExtend or it is not drawn at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Two suites, split by what actually needs a pointer. The drop path goes in the timeline suite, driven by calling the callbacks with a payload parked at real coordinates. That is not a shortcut around the interesting part, it IS the interesting part: GuiDragAndDropCtrl hit-tests from its own parent and findHitControl answers "me" without ever testing its bounds, so a drop anywhere on the screen arrives at the timeline and the boundary check is the timeline's own to make. The suite drops a frame over the asset library and asserts nothing changed, then drops one on the left half of a slot and asserts it went in before that slot -- the caret's promise, kept. The palette-to-timeline DRAG is deliberately not tested with posted input, and the reason is written down rather than left as a gap: a GuiDragAndDropCtrl gesture follows the real cursor, which a posted WM_MOUSEMOVE does not move. What would be proved is the engine's capture, not this feature's code -- and this feature's share of it, the boundary policing and reading the cursor back off the payload, is script and is covered above. What does need real clicks is the touch path in the two grids, which is new code: the press, the five pixels of slop that decide click from drag, the capture taken and given back, and the suppression that stops a released drag also counting as a click. So assetAnimationClick posts two, and like explorerGutter it is handed the points rather than knowing them -- where a cell lands depends on how many columns the palette wrapped into and how far its scroller sits, and a hard-coded point that drifted off the cell would report a control that never fired, which is exactly what a broken hit test reports. All 51 smoke suites and 240 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Feedback from using it, and one of them turned out to be a real bug rather than a rough edge. The palette had no scrollbar, so the only way to reach a frame near the bottom was to shrink the cells until they all fitted. Two causes. The wheel handler zoomed instead of scrolling, and by taking the event it stopped the wheel ever reaching the scroller -- a wheel over a scrolling list of pictures means scroll, so that handler is gone and cell size stays a field a pane sets. And the vertical bar was dynamic, which has to be decided from the strip's height during the very layout pass in which the strip is working that height out; a sheet worth opening the palette for has more frames than fit, so it is simply always on. The timeline drew a yellow bar and a blue box out of hard-coded colors, which ignored the theme. Every color now comes off the profile: HighlightState fill for hover, SelectedState for the picked cell and for the run joining a held frame, DisabledState for a cell that dragging further would discard, and the selected and highlight FONT colors for the playhead and the caret -- inks rather than fills, so they stay legible on the cell they sit on. Backgrounds moved to behind the art rather than over it, which they had to: a theme's fills are opaque, and a hover painted on top hid the frame the pointer was hovering over. Play and Stop are two buttons with one hidden rather than one toggle. A toggle says "this setting is on"; these say "here is what pressing me will do", which is what a transport means -- and it is why the button could not get stuck. Play is half again the size of the rest, and the order now reads rewind, play, gap, then the three that are settings. The toggles drew their icons at 16 against the push buttons' 20, so EditorToggleIcon takes an iconSize now, defaulted to what every existing caller already gets. The Stop button stayed showing after anything other than the Stop button halted the preview -- clicking a slot to scrub, dragging a frame off the timeline, or a one-shot animation reaching its end. Every one of those goes through the stage, so the stage tells the bar. And the two captions used labelProfile, which is meant for text on the window background: near-black on dark blue under Lab Coat. panelProfile is what the Asset Inspector's own title bar wears and its font is the theme's color5. The bug the tests caught while fixing the rest: with keep-frame-rate on, a commit writes the asset TWICE, and the remembered playhead was being cleared between the two. The second refresh then fell back to the strip's cached marker -- a value onPreRender updates once a frame, so mid-script it is whatever the last drawn frame said -- and scrubbed to it, undoing the restore the first refresh had just made. The playhead went back to zero after every edit. One slot now covers both writes, and the fallback is gone: a refresh this editor did not cause has nothing to restore and should leave the preview alone. 73 checks in the timeline suite now. All 51 smoke suites and 240 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
…sizes Both widgets hard-coded their numbers, and a caller wanting a bigger button had no way to ask. Setting the sizes afterwards did not work either: EditorIconButton forces its own extent in onAdd, and its hover handlers animate the icon to numbers of their own, so anything a caller set survived exactly until the pointer first crossed it. They take buttonSize and iconSize now, defaulted to what every existing caller already gets. What took three attempts to get right is what those two names mean. GuiSpriteCtrl::growTo animates mImageSize -- the PICTURE -- and leaves the sprite control alone. So there are three numbers, not two: the button, the sprite holding the picture, and the picture itself. Conflating the last two is what made a 36 pixel button animate its icon from 32 down to 28 on first hover, which looked like the icon exploding and never recovering. iconSize is therefore the picture. The sprite holding it is deliberately larger, because a sprite clamps its picture to its own content rect -- a holder the same size as the artwork loses a pixel or two of it to the profile's insets, and the symptom is subtle and awful: the icon comes up small, the hover appears to grow it, and it stays grown. The slack is what stops that, and there is a comment telling the next person not to tidy it away. The defaults reproduce the original numbers exactly: a 24 button, a 20 sprite, a 16 picture that goes to 18 under the pointer. EditorToggleIcon gets the same arrangement and the same meaning for iconSize -- the two are frequently sat next to each other and had no reason to disagree about what a size is. headerPane, toggleTip, profileForm, menuBar and the rest of the suites that drive these two are unchanged and green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Four things wrong with the animation editor, and the first was the one that mattered. A frame dragged out of the palette always showed frame 0 under the cursor, and only became the right frame once it was dropped. GuiSpriteCtrl::setImage returns early when the control is not awake -- it keeps the asset id and throws the frame away -- and a drag payload is built detached, so the frame never landed. onWake then re-applies the image from the Frame FIELD, which nothing had set. Setting the fields is the fix; setImageFrame after the payload is on the canvas makes it right whichever order the waking happens in. The test reports 0 without it. The timeline was drawing hard-coded colors, so it ignored the theme. Borrowing listBoxProfile did not work either, because three of its fields mean something else there: a list row's hover is deliberately a whisper, which over a picture is no change at all, and its selected FONT color is the ink drawn ON a selected row, so the playhead bar was dark-on-dark and could not be seen. So the grids get frameGridProfile, which exists for this and says in BaseTheme what each of its six colors is for. The accent goes to the playhead -- the one thing that has to be findable while the animation runs -- and the selection is a quieter raised surface, so the two stay legible when they land on the same cell, which while scrubbing is most of the time. Backgrounds moved behind the art rather than over it, because a theme's fills are opaque and a hover painted on top hid the frame the pointer was hovering over. The transport bar was clipping its big play button, and the cause is a one-line-of-difference bug worth knowing about: a GuiChainCtrl is born VERTICAL, and its resize refuses to change whichever axis is currently the length. Extent was being set before IsVertical, so the height was rejected, the bar stayed at the constructor's mEditOpenSpace of 30, and a 36 pixel button was centred in it -- three pixels off each end. IsVertical now comes first. A chain never grows to fit a taller child, so the height is stated and commented as such. The toggles looked smaller than the push buttons at the same extent, because they are: a GuiButtonCtrl paints across its whole rect and a GuiCheckBoxCtrl paints a box that onRender clamps into the CONTENT rect, inside the borders. With a 2 pixel border all round that is 24 against 20, and no boxExtent can fix it -- the clamp will not let the box out. The toggle is built that much bigger instead, and the amount is read from the profile rather than written as 4. And choosing an animation arrived showing Play over a preview that was already running. A sprite built with an Animation on it does not wait to be started, so the playing state is read off the sprite now instead of being assumed false. The palette also lost its wheel-zoom. It consumed the event, so the wheel never reached the scroller and shrinking the cells until they all fitted was the only way to reach the frames at the bottom. 85 checks in the timeline suite. All 51 smoke suites and 240 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Editing an asset wrote its file. Every setter ended in refreshAsset, and
refreshAsset ended in Taml::write -- so dragging a particle graph key was
indistinguishable from deciding to keep it, and there was no way back. It was
also wrong outside the editor: a running game rewrote its own content as a side
effect of a setter.
refreshAsset now marks the asset unsaved and announces the change. saveAsset
writes, and nothing else does. The private-asset branch was already exactly that
shape, so the two paths are one.
Around that: Save, Revert, Duplicate and Undo/Redo on the inspector's title bar,
a badge on unsaved library tiles, and one Save All / Discard All / Cancel prompt
in front of Close Project and Exit. Any number of assets may be unsaved at once;
switching between them deliberately asks nothing.
The dirty flag and the snapshots are in C++ because two of the editors never
reach TorqueScript -- GuiParticleGraphInspector does every key drag itself, and
the stock GuiInspector writes emitter fields straight onto the object. A recorder
built out of script writes would have been blind to the particle editor, which is
the thing that most needed undo. Script keeps only the policy: what counts as one
step and what it is called.
Undo is whole-asset snapshots of unowned clones. Unowned is the point: with no
owning manager every setter is inert, so taking one marks nothing, notifies
nobody and loads no bitmap. A restore copies onto the LIVE object, never
replaces it -- every AssetPtr holds a raw pointer that a swap would null.
That rested on copyTo, which was broken in four places, all of them live bugs
today via clone() and acquireAsset(id, true):
ImageAsset copied cell COUNT into cell OFFSET, never copied image
layers, and dropped explicit cells unless ExplicitMode
happened to be on
AnimationAsset chose between numbered and named frames by reading the
TARGET's mode, still the default at that point
ParticleAssetEmitter same shape, so an animated emitter copied as a blank
static one -- and ParticleAsset inherited it
Fixed by not listing fields at all: AssetBase::copyTo walks the field table via
copyFieldsFrom, and each type overrides copyAssetStateTo only for what no field
describes. assetStateCopyTests enumerates the field table rather than a list of
its own, so a field added later is covered the day it lands.
Also fixed on the way through, each found by the work above:
- a named-cells animation did not survive its own file. The vector type's
getter joins with commas; the setter split on whitespace alone.
- setAnimationFrames had no "ignore no change" guard, so writing the same list
back counted as an edit and left an undo step that put nothing back.
- a dropped frame committed twice: insertFrameAtPoint announces itself and the
handler announced it again. Two presses of undo to remove one frame.
- refreshAsset's onRefresh callback never reached dependents, contradicting
what the editor assumed. It fires from the manager now, with a flag saying
whether the asset was changed or merely reads from something that was.
- the dependency and loose-file graphs were rebuilt by re-parsing the file that
had just been written, so with no write they went stale. Rebuilt in memory.
- unloadAsset would delete an asset holding unsaved work.
- preloadAsset marked every preloaded asset unsaved at startup.
- findAssetPrivate's five-argument binding called findAssetInternal.
- the Frame Range dialog's Mode list was filled with GuiControl::add, so it was
empty, read "none", and Replace could not be picked.
- three dialogs placed their buttons using the window's height rather than the
content's, which is 34 less; the buttons sat below the fold.
Asset ids are resolved to modules by path, never through
AssetDefinition::mpModuleDefinition -- the editor calls clearDatabase() when it
picks up a project, which frees every ModuleDefinition and leaves that pointer
dangling.
252 unit tests and 52 smoke suites pass. assetDirtySave is the new one, and most
of what it asserts is about the FILE rather than the object, because not touching
it is the whole point.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
File, Edit, Layout and Select were written into the shared bar in EditorCore and
switched on when the Gui Editor opened. Every command in them named GuiEditor.
The Asset Manager grew the same class of features last commit -- save, revert,
duplicate, undo, redo -- and could not join them: setMenuActive matches by item
TEXT across the whole tree, so a second editor toggling Undo or Delete would
flip the first editor's items too.
So each editor owns its menus now and lends them to the bar for as long as it is
the one open. The bar always reads Torque2D | the open editor's menus | Theme.
Nothing became generic: the Gui Editor's item still says "Save Gui..." and runs
GuiEditor.SaveGui(), the Asset Manager's says "Save Asset" and runs
AssetAdmin.inspector.SaveAsset(), and Ctrl+S means both. The Console and the
Project Manager show the two permanent ends and needed no code at all -- the
outgoing editor's close() clears the bar, where before they sat behind four
greyed-out menus that would never do anything.
The Asset Manager's File carries New Asset (five kinds), Save Asset, Save All
Assets and Revert Asset; its Edit carries Undo, Redo, Duplicate and Delete, with
Undo and Redo naming the step the way the document bar's tooltips already did.
Revert, Delete and the five New items have no accelerator, for the reason the Gui
Editor's Revert has none.
Four engine facts shaped the mechanism, and they are worth knowing before
touching a menu again:
build into the bar GuiMenuItemCtrl learns which bar it belongs to when it is
added to one, and a submenu learns it from its parent when
IT is added; nothing back-fills it. A tree built standalone
and handed over whole leaves every descendant with no bar,
and openMenu dereferences it. The old literal only worked
because the VM adds a parent to its group before compiling
its sub-objects. EditorMenuSet::addMenu returns an
already-attached empty menu, so the rule is the shape of
the code rather than something to remember.
append only onChildAdded links the sibling chain the keyboard walk
follows by taking end()-2, and childrenReordered rebuilds
the layout but not the chain. So the fixed Theme tail comes
off and goes back on around every swap.
move, never remove SimSet::remove leaves a control registered with no group at
all. Each set parks its menus in a SimGroup of its own.
rebuild accelerators The canvas keeps one flat global list, rebuilt only when a
dialog is pushed or popped. A tab change is neither.
That last one was a bug already, not a new hazard. GuiMenuItemCtrl::onAction
checks its own active flag and never its parent menu's, and buildAcceleratorMap
filters nothing -- so with File greyed out, Ctrl+N still ran GuiEditor.NewGui()
from inside the Asset Manager. Physically removing the items is what fixes it,
plus a GuiCanvas::rebuildAcceleratorMap() for the moment nothing else notices.
setContentControl keeps its own walk deliberately: it descends until it reaches a
control that takes input, and sharing that with the dialog paths would leave the
editor's shortcuts live underneath an open dropdown.
setMenuActive is now used nowhere in editor/. It was a text-compare walk of the
whole tree PLUS a full profile re-apply per top-level menu, fired 22 times per
selection change; greying is item.setActive() on a held handle. That deleted the
menuUndo/menuRedo/menuPaste caches and both forceRefreshMenu twins, which existed
only to dodge that cost. Items answering one shared question -- thirteen on "is
anything selected" -- are groups instead, so toggleMenuItems is four calls.
Two bugs found on the way, both caught by tests:
- a group's count starts unset, and using "" as an index writes to a slot
nothing reads back. The FIRST item of every group silently stopped greying:
Cut, Align Top, Space Vertically, Bring to Front. The clipboard suite caught
it before the code ran anywhere else.
- AssetInspector::documentAsset deduced the asset from the bound pane, and
every load method binds its pane AFTER calling beginDocument -- so the
refresh that follows a load had nothing to ask. It also never cleared, so
with nothing selected it answered with the asset before last. Neither showed
while only the document bar asked, because the bar is hidden in exactly those
moments; the menus never are. It returns what beginDocument was handed now,
which settles the particle case directly (an emitter has no file; its owner
does) and let deleteAsset drop its own copy of the dropdown logic.
tests/smoke/menuSwap.cs covers the bar's order, the two same-named File menus
told apart by object rather than by text, parked menus staying alive in their own
editor's group, the Theme radio group surviving the round trip, and the group
registry. Its input script presses Ctrl+N in both editors: once where it must
reach nothing and once where it must make a new document, because a shortcut that
reaches nothing and a shortcut that was never pressed look identical from inside.
Remove the rebuildAcceleratorMap call and it fails with the sentinel wiped.
53 smoke suites and 252 unit tests green; tests/shots/menuSwap.cs is the visual.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Font assets get an inspector pane of their own in place of the stock GuiInspector: three reflowing blocks, and a line saying what came out of the .fnt -- the native size, the glyph count, and the pages at the size they actually loaded at. That readout is the point. A FontAsset registers exactly one field of its own, so a pane that merely rearranged fields would not have earned its place. What it adds is the two ways a bitmap font goes wrong silently: a .fnt that did not parse, and a page image named inside the .fnt that is missing. Both were a line in the console log and nothing on screen. None of it could be put on screen until BitmapFont could be trusted: - the constructor had an empty body, so mSize, mLineHeight, mBaseline, mWidth, mHeight and mPages were whatever was on the heap. The last two are the divisors ProcessCharacter uses to turn glyph rects into texture coordinates, so this was worse than cosmetic. - buildFontData returned on a failed open without clearing anything, so an asset pointed at a missing file kept the glyphs of the font it used to have, and "did not load" was undetectable from outside. - nothing ever cleared mChar or mKerning -- only the page list and the textures -- so pointing an asset at a SECOND .fnt left the union of both fonts and a glyph count that only ever grew. Re-pointing that file is exactly what this pane makes easy for the first time. getRelativeFontFile mirrors ImageAsset's: the field holds the expanded absolute path, which is neither readable nor portable in a text box. The read-only queries behind the info line are new bindings. BitmapFont needed one accessor for the glyph count and deliberately none for mWidth/mHeight, which are what the .fnt declares rather than what loaded. Two things the shared pane grew here and the sound pane will use next: tipFor, because a field's registered doc string is empty on nearly everything, and fileFilters/fileTitle on EditorFieldRow, because a "file" row had been a bitmap everywhere and an image filter offers a font asset nothing it can choose. bitmapFontParseTests drives the parser directly. It touches only the console, the string table and the stream, so unlike anything reaching buildFontData -- which loads page textures through TextureManager -- it runs without a GL context. assetAnimationInspector was asserting the pane registry's exact contents, and was checking the read-only name row with row[...].isEnabled(). There is no such binding anywhere, so that logged "Unknown command" and had been passing vacuously; it asks the box now, as the image suite already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Audio assets get an inspector pane of their own in place of the stock GuiInspector: three reflowing blocks, with the length and format of the file sitting under the file itself, where there was room for it. Half of what this pane adds is the tooltips. An AudioAsset registers six fields and gives every one of them an empty doc string, so the generic inspector offered six labels and explained none of them -- least of all VolumeChannel, whose numbering is a per-game convention the engine attaches no meaning to. AssetAutoUnload is left out, and not because it is uninteresting. AudioAsset::initializeAsset calls setAssetAutoUnload(false) unconditionally, so every audio asset reads false whatever its file says and a tick would silently come back off on the next load. A checkbox that cannot be changed is worse than no checkbox: it invites the attempt. The preview no longer goes through alxPlay, which is the fix for a real complaint: turn a game's music channel down to nothing and every music asset in the library became unplayable in the editor. Not quiet -- unplayable. alxCreateSource refuses to build a source at all on a muted channel, so there was no handle, and nothing to distinguish that from a broken file. alxPlayPreview keeps the asset's file, looping and streaming flags but forces full volume on a reserved channel and writes AL_GAIN directly to get past the master volume. It never touches the game's own channels, so auditioning an effect cannot blast the music playing behind it. That write survives because a 2D source is AL_SOURCE_RELATIVE, which is exactly what alxUpdateMaxDistance skips each frame; only alxUpdateTypeGain would recompute it, and that runs when somebody moves the mixer, which while a preview is playing is a thing they meant to do. The game still owns the audio driver, and nothing here changes that -- a shipped game must not depend on anything under editor/. But the Asset Manager can be opened before a project is picked, or against one with no audio module, and there the driver simply is not running. AssetAdmin::ensureAudioDriver covers that case and no other. It asks OpenALIsInitialized first, which is new and is not a nicety: OpenALInit BEGINS by calling OpenALShutdown, so starting a driver that is already up drops every playing source and resets the channel volumes, silently undoing the project's own SetMusicVolume. Two engine bugs found on the way: - setVolume and setVolumeChannel compared the value they were handed against the one they held and clamped only afterwards, so handing either an out-of-range number read as a change every single time -- calling refreshAsset and marking the asset unsaved for an edit that moved nothing. - alxGetAudioLength acquired the asset and released it on none of its three return paths, so every call raised the reference count for good. The pane clamps in writeField as well, which is how it avoids the first of those rather than relying on it. Note mClamp, not mClampF -- the latter is the C++ name and is not bound to script at all, so it returns an empty string and quietly writes a zero. Nothing warns about the channel being muted, deliberately. mAudioChannelVolumes is a static global filled in only by OpenALInit, so before the driver starts every channel reads zero and such a warning cannot tell "somebody muted this" from "nothing has made a sound yet" -- and with the preview no longer caring about the game's mix, it would be warning about something that no longer affects what the user is looking at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Particle assets get inspector panes of their own in place of the stock
GuiInspector, and were the last editable asset kind still on it. Two panes,
swapped by the emitter dropdown that already sat in the title bar: a small one
for the effect, and a five-block one for an emitter.
The emitter pane is the point. A ParticleAssetEmitter registers about thirty
persistent fields and a large fraction of them are inert depending on four or
five of the others -- a single-particle emitter ignores ten, a POINT emitter
ignores its size and its angle, a fixed-aspect one ignores every Size-Y curve.
The generic inspector listed all thirty flat and alphabetically, so the one
thing it could not tell you was which knobs were live.
Two ways of saying "this does not apply", and the difference is deliberate.
Alternatives SWAP: an emitter draws an image or an animation and never both,
and an orientation has exactly one offset, so showing the other arm invites
filling in both when one would silently win. A field that is real, holds a
value, and is merely unread by this mode is GREYED, with the reason as its
tooltip -- hiding those would lose the value from sight and make the pane jump
about as you tried modes.
The blocks are five rows each, and that is a constraint on the grouping rather
than an outcome of it. A GuiGridCtrl row is as tall as its tallest cell, so
unequal blocks do not give a short column and a long one -- they give columns
of the same height with the short ones mostly empty, which is what makes a wide
layout read as unplanned. The first cut was 7/4/2/6/6. Three fields moved to
fix it and each reads better where it landed: aiming joined orientation (both
answer "which way", where emission answers "where"), PivotPoint joined it too
(it is the point a particle is rotated about), and AlphaTest went to Particle
Image (it is a threshold on that image's own alpha). The emitter's name is a
header above the grid, not a sixth block -- the same failure in miniature.
SELECTING A PARTICLE ASSET CRASHED THE EDITOR BEFORE ANY OF THIS, on unmodified
HEAD. beginDocument snapshots for undo, copyFieldsFrom walks the whole field
table, and every numeric-frame emitter's empty NamedFrame therefore reached
ImageAsset::containsNamedRegion -> dStrcmp(mRegionName, ""). PixelArea's default
constructor was empty and its four-argument setArea never set mRegionName, so
every frame of every ordinary cell-mode image carried an indeterminate pointer.
Initialised now; containsNamedRegion refuses an empty name, which it must
independently, or an empty name would MATCH an unnamed frame and flip an emitter
into named-frame mode; and setNamedImageFrame refuses one too.
Five more engine defects, all confirmed by reading the render path:
- the emitter's BlendMode, SrcBlendFactor and DstBlendFactor round-tripped
through TAML and were read by nothing. sceneRender used mBlendMode and the
two factors, which ParticlePlayer does not declare -- they resolved to the
inherited SceneObject members, so one setting on the player covered every
emitter and the per-emitter fields did nothing. IntenseParticles still
overrides, as it always did.
- quantityVaritationField was initialised from getQuantityBaseField, so the
QuantityVariation graph did nothing and every emitter got a spurious
half-base jitter instead. This one is content-visible: bonfire emitted 5 to
15 per interval and now emits ten.
- the console setEmitterAngle stored mDegToRad(angle) and its getter handed
back mRadToDeg(stored), so the two agreed with each other and with nothing
else. The persist field writes what it is given and configureParticle does
mDegToRad(getEmitterAngle()), so degrees is what the file and the renderer
both mean.
- AlphaChannelScale was sampled at time zero, which reads its first key and
discards the curve the Scale Graph tab exists to draw.
- setTargetPosition has no refreshAsset and DELIBERATELY so -- AngleToy steers
an emitter at the cursor with it on every mouse move, and a refresh there
rebuilds every emitter node per frame. Commented in place so nobody corrects
it; the pane asks for the refresh itself.
getFieldValue was the expensive one. ParticleAsset and ParticleAssetEmitter
each declared getFieldValue(time), the graph sampler, which SHADOWED
SimObject::getFieldValue(fieldName) -- the call the whole editor reads rows
with. Asking an emitter for EmitterName sampled whichever curve was selected at
dAtof("EmitterName") == 0 seconds and returned a plausible 1.0. Every row on
both panes showed "1" while a hundred and forty assertions passed, because they
checked the asset and writes were never shadowed. Renamed to
getFieldValueAtTime; no script called it, the graph editor being C++. The suite
now asserts what a row SHOWS, not only what the object holds.
The preview grew a transport: play/pause, stop, restart, a cycling speed, and
per-emitter solo and switch-off. Its chrome came out into EditorTransportBar in
EditorCore, shared with the animation bar. Solo and switch-off are player state
and never touch the asset -- but every edit rebuilds the preview, so the bar
re-applies them keyed on asset id and resets only when the asset really changes.
Stop is the immediate form: the graceful one leaves mPlaying set until the last
particle dies AND pauses every emitter, which is the flag solo writes in.
isStaticMode is a new binding because the mode cannot be inferred from the
assets: an emitter switched to animation before an animation is chosen is
animated holding nothing, which is indistinguishable from static holding
nothing. EditorFieldRow gained a per-row assetType, having hardcoded ImageAsset
since the animation pane; the emitter has an image row and an animation row side
by side. The emitter button bar was rerouted off inspector.getInspectObject,
which stops answering the moment index 0 leaves the stock inspector, and its
remove-last path no longer asks for getEmitter(-1).
assetParticleInspector drives both panes, the gating, the chrome and the
transport, and counts visible rows per block so the balance cannot rot. Its
screenshot harness is what caught the shadowed reads. Four cases went into
assetStateCopyTests for the parts reachable without a canvas.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
Choosing an animation asset while any other kind was selected left the preview showing the previous asset. The split, the timeline and the transport bar all came up correctly around it; only the picture was of the wrong thing. AssetWindow::onExtentChange answers a resize by re-clicking the selected tile, which is how the preview refits when a divider moves. But a tile does not record itself as the selected one until the LAST line of its onClick, and choosing an animation is the one selection that resizes the preview from inside that call: AssetAnimationStage::build moves two dividers to make room for the palette and the timeline. So the sequence was -- display the animation, start building the split, get resized, re-click the tile that is still recorded as selected, which is the PREVIOUS one, and have it clear the scene and repaint its own asset over the animation sprite made a moment earlier. select() then resumed and adopted a sprite id that no longer pointed at anything, and stopped quietly. Which is why the two cases that worked did: an animation chosen as the first tile of a session has no previous tile to re-click, and animation to animation never builds a split because one is already up. The window now asks the stage for first refusal on a resize rather than calling resizePreview itself. While the stage is busy -- putting a split up or taking one down -- the resizes are its own doing and it says so, and the selection that started the rebuild paints the preview itself either side of them. That uncovered a second bug the first had been hiding. The sprite is measured against the whole preview area, because it is made before the split exists, and nothing put it right afterwards: it came out at 38.4 units where 26.7 fits. The re-click had been rebuilding it at the correct size by accident. select() now resizes it once, at the end, with the resizePreview a divider drag already uses. tests/smoke/assetPreviewSwitch.cs walks image -> animation -> font -> animation -> the same animation again and checks what the preview scene actually holds at each step. On the old code five of its twenty checks fail, and the report line of the first reads "100 object(s): Sprite(ToyAssets:TD_Barbarian_CompSprite)..." where one animation sprite belongs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
An emitter's color is four curves -- RedChannel, GreenChannel, BlueChannel and AlphaChannel, each 0 to 1 over a normalized lifetime -- and the Emitter Graph tab offered them as four list entries opening four separate graphs. Every color was already reachable that way. What was not reachable was the question anyone actually opens them to ask: not "what does red do" but "what does this LOOK like over its life". Nobody answers that from three pictures and a mental model of additive mixing. Red, green and blue are now one "Color Channel" entry: the three curves layered on a single plot, with a strip under it showing the color they mix to across the particle's life. Alpha keeps its own entry and its own ordinary graph -- it is not a hue, and folding it into the strip would darken every reading of one. THE LIVE CHANNEL IS THE PARENT'S TARGET FIELD, and that one decision is why this is small. Three toggles down the left pick which channel a click edits; setting one calls setDisplayField, so the hit test, add, delete, drag, the refreshAsset on release -- and therefore snapshot undo -- are all GuiParticleGraphInspector's existing code, unchanged and unduplicated. GuiEditParticleColorGraph adds two things and only two: the other two channels drawn dim and read-only, and the strip. The other two are drawn at the same hue and a lower alpha rather than a darker shade, so they read as sitting behind the live curve rather than as three more colors; dglDrawLine blends, so the grid still shows through them. The strip is exact rather than sampled. Between two consecutive members of the union of the three channels' key times every channel is linear, so one interpolated quad per span is not an approximation of the gradient, it is the gradient. dglDrawBlendRangeBox looks like the tool for that and is not -- its stops are spaced EVENLY across the rect, so it cannot put a stop at 0.13 -- so this is one dglDrawBlendBox per span, with the pixel floored rather than rounded because rounding a pair of adjacent spans independently overlaps them by a pixel and a pixel of overlap between two opaque quads is a visible seam. It samples the key arrays and NOT ParticleAssetField::getFieldValue, which applies a RepeatTime warp and a ValueScale that the plotted curve ignores, and which reads key zero before checking there is one. Where those differ the strip has to agree with the picture directly above it rather than with the runtime, because that is what an editor is for. Neither honors them; said so in the class comment rather than leaving it to be discovered. The parent became a template method to make room: getUnderPlotBandHeight asks for a band between the plot and the x axis labels, renderUnderlay draws behind the curve, renderUnderPlot draws into the band. The layout arithmetic came out into two statics, and getUnderPlotReserve(0) == 0 is the invariant that keeps every existing graph pixel-identical -- it is the first thing the unit tests assert. Two orderings in there are load-bearing and commented as such: the band is reserved BEFORE the rect is snapped to the grid and placed against the snapped rect afterwards, because the snap moves the plot by up to nine pixels; and the offset-changed dirty check moved above the underlay hook, because renderPoints clears mDirty halfway through the frame and the strip is drawn after that. Anything the subclass cached on mDirty alone would have shown the previous frame's color on every frame the user was dragging. The toggles are EditorToggleIcon, whose refresh gained a getIconTint hook. The stock toggle tints with the editor's own inks, bright for on and dim for off, which is right for a switch -- but these three stand for red, green and blue, so the color IS the label and no theme can restyle it without lying. Fixed hues lifted off the primaries, matched to the curve each one controls. That hook is also why this needed no new theme profiles at all. ZOOM WAS DEAD ON EVERY 0-1 FIELD, and had been. ParticleGraphCameraController builds its levels by asking whether max > 1, > 10, > 100, so a field whose max is exactly 1.0 got one level and both zoom buttons answered "no" -- on all four color channels, on both axes. Unit-range fields now get four window widths (1 / 0.5 / 0.25 / 0.1), the last of which is the whole field, which is what makes zooming out unable to go past 0-1. Alpha had the same dead buttons for the same reason and gets the same fix; two 0-1 graphs sitting beside each other should not behave differently. Then the axis labels ate the graph. setDisplayArea kept the caller's string as the label, script hands it a float, and a script float is an F32 printed with "%.9g" -- so a tenth arrived as the eleven-character "0.100000001". The y labels are the entire reason the plot gives up a left margin, so the tightest zoom was spending a third of its width on rounding error. The label is now printed from the value it parsed; the window keeps the value, so nothing the camera computes has to agree with what is drawn to the pixel. Six engine defects fixed on the way through, all reachable before any of this: - getTargetField walked off an emitterless asset. mEmitterIndex = getEmitterCount() - 1 on an unsigned zero is 0xFFFFFFFF, getEmitter warns and returns NULL, and the dereference came BEFORE the AssertFatal meant to catch it -- which compiles out of shipping entirely. Routine here rather than exotic: the color channels exist only on emitters. Now findField, which returns NULL and warns, with guards at all five call sites and the empty-list guard in renderPoints that has to land with them, since the tail there indexes count - 1 unsigned. - the key repair deleted the key it had just inserted, given a first key at a negative time: addDataKey inserts in time order and refuses nothing below mMaxTime, so the new key at zero landed at index 1 and removeDataKey(1) took it straight back out. - the same loop skipped a key after every removal, continuing without stepping i back. Two adjacent bad keys left one behind for a frame. - setDisplayField(name, index) reset the selected point when the field name changed but not when the EMITTER did, so the same channel on a different emitter kept an index into the old key list. Exactly the color graph's normal usage. - dglDrawBlendBox's mobile and web path left GL_COLOR_ARRAY enabled pointing at squareColors, a stack local, so every later vertex-array draw read a dead frame for its colors. Its neighbours in the same file disable it. - initEmitter read %itemWidth, a local of init(), so variGraph and lifeGraph were built with a malformed Extent and positioned all sixteen of their buttons against it. The grid resizing cells afterwards is what had been hiding it. mGridRect and mCalculationOffset were uninitialized and onTouchDown reads the first; findHitGraphPoint returned -1 from a U32 and worked only because the wrap round-tripped through an S32. Both corrected in passing. guiParticleColorGraphTests covers what has no canvas: the reserve invariant, the grid snap including a negative extent that used to become an unsigned four billion, channel sampling, the three-way merge (cross-channel de-duplication, a window that clips, a stop budget that runs out and must still reach the far edge), and the joint property that a strictly increasing stop list produces pixels that never decrease at any rect width -- a negative-width RectI reaches dglDrawBlendBox as a reversed quad. tests/smoke/particleColorGraph.cs drives the collapse, the radio, the mix and the zoom; the shot harness carries the part only a picture settles, including the three hues on all four editor themes, since they are the one thing here a theme cannot restyle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
An image asset in explicit mode cuts its sheet into cells that each carry a
RegionName, and an animation on it can list those names -- block1 block2 block3
block4 -- instead of listing 0 1 2 3. The point is that a named list survives the
sheet being re-cut or re-ordered, which a numbered one does not.
The engine has played these correctly for as long as they have existed.
ImageFrameProviderCore branches on the mode in all four places that matter, and
the runtime path from validate through play and update to the frame area is
mode-aware throughout. Only AUTHORING was broken, and by one character:
AnimationAsset_ScriptBinding.h formatted a StringTableEntry -- a const char* --
through "%d", so getNamedAnimationFrames returned a row of pointer addresses.
Nothing that asked an animation for its named frames could recover them, so the
Asset Manager refused such an asset twice over: AssetInspector sent it to the
stock inspector, and AssetAnimationStage::canEdit denied it a palette and a
timeline. Both refusals carried comments explaining that the API did not round
trip, and both were right.
NAMED CELLS MODE IS NO LONGER A FLAG ANYBODY SETS. The field, its setter, its
write function and the member are gone; getNamedCellsMode() is a live read of
mImageAsset->getExplicitMode(). The image already held the only honest answer,
and a stored copy could disagree with it the moment that image was re-cut -- and
did worse than that, since a person could set the flag true on an image with no
names at all and get an animation with no frames and no explanation. Nothing has
to keep the two in step now because there is only one of them. The refresh
cascade already reaches here: setExplicitMode ends in refreshAsset,
AssetManager::updateAssetDependencies has the edge from the animation's Image
field, and the drain loop is index-based over a growing vector, so the dependent
animation is dispatched in the same drain that dispatched the image.
The whole editor stays in INDEX space, and that is what kept this small. The
palette shows cell N, the timeline holds cell N, a drag carries cell N, the
range dialog builds "28 29 30"; only loading and committing know that names
exist. Every gesture, the caret arithmetic, the hold detection and the undo
transaction are unchanged.
The one thing index space cannot carry is a name whose cell has been deleted. It
resolves to no index, and EVERY such name resolves to the same -1, so a list
round-tripped through indices would come back from a single edit with two broken
frames merged into one and the other silently committed away. So the timeline
keeps a parallel mSlotNames, always exactly the size of mSlots: mSlots stays the
drawing truth and mSlotNames the authoring truth. setFrames takes indices and
derives names, setNamedFrames takes names and derives indices, and both always
fill both -- which is why appendFrame, insertFrameAtPoint and the range dialog
needed no changes at all. A missing frame draws as an outlined empty cell in the
theme's error color with the name it could not find under it, in the same ink,
and says so again in its tooltip and in the inspector's warning line. It is kept
rather than dropped because dropping it is a deletion the user never asked for
and could not have seen happen.
Cells label themselves in both grids now, so the payoff is visible where the
work is done. That is one virtual on the shared base, so the palette and the
timeline cannot label the same cell differently -- which would make dragging
between them a guess. Long names clip with an ellipsis and the full text is in
the tooltip; the clip measures and draws the same string rather than pairing
getStrNWidth with dglDrawTextN, whose counts are bytes and UTF16 units
respectively and disagree the moment a name is not ASCII.
EVERY EXPLICIT CELL NOW HAS A NAME, which is the invariant the rest rests on. A
cell stored without one is named Frame<N>, seeded at its own index and walked
past anything already taken -- not hypothetical, since deleting a cell from the
middle renumbers every one after it. It happens in calculateExplicitMode, the
one funnel every path ends in, because the TAML read pushes straight into
mExplicitFrames and never goes near addExplicitCell. What stood there before was
a "repair" in four places that could not have worked: it read dSscanf FROM the
empty name INTO a U32 passed by value where a pointer was required, and never
assigned a name to anything. Its guard never fired either -- it compared a
console or TAML buffer against StringTable->EmptyString by POINTER, and neither
is ever interned. The image editor's Add Cell button now asks the engine for the
name instead of building "Frame" @ index itself with no uniqueness check, which
is how adding a cell after deleting one from the middle produced a duplicate
that the rename box beside it would have refused.
Switching an image between explicit and cell mode converts the animations on it,
so the switch is a decision rather than a commitment. Both lists are kept in
memory and only the one in use is written, and the conversion runs from
onAssetRefresh, setImage and initializeAsset -- NOT from validateFrames, which
is where it obviously belongs and where it would have destroyed data. That is
called from inside both frame setters, so setAnimationFrames("") -- what the
editor sends when the timeline is emptied, and what copyFieldsFrom sends on
every single copy -- would have seen an empty active list beside a full one and
put the frames the user had just cleared straight back. validateFrames stays a
pure derivation that touches neither specified list. initializeAsset is new
here: settling this after the whole file is read is what makes the result
independent of TAML field order, which mattered as soon as anything depended on
Image and a frame list together.
Gating the write on the mode is what closes the round trip. Both lists used to
be written whenever they had content, and the named one is applied last and used
to force named mode on -- so an animation given numbered frames after ever
having had named ones came back from its own file named. That in turn made an
older landmine reachable: onTamlCustomWrite gated the Cells node on explicit
mode, so saving an image with the mode off deleted every cell in the file and
with them the only thing that could ever resolve those names again. The cells
are authored data that outlive the mode -- copyAssetStateTo says so in as many
words -- so they are written whenever there are any. Which means the file has to
state the mode out loud, because the read infers explicit mode from the presence
of a Cells node and must keep doing so for every file written before this. A
file that states it is believed; only a file that says nothing is inferred from.
Engine defects fixed on the way, all reachable before any of this:
- getExplicitCellOffset returned NULL from a Vector2-returning function when not
in explicit mode, which selects Vector2(const char*), which calls setString on
a null pointer and dereferences it. The image editor's swap-cells path reaches
it.
- all four getExplicitCell accessors indexed with Vector<T>::at, which takes a
U32 and only asserts -- so it is unchecked in release and at(-1) was a read at
four billion. A failed name lookup is exactly what -1 means around here.
- getExplicitCellName and getExplicitCellIndex refused to answer while explicit
mode was off, which is precisely when a name has to be translated back into an
index. The guards are off those two; the four mutators keep theirs.
- getCellByName's empty-name guard was the same pointer comparison as above, so
an empty name matched the first frame of any image whose cells are unnamed.
- ImageFrameProviderCore::mUsingNamedFrame and mNamedImageFrame were never
initialized by the constructor and never cleared by clearAssets, and
validRender reads the first on the first frame of every static sprite -- an
indeterminate true then dereferences an equally indeterminate name.
- getNamedAnimationFrames sized its return buffer at a fixed 4096 that suits a
list of integers. A region name has no length limit and dSprintf truncates in
silence, so a long animation would have lost its tail and said nothing. Both
it and getMissingFrames measure first.
- mValidatedNameFrames was missing its VECTOR_SET_ASSOCIATION, and the dead
mAnimationIntegration field is gone.
Two new bindings exist to stop script having to branch. getFrameCount answers in
whichever space the animation uses; getAnimationFrameCount refuses in named mode
and returns -1, which read as "fewer than one" to Keep Frame Rate and as "-1
frames" on the inspector's info line. getMissingFrames returns the names no cell
answers to, which is cheaper and more honest than N console calls per refresh.
animationFrameConversionTests and imageAssetCellNameTests cover the arithmetic
through the statics, because building a real explicit cell needs a bitmap and a
unit test has no GL context to load one into: the round trip, a hold surviving
it, everything that fails to resolve in either direction, and the naming search
including the collision walk and the case fold. AnimationAssetCarriesNamedFrames
lost its mode assertion with the field and gained a sibling asserting that BOTH
lists survive a copy, which is what makes the mode switch reversible.
animationFrameValidation carries the engine half -- most valuably that
getNamedAnimationFrames returns four names and not four numbers -- plus the mode
switch, the auto-naming, and the file keeping its cells with the mode off.
assetAnimationTimeline drives the editor half through to reading the saved file
back. The shot harness gains the two pictures that only a picture settles:
whether a name reads at 48 pixels, and whether a missing frame is findable.
The toybox 1234 image and 1234Animation are the demo pair, and the only named
assets in the tree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Asset Manager could show you your assets and let you edit them through the
stock
GuiInspector. That is a flat alphabetical reflection of every registeredfield, which is the same screen for a font as for a particle emitter, and it
cannot say the one thing a person opens an asset to find out: what this asset
actually is — what loaded, how it cut up, how long it plays, which of an
emitter's thirty knobs are connected to anything.
This is the work that turns it into an editor. A library you can search and sort,
a purpose-built inspector for each of the six asset kinds, an animation editor
with a frame palette and a draggable timeline, and — underneath all of it — an
asset you can change your mind about, because editing one no longer writes its
file behind your back.
About a third of the diff is engine work, and it is not incidental. Selecting a
particle asset crashed the editor on unmodified
development; an emitter'sper-emitter blend mode had never been read; four of the six
copyToimplementations were wrong. Each of those is a fix for everyone, not just for the
editor that found it.
26 commits on top of
development. 219 files, +26,758 / −1,471.The library
Assets were picture-only thumbnails in whatever order the asset database's hash
table returned — which is not the same order twice. The name reached a person
only as a tooltip, and
AssetCategoryandAssetDescription, both editable inthe inspector, were read by nothing at all.
AssetLibraryWindownow owns a pinned toolbar over the scroller: a search boxthat matches name, description and category as you type across every group at
once, a tiles/rows toggle, and sort by name or category. A group whose matches
are all gone keeps its header and reads
Images (0), so the shape of the librarydoes not move under the person typing. Sorting reorders the tiles that are
already there rather than rebuilding them, which keeps the selection, the running
animations and the asset acquisitions intact.
The search could not use the engine's own queries, and that is worth knowing
before anyone tries again:
findAssetName's partial mode is a case-insensitiveprefix match,
findAssetCategoryis exact and case-sensitive, and there isno
findAssetDescriptionat all.EditorPreferencesis the editor's first memory between runs — dynamic fields ona
ScriptObjectwritten as TAML togetPrefsPath, holding the view mode and thesort field. Deliberately not
$pref::globals: script has nosetVariable(), sowriting one by name would need
eval().An inspector per asset kind
Six kinds, five purpose-built panes over one
AssetInspectorPanesuperclass, eachlaid out as blocks of roughly equal size in a
GuiGridCtrlso the same pane is1×N in a tall narrow frame, 2×2 at the size the inspector opens at, and N×1
across the foot of a wide screen.
Every pane carries an info line and a warning line, and those are the
parts the separate fields cannot say:
The warnings surface what had previously been a line in a console log and nothing
on screen — an image that did not load, a
.fntthat did not parse, a page imagemissing, explicit frame mode on, a cell layout that does not fit, a frame list
that names cells the image no longer has.
Fields absent on purpose, since "why is this missing" is the first review
question:
AssetInternalandAssetPrivateexist to keep an asset out of theeditor; the asset id and file only restate what is on show;
AssetNameisread-only until renaming is done properly, since it changes the asset id and every
file that refers to it;
AssetAutoUnloadis left off the sound pane becauseAudioAsset::initializeAssetcallssetAssetAutoUnload(false)unconditionally,so a checkbox there could not be changed and would only invite the attempt.
The emitter pane is where the gating matters. A single-particle emitter ignores
ten fields, a POINT emitter ignores its size and its angle, a fixed-aspect one
ignores every Size-Y curve. Two ways of saying "this does not apply", and the
difference is deliberate: alternatives swap (an emitter draws an image or an
animation and never both, so showing the other arm invites filling in both when
one would silently win), while a field that is real, holds a value and is merely
unread by this mode is greyed with the reason as its tooltip — hiding those
would lose the value from sight and make the pane jump about as you tried modes.
The animation editor
Choosing an animation asset splits the preview three ways: the art playing on the
left, every frame the image offers on the right, and the frames the animation
actually plays along the bottom. Choosing anything else puts it back.
Frames get into the timeline by dragging or by clicking; a click is a drop that
never moved, and both end in the same
appendFrame. The timeline holds a copyof the list and reports one change per completed gesture —
setAnimationFrameshas no equality guard, so a per-drag-tick write would be hundreds of file writes
for one reorder.
Two grids share their arithmetic through a C++ base (
GuiEditFrameStripCtrl),and it had to be C++: script cannot ask an image where a frame is.
ImageAsset::getImageFrameAreahas the source rect and no binding exposed it. Thelayout is 22 static functions taking everything they use, so the renderer and the
hit test call the same function with the same numbers and cannot drift — a
disagreement between where a cell is drawn and where it is clicked is experienced
as clicking the wrong frame.
A transport bar over the preview plays, loops, and fills the timeline from
"frames 28 to 32" with an optional ping-pong and hold. Two details there are
design rather than shortcut. Stop pauses —
SpriteBase::stopAnimationsetsthe finished flag and
setAnimationFramegoes throughupdateAnimation, so apreview stopped that way can never be scrubbed again. And keep-frame-rate is a
preference, not an asset field: uniform timing means
AnimationTimeis sharedover however many frames there are, so adding one makes every frame play faster —
which of those you want depends on whether you are lengthening a walk cycle or
dropping in a hold.
Named frames now work. An image in explicit mode cuts its sheet into cells
that carry a
RegionName, and an animation can list those names instead of0 1 2 3— the point being that a named list survives the sheet being re-cut. Theengine has played these correctly for as long as they have existed; only
authoring was broken, and by one character:
getNamedAnimationFramesformatted aStringTableEntrythrough"%d"and returned a row of pointer addresses. Theeditor stays entirely in index space, with the timeline keeping a parallel
mSlotNames— because every name whose cell was deleted resolves to the same-1, so a list round-tripped through indices alone would come back with twobroken frames merged into one. A missing frame draws as an outlined empty cell in
the theme's error color with the name it could not find under it, and is kept
rather than dropped: dropping it is a deletion the user never asked for and could
not have seen happen.
Save, revert, and an undo you can trust
Editing an asset used to write its file. Every setter ended in
refreshAssetand
refreshAssetended inTaml::write, so dragging a particle graph key wasindistinguishable from deciding to keep it. It was also wrong outside the editor:
a running game rewrote its own content as a side effect of a setter.
refreshAssetnow marks the asset unsaved and announces the change.saveAssetwrites, and nothing else does. Around that: Save, Revert, Duplicate and Undo/Redo
on the inspector's title bar, a badge on unsaved library tiles, and one Save All /
Discard All / Cancel prompt in front of Close Project and Exit.
The dirty flag and the snapshots live in C++ because two of the editors never
reach TorqueScript —
GuiParticleGraphInspectordoes every key drag itself, andthe stock
GuiInspectorwrites emitter fields straight onto the object. Arecorder built out of script writes would have been blind to the particle editor,
which is the thing that most needed undo. Script keeps only the policy: what
counts as one step and what it is called.
Undo is whole-asset snapshots of unowned clones. Unowned is the point: with no
owning manager every setter is inert, so taking a snapshot marks nothing, notifies
nobody and loads no bitmap. A restore copies onto the live object and never
replaces it — every
AssetPtrholds a raw pointer that a swap would null.That rested on
copyTo, which was broken in four places, all of them live bugstoday via
clone()andacquireAsset(id, true):layers, and dropped explicit cells unless
ExplicitModehappened to be ontarget's mode, still the default at that point
blank static one — and ParticleAsset inherited it
Fixed by not listing fields at all:
AssetBase::copyTowalks the field table viacopyFieldsFrom, and each type overridescopyAssetStateToonly for what nofield describes.
assetStateCopyTestsenumerates the field table rather than alist of its own, so a field added later is covered the day it lands.
Menus that belong to whoever is in front
File, Edit, Layout and Select were written into the shared bar in
EditorCoreandswitched on when the Gui Editor opened; every command in them named
GuiEditor.The Asset Manager grew the same class of features and could not join them —
setMenuActivematches by item text across the whole tree, so a second editortoggling Undo or Delete would flip the first editor's items too.
Each editor owns its menus now and lends them to the bar while it is the one open.
The bar reads
Torque2D | the open editor's menus | Theme. Nothing becamegeneric: the Gui Editor's item still says "Save Gui…" and runs
GuiEditor.SaveGui(), the Asset Manager's says "Save Asset" and runsAssetAdmin.inspector.SaveAsset(), and Ctrl+S means both.This closed a live bug.
GuiMenuItemCtrl::onActionchecks its own active flag andnever its parent menu's, and
buildAcceleratorMapfilters nothing — so with Filegreyed out, Ctrl+N still ran
GuiEditor.NewGui()from inside the AssetManager. Physically removing the items is what fixes it, plus a
GuiCanvas::rebuildAcceleratorMap()for the moment nothing else notices.Four engine facts shaped the mechanism and are worth reading before touching a
menu again: a menu item learns its bar when it is added to one and nothing
back-fills it;
onChildAddedlinks the keyboard chain by takingend()-2, somenus may only be appended;
SimSet::removeleaves a control registered withno group at all, so each set moves its menus into a
SimGrouprather thanremoving them; and the canvas rebuilds accelerators only when a dialog is pushed
or popped, which a tab change is not.
The particle color graph
An emitter's color is four curves, and the Emitter Graph tab offered them as four
list entries opening four separate graphs. Every color was reachable that way.
What was not reachable was the question anyone opens them to ask: not "what does
red do" but what does this look like over its life.
Red, green and blue are now one Color Channel entry — the three curves layered
on one plot, with a strip under it showing the color they mix to. Alpha keeps its
own graph; it is not a hue, and folding it into the strip would darken every
reading of one.
The live channel is the parent's target field, which is why this is small: three
toggles pick which channel a click edits, and setting one calls
setDisplayField,so the hit test, add, delete, drag and the
refreshAsseton release — andtherefore snapshot undo — are all existing
GuiParticleGraphInspectorcode,unchanged. The strip is exact rather than sampled: between two consecutive members
of the union of the three channels' key times every channel is linear, so one
interpolated quad per span is the gradient. (
dglDrawBlendRangeBoxlooks likethe tool and is not — its stops are spaced evenly across the rect, so it cannot
put a stop at 0.13.)
Zoom was dead on every 0–1 field, and had been.
ParticleGraphCameraControllerbuilds its levels by asking whether max > 1, > 10,Engine fixes
The ones with consequences beyond this editor:
development.PixelArea's default constructor was empty and its four-argumentsetAreanever set
mRegionName, so every frame of every ordinary cell-mode imagecarried an indeterminate pointer into
dStrcmp.ImageFrameProviderCore::mUsingNamedFramewas never initialized and nevercleared, and
validRenderreads it on the first frame of every static sprite —an indeterminate
truethen dereferences an equally indeterminate name.ParticleAsset/ParticleAssetEmitter::getFieldValue(time)shadowedSimObject::getFieldValue(fieldName), the call the whole editor reads rowswith. Asking an emitter for
EmitterNamesampled whichever curve was selectedat 0 seconds and returned a plausible
1.0. Renamed togetFieldValueAtTime.AnimationAsset::onAssetRefreshdid nothing, so re-cutting an image left theanimations on it holding indices from the old cut — and
getImageFrameAreaclamps an out-of-range index rather than failing, so they went on playing the
wrong art silently.
GuiControl::getLineListemitted a stray empty line after a word too long towrap, which becomes
blockHeight— so it moved text under bottom and middlealignment and gave the wrong answer about whether text fits at all.
Platform::pathCopywould not create the folder its destination sitsin, and would copy a directory into itself until the path outgrew
MAX_PATH(the UNIX build has refused that since it was implemented).
BitmapFonthad an empty constructor —mWidthandmHeightare thedivisors
ProcessCharacteruses for texture coordinates — never clearedmCharor
mKerning, so pointing an asset at a second.fntleft the union of bothfonts, and returned on a failed open without clearing, so a missing file kept
the glyphs of the font it used to have.
alxGetAudioLengthleaked a reference on all three return paths;setVolume/setVolumeChannelcompared before clamping, so an out-of-rangevalue read as a change every time.
dglDrawBlendBox's mobile and web path leftGL_COLOR_ARRAYenabledpointing at a stack local, so every later vertex-array draw read a dead frame
for its colors.
getExplicitCellOffsetreturningNULLfrom aVector2-returningfunction, four
getExplicitCellaccessors indexing unchecked withat(-1),getTargetFieldwalking off an emitterless asset before theAssertFatalmeantto catch it, the graph key repair deleting the key it had just inserted,
unloadAssetdeleting an asset holding unsaved work,preloadAssetmarkingevery asset unsaved at startup, and
findAssetPrivate's five-argument bindingcalling
findAssetInternal.Testing
TESTblocks. All 309 unit tests pass.smoke suites pass, and the
$Expectedknown-failure table inrun.ps1isempty, so every one of them is expected to.
point — most of what
assetDirtySaveasserts is that the.tamlon disk didnot change, since not touching it is the whole feature.
assetAnimationClickposts realWM_input for the two gestures that need it —the press/slop/capture fork in the two grids — and, like
explorerGutter, ishanded its points rather than knowing them, because where a cell lands depends
on how many columns the palette wrapped into.
the reason is written down rather than left as a gap: a
GuiDragAndDropCtrlgesture follows the real cursor, which a posted
WM_MOUSEMOVEdoes not move.What would be proved is the engine's capture, not this feature's code.
run.ps1now gives each suite its owntests/logs/<name>.log— every enginestarted from the repo root wrote one shared
console.log, so a test run and ahand-started editor produced a log nobody could read. It also sweeps the project
folders and theme cursor art that tests create, both before each test and
after the run.
Notes for reviewers
Behavior changes outside the editor. These affect shipped games and existing
content, and are the part worth a careful look:
refreshAssetno longer writes the file. Anything relying on a setterpersisting must call
saveAsset. This is the headline change and the reasonthe branch exists.
BlendMode/SrcBlendFactor/DstBlendFactorare nowhonored.
sceneRenderusedmBlendModeand the two factors, whichParticlePlayerdoes not declare — they resolved to the inheritedSceneObjectmembers, so one setting on the player covered every emitter. Existing particle
content may render differently.
QuantityVariationnow does something. It was initialized fromgetQuantityBaseField, giving every emitter a spurious half-base jitter.Content-visible:
bonfireemitted 5–15 per interval and now emits ten.setEmitterAngle/getEmitterAngleare degrees, agreeing with the persistfield and with
configureParticle. They previously stored radians and convertedon the way out, so they agreed with each other and with nothing else.
AnimationAsset'sNamedCellsModefield is gone. Whether an animation usesnames is a live read of the image's explicit mode — the image already held the
only honest answer, and a stored copy could disagree with it the moment that
image was re-cut. Older files stating the flag are ignored.
.image.tamlnow writes itsCellsnode whenever there are cells, andstates
ExplicitModeout loud. Previously the node was gated on the mode, sosaving an image with the mode off deleted every cell in the file. Files written
before this still read correctly: a file that states the mode is believed, and
only a file that says nothing has it inferred from the presence of cells.
GuiTreeViewCtrl::drawIconFramemoved torenderImageAssetFrameinguiDefaultControlRender, beside the two functions a reader would otherwisefind first and misuse.
Also worth knowing:
setMenuActiveis now used nowhere ineditor/. It was a text-compare walk ofthe whole tree plus a full profile re-apply per top-level menu, fired 22 times
per selection change.
setLogFileName,alxPlayPreview,OpenALIsInitialized,isStaticMode,getFrameCount,getMissingFrames.alxPlayPreviewexistsbecause
alxCreateSourcerefuses to build a source at all on a muted channel —so turning a game's music channel down made every music asset unplayable in
the editor, not quiet.
ParticleAssetEmitter::setTargetPositionstill has norefreshAsset, anddeliberately —
AngleToysteers an emitter with it on every mouse move. It iscommented in place so nobody corrects it; the pane asks for the refresh itself.
tooltipProfileremains intermittently flaky in a full run and passes inisolation — it rides on a real posted mouse hover. Re-run before treating it as
a regression.
🤖 Generated with Claude Code
https://claude.ai/code/session_014Xh3XULt9PtdCtsaYHGnYE