fix: remediate audit of studio pipeline, 3D culling, and instancing

16-reviewer audit of ae815f489 (103 findings) remediated in six groups.

1. Lazy THREE init (runtimeobject.ts): scratch Box3/Matrix4/Euler allocated on
   first use, so a 2D-only export (never loads three.js) boots.

2. Culling correctness: computeAABB uses |size|/2 half-extents and places the box
   at the true rotation pivot C' = P + R*(C-P); custom-object children union
   conservative boxes; the frustum is rebuilt in the boxes' space (right-multiply
   _threeGroup.matrixWorld) with a numeric cache incl. camera-Z; hide/cull route
   through setCullingVisible/applyRenderVisibility (no detached-carrier writes);
   custom-object children and nested tile maps no longer cull with a wrong window.

3. Studio pipeline wedge (Utils.js + Studio/*): lock release over all calls,
   working marks only editor calls, spawn failures reach the model, plan merge on
   objects (JSON.parse bug), one persistent LoopGuard with real call_id +
   re-arming + recordSuccess, RequestWriteGate self-retry + supersede-rejects,
   UseStudioRuntime fire-and-forget gate write + recomputed plan + sent check,
   FinalizeSubAgents terminal-'error' + executed-but-unsent + turn-cap + enriched
   report, send-returns-boolean, BYOK cache write-through, suspension cascade,
   turn-cap dispatch, stand-alone cannot spawn.

4. Instancing: useInstancing now serializes (C++); the pool grows without null.add
   and collapses deleted slots; the instance matrix is normalized
   (carrier * correction) and rewritten on rotation/hide; animated/skinned/morph/
   multi-mesh models rejected; per-slot shadows; layer moves re-acquire.

5. BYOK role enforcement: studioRoleId persisted and re-applied every turn (tools +
   prompt); dispatch filters tools outside the role; readOnly roles skip edit
   approval.

6. Warnings + Info: mutating-tool classification, hasOwnProperty role guards,
   cache-message merge, spawn arg caps, per-batch GDD note, fan-out cap, plan
   dependsOn-omission + caps + duplicate-id + fail-closed + trimmed ids, tilemap
   rotated/mirror bail + window margin + null-map guard, pool dispose wired to
   unloadScene, schema description required, plus doc/comment corrections.

Verified: engine builds clean; boot + computeAABB (13/13) + Model3DInstancePool
(9/9) harnesses pass; Jest green for src/AiGeneration + src/AI (175), CustomAIClient
(27, incl. the critical second-turn role regression), PlanStore (25),
FinalizeSubAgents (21), LoopGuard (18), RequestWriteGate (11), Roles (9).
This commit is contained in:
John Smith
2026-09-22 15:31:25 -04:00
parent ae815f489b
commit 08a49e2f55
38 changed files with 1579 additions and 385 deletions
+8 -6
View File
@@ -4,7 +4,8 @@ Answers three questions the fork asked, then records what this plan builds on th
Every claim below carries a `path:line` (or a `path`) that was read while writing this
document, in this checkout. Where a figure is quoted from another project, the file that
contains it is named.
contains it is named. The gap claims were measured at the parent of `ae815f489`, before
the phases of section 5 landed; those that section 5 closes are stated in the past tense.
---
@@ -37,9 +38,9 @@ common repeated-prop case, not for an arbitrary scene.
`GDJS/Runtime/CustomRuntimeObjectInstanceContainer.ts:263`.
- `RuntimeObject3D.getRendererObject()` returns `null`
(`Extensions/3D/A_RuntimeObject3D.ts:97-99`), so the 2D AABB culling path in
`CustomRuntimeObjectInstanceContainer._updateObjectsPreRender` skips every 3D object
entirely: `if (rendererObject) { ... } else { object.updatePreRender(this); }`
(`GDJS/Runtime/CustomRuntimeObjectInstanceContainer.ts:266-283`).
`GDJS/Runtime/runtimescene.ts` `_updateObjectsPreRender` (pre-change `:529-573`)
skips every 3D object entirely:
`if (rendererObject) { ... } else { object.updatePreRender(this); }`.
So every 3D object in the scene is submitted to the GPU every frame regardless of where the
camera is. Combined with 1.1, a large city is not merely slow — it is bounded by total
@@ -206,7 +207,8 @@ create_or_update_plan, run_gameplay_test, change_gameplay_tests
`spawn_agent` is added to this list by Phase 4. The four roles and their exact tool names are
defined in `newIDE/app/src/AiGeneration/Studio/Roles.js` (Phase 2); the read-only invariant
(tester mutates nothing; designer's only mutating tool is `add_or_edit_variable`) is asserted
(tester mutates no game content, but writes test definitions (`change_gameplay_tests`,
`run_gameplay_test`); designer's only mutating tool is `add_or_edit_variable`) is asserted
by `Roles.spec.js`.
---
@@ -218,7 +220,7 @@ by `Roles.spec.js`.
| No instancing: every Model3D instance clones its own scene graph | `Extensions/3D/Model3DRuntimeObject3DRenderer.ts:349` (`SkeletonUtils.clone`); no `THREE.InstancedMesh` outside vendored Three.js | Draw calls rise 1:1 with repeated props; framerate collapses on any repeated-model set | **7** |
| 3D objects never culled in a scene | `GDJS/Runtime/runtimescene.ts:524` (`TODO (3D) culling`) | Off-camera 3D objects still submitted every frame | **6** |
| 3D objects never culled inside a custom object | `GDJS/Runtime/CustomRuntimeObjectInstanceContainer.ts:263` (`TODO (3D) culling`) | A custom object holding many 3D children is never culled as a whole or in part | **6** |
| `RuntimeObject3D` reports no renderer object, so the 2D AABB path skips it | `Extensions/3D/A_RuntimeObject3D.ts:97-99` (`getRendererObject() { return null; }`); branch at `GDJS/Runtime/CustomRuntimeObjectInstanceContainer.ts:266-283` | Reinforces the two rows above: even the existing 2D culling cannot see 3D objects | **6** |
| `RuntimeObject3D` reports no renderer object, so the 2D AABB path skips it | `Extensions/3D/A_RuntimeObject3D.ts:97-99` (`getRendererObject() { return null; }`) | Reinforces the two rows above: even the existing 2D culling cannot see 3D objects | **6** |
| Base container has no camera to cull against | `GDJS/Runtime/RuntimeInstanceContainer.ts:496` (`TODO (3D) culling`) | Accepted — see below | accepted, not fixed |
| TileMap viewport culling only for SimpleTileMap, only as a direct scene child, only with an unrotated 3D camera | `Extensions/TileMap/tilemapruntimeobject-pixi-renderer.ts:152-159` (`dimX + dimY > 100 && isSimpleTileMap(object) && instanceContainer === scene && (!gdjs.scene3d || rotations are 0)`) | A large `TileMap` (not Simple) or a tilemap nested in a custom object re-renders the whole map every frame | **8** |
| Any tilemap opacity change forces a full re-render | `Extensions/TileMap/tilemapruntimeobject-pixi-renderer.ts:90-95` (`updateTileMap(true)`) | Fading a district stutters | accepted, not fixed |
+5 -1
View File
@@ -110,6 +110,10 @@ namespace gdjs {
return gdjs.Object3DCulling.isInFrustum(this, frustum);
}
setCullingVisible(culled: boolean): void {
this.getRenderer().setCullingVisible(culled);
}
getRendererObject() {
return null;
}
@@ -612,7 +616,7 @@ namespace gdjs {
hide(enable: boolean): void {
super.hide(enable);
this.getRenderer().updateVisibility();
this.getRenderer().updateVisibility(!this.isHidden());
}
hasEstimatedVelocity(): boolean {
+24 -2
View File
@@ -54,8 +54,30 @@ namespace gdjs {
this.updatePosition();
}
updateVisibility() {
this._threeObject3D.visible = !this._object.isHidden();
/**
* The object's own hidden state (`!isHidden()`) and its frustum-cull state,
* combined by `applyRenderVisibility`. Kept separate so hiding and culling
* stay independent and can both be routed through the renderer (a detached
* three.js node is not enough: an instanced model's real draw is an
* `InstancedMesh` slot, which `Model3DRuntimeObject3DRenderer` collapses).
*/
protected _isVisible: boolean = true;
protected _isCulled: boolean = false;
updateVisibility(visible: boolean) {
this._isVisible = !!visible;
this.applyRenderVisibility();
}
setCullingVisible(culled: boolean): void {
this._isCulled = !!culled;
this.applyRenderVisibility();
}
/** Combine the object's hidden state with its frustum-cull state. */
protected applyRenderVisibility(): void {
const threeObject = this.get3DRendererObject();
if (threeObject) threeObject.visible = this._isVisible && !this._isCulled;
}
invalidateRotation(): void {
+36 -1
View File
@@ -41,6 +41,7 @@ namespace gdjs {
private _rotationY: float = 0;
private _customCenterZ: float = 0;
private static _temporaryVector = new THREE.Vector3();
private static _cullingScratchBox: THREE.Box3 | null = null;
private _hasEstimatedVelocity = false;
private _estimatedVelocityX: float = 0;
@@ -81,9 +82,28 @@ namespace gdjs {
/**
* Writes the object's conservative axis-aligned box into `box` and returns
* it. Allocates nothing. See `gdjs.Object3DCulling.computeAABB`.
*
* The box is widened to cover each 3D child's own box so that content which
* pokes outside the parent's declared size is never wrongly culled. This is
* conservative in the safe direction: it can only draw more, never hide.
*/
getAABB3D(box: THREE.Box3): THREE.Box3 {
return gdjs.Object3DCulling.computeAABB(this, box);
gdjs.Object3DCulling.computeAABB(this, box);
const scratch =
CustomRuntimeObject3D._cullingScratchBox ||
(CustomRuntimeObject3D._cullingScratchBox = new THREE.Box3());
for (const childInstance of this._instanceContainer.getAdhocListOfAllInstances()) {
if (!childInstance.isIncludedInParentCollisionMask()) {
continue;
}
if (!gdjs.CullableRuntimeObject3D.isCullable3D(childInstance)) {
continue;
}
childInstance.getAABB3D(scratch);
box.min.min(scratch.min);
box.max.max(scratch.max);
}
return box;
}
/**
@@ -94,6 +114,10 @@ namespace gdjs {
return gdjs.Object3DCulling.isInFrustum(this, frustum);
}
setCullingVisible(culled: boolean): void {
this.getRenderer().setCullingVisible(culled);
}
override extraInitializationFromInitialInstance(
initialInstanceData: InstanceData
) {
@@ -396,6 +420,17 @@ namespace gdjs {
if (!childInstance.isIncludedInParentCollisionMask()) {
continue;
}
if (gdjs.CullableRuntimeObject3D.isCullable3D(childInstance)) {
// Conservative z-extent including the child's rotation (see the
// rotation-aware box in `gdjs.Object3DCulling.computeAABB`).
const scratch =
CustomRuntimeObject3D._cullingScratchBox ||
(CustomRuntimeObject3D._cullingScratchBox = new THREE.Box3());
childInstance.getAABB3D(scratch);
minZ = Math.min(minZ, scratch.min.z);
maxZ = Math.max(maxZ, scratch.max.z);
continue;
}
if (!gdjs.Base3DHandler.is3D(childInstance)) {
continue;
}
+16 -2
View File
@@ -84,7 +84,7 @@ namespace gdjs {
this._object.isFlippedZ() ? -scaleZ : scaleZ
);
threeObject3D.visible = !this._object.hidden;
this.updateVisibility();
this._isContainerDirty = false;
}
@@ -129,8 +129,22 @@ namespace gdjs {
this._isContainerDirty = true;
}
private _isVisible: boolean = true;
private _isCulled: boolean = false;
updateVisibility(): void {
this._threeGroup.visible = !this._object.hidden;
this._isVisible = !this._object.hidden;
this.applyRenderVisibility();
}
setCullingVisible(culled: boolean): void {
this._isCulled = !!culled;
this.applyRenderVisibility();
}
/** Combine the object's hidden state with its frustum-cull state. */
applyRenderVisibility(): void {
this._threeGroup.visible = this._isVisible && !this._isCulled;
}
updateOpacity(): void {
+100 -25
View File
@@ -29,12 +29,31 @@ namespace gdjs {
usedSlotCount: integer;
/** Whether the instance matrices changed since the last `flush`. */
isMatrixDirty: boolean;
/**
* Slots that cast/receive shadows. Drives the `InstancedMesh`'s mesh-level
* shadow flags (Three.js shadows are per-mesh, not per-instance).
*/
castShadowSlots: Set<integer>;
receiveShadowSlots: Set<integer>;
};
export class Model3DInstancePool {
private _entries: Map<string, InstanceEntry> = new Map();
/** The scratch matrix, reused by `setMatrix`; never returned. */
private _temporaryMatrix: THREE.Matrix4 = new THREE.Matrix4();
/**
* Reused scratch (a `getMatrixAt` target and the collapsed matrix); never
* returned to callers. Allocated lazily so an unused pool allocates nothing.
*/
private _scratchMatrix: THREE.Matrix4 | null = null;
/** Preallocated `flush` handler so `flush` allocates no closure per frame. */
private _flushEntry: (entry: InstanceEntry) => void;
constructor() {
this._flushEntry = (entry: InstanceEntry): void => {
if (!entry.isMatrixDirty) return;
entry.instancedMesh.instanceMatrix.needsUpdate = true;
entry.isMatrixDirty = false;
};
}
/**
* Take a free slot for `key`, creating the `InstancedMesh` when the key is
@@ -52,23 +71,31 @@ namespace gdjs {
instancedMesh,
growMesh: (newCapacity: integer) => {
const grown = createMesh(newCapacity);
// Move the matrices already written onto the bigger mesh.
// Move the matrices already written onto the bigger mesh. Read from
// the *current* mesh, not the one this closure first saw - a second
// grow must copy from the previous grown mesh, not a disposed one.
const current = newEntry.instancedMesh;
const scratch =
this._scratchMatrix || (this._scratchMatrix = new THREE.Matrix4());
for (let slot = 0; slot < newEntry.usedSlotCount; slot++) {
instancedMesh.getMatrixAt(slot, this._temporaryMatrix);
grown.setMatrixAt(slot, this._temporaryMatrix);
current.getMatrixAt(slot, scratch);
grown.setMatrixAt(slot, scratch);
}
grown.instanceMatrix.needsUpdate = true;
if (instancedMesh.parent) {
instancedMesh.parent.remove(instancedMesh);
instancedMesh.parent.add(grown);
}
instancedMesh.dispose();
// `createMesh` already adds the grown mesh to the layer group; only
// the old one must be dropped (capture the parent first: `remove`
// nulls `.parent`).
const parent = current.parent;
if (parent) parent.remove(current);
current.dispose();
newEntry.instancedMesh = grown;
return grown;
},
freeSlots: [],
usedSlotCount: 0,
isMatrixDirty: true,
castShadowSlots: new Set<integer>(),
receiveShadowSlots: new Set<integer>(),
};
this._entries.set(key, newEntry);
return this._takeSlot(newEntry);
@@ -95,19 +122,67 @@ namespace gdjs {
if (!entry) return;
if (slot < 0 || slot >= entry.usedSlotCount) return;
if (entry.freeSlots.indexOf(slot) !== -1) return;
// Collapse first: a deleted object must stop being drawn even before the
// slot is reused.
this._collapseSlot(entry, slot);
entry.freeSlots.push(slot);
}
/**
* Write the instance matrix of `slot`. Copies into a scratch matrix and marks
* the entry dirty; the caller flushes once per frame.
* Collapse a slot to zero scale (a zero instance matrix turns the triangle
* into a point, so it stops being drawn). Marks the entry dirty and clears
* the slot from the shadow sets so it stops driving the mesh's shadow flags.
*/
private _collapseSlot(entry: InstanceEntry, slot: integer): void {
const scratch =
this._scratchMatrix || (this._scratchMatrix = new THREE.Matrix4());
scratch.makeScale(0, 0, 0);
entry.instancedMesh.setMatrixAt(slot, scratch);
entry.isMatrixDirty = true;
entry.castShadowSlots.delete(slot);
entry.receiveShadowSlots.delete(slot);
entry.instancedMesh.castShadow = entry.castShadowSlots.size > 0;
entry.instancedMesh.receiveShadow = entry.receiveShadowSlots.size > 0;
}
/** Collapse a live slot without releasing it (used by hide/cull). */
collapseSlot(key: string, slot: integer): void {
const entry = this._entries.get(key);
if (!entry) return;
if (slot < 0 || slot >= entry.usedSlotCount) return;
this._collapseSlot(entry, slot);
}
/**
* Record which slots cast/receive shadows and update the `InstancedMesh`'s
* mesh-level shadow flags (the mesh casts if any live slot does).
*/
setSlotShadows(
key: string,
slot: integer,
castShadow: boolean,
receiveShadow: boolean
): void {
const entry = this._entries.get(key);
if (!entry) return;
if (slot < 0 || slot >= entry.usedSlotCount) return;
if (castShadow) entry.castShadowSlots.add(slot);
else entry.castShadowSlots.delete(slot);
if (receiveShadow) entry.receiveShadowSlots.add(slot);
else entry.receiveShadowSlots.delete(slot);
entry.instancedMesh.castShadow = entry.castShadowSlots.size > 0;
entry.instancedMesh.receiveShadow = entry.receiveShadowSlots.size > 0;
}
/**
* Write the instance matrix of `slot` (passed straight through, no copy);
* the caller flushes once per frame.
*/
setMatrix(key: string, slot: integer, matrix: THREE.Matrix4): void {
const entry = this._entries.get(key);
if (!entry) return;
if (slot < 0 || slot >= entry.instancedMesh.count) return;
this._temporaryMatrix.copy(matrix);
entry.instancedMesh.setMatrixAt(slot, this._temporaryMatrix);
entry.instancedMesh.setMatrixAt(slot, matrix);
entry.isMatrixDirty = true;
}
@@ -116,11 +191,7 @@ namespace gdjs {
* once per frame, after all the objects of the frame wrote their matrices.
*/
flush(): void {
this._entries.forEach((entry) => {
if (!entry.isMatrixDirty) return;
entry.instancedMesh.instanceMatrix.needsUpdate = true;
entry.isMatrixDirty = false;
});
this._entries.forEach(this._flushEntry);
}
/** The `InstancedMesh` of a key, for a caller that needs to check or scene it. */
@@ -131,9 +202,7 @@ namespace gdjs {
/** The keys currently held, in insertion order. */
getKeys(): Array<string> {
const keys: Array<string> = [];
this._entries.forEach((entry, key) => keys.push(key));
return keys;
return Array.from(this._entries.keys());
}
/** Release every mesh this pool created and forget every key. */
@@ -149,13 +218,19 @@ namespace gdjs {
}
/**
* The pool key of one Model3D instance: the model resource, the material type and
* the layer it is rendered on. Two objects with the same key can share one
* The pool key of one Model3D instance: the model resource, the material type
* and the layer it is rendered on. Two objects with the same key can share one
* `InstancedMesh` because their meshes and material are interchangeable.
*
* Each part is `JSON.stringify`-escaped so the comma-joined key is injective
* (a name containing `,` or `|` cannot collide with another triple).
*/
export const getModelInstanceKey = (
modelResourceName: string,
materialType: string,
layerName: string
): string => `${modelResourceName}|${materialType}|${layerName}`;
): string =>
`${JSON.stringify(modelResourceName)},${JSON.stringify(
materialType
)},${JSON.stringify(layerName)}`;
}
@@ -283,6 +283,7 @@ void Model3DObjectConfiguration::DoUnserializeFrom(
crossfadeDuration = content.GetDoubleAttribute("crossfadeDuration");
isCastingShadow = content.GetBoolAttribute("isCastingShadow");
isReceivingShadow = content.GetBoolAttribute("isReceivingShadow");
useInstancing = content.GetBoolAttribute("useInstancing", false);
RemoveAllAnimations();
auto &animationsElement = content.GetChild("animations");
@@ -314,6 +315,7 @@ void Model3DObjectConfiguration::DoSerializeTo(
content.SetAttribute("crossfadeDuration", crossfadeDuration);
content.SetAttribute("isCastingShadow", isCastingShadow);
content.SetAttribute("isReceivingShadow", isReceivingShadow);
content.SetAttribute("useInstancing", useInstancing);
auto &animationsElement = content.AddChild("animations");
animationsElement.ConsiderAsArrayOf("animation");
+1 -2
View File
@@ -162,6 +162,7 @@ public:
bool shouldKeepAspectRatio() const { return keepAspectRatio; };
bool shouldCastShadow() const { return isCastingShadow; };
bool shouldReceiveShadow() const { return isReceivingShadow; };
bool shouldUseInstancing() const { return useInstancing; };
///@}
protected:
@@ -188,8 +189,6 @@ private:
bool isReceivingShadow;
bool useInstancing;
bool shouldUseInstancing() const { return useInstancing; };
std::vector<Model3DAnimation> animations;
static Model3DAnimation badAnimation; //< Bad animation when an out of bound
// animation is requested.
+11
View File
@@ -528,6 +528,17 @@ namespace gdjs {
}
super.onDeletedFromScene();
}
/**
* Moving to another layer re-acquires the instance slot from that layer's
* pool (the pool key embeds the layer name).
*/
setLayer(layerName: string): void {
const wasInstanced = this._renderer.getInstanceSlot() !== null;
if (wasInstanced) this._renderer.releaseInstancing();
super.setLayer(layerName);
if (wasInstanced) this._renderer.trySetupInstancing();
}
}
/** @category Objects > 3D Model */
@@ -89,6 +89,11 @@ namespace gdjs {
*/
private _instanceSlot: integer | null = null;
private _instanceKey: string | null = null;
/**
* Bakes everything after the carrier (stretch/centering + the model's own
* node transforms) into one matrix so the instance matrix matches the clone.
*/
private _correctionMatrix: THREE.Matrix4 | null = null;
private _layerRenderer: gdjs.LayerPixiRenderer | null = null;
private static _instanceMatrix: THREE.Matrix4 | null = null;
@@ -151,6 +156,15 @@ namespace gdjs {
) {
return false;
}
// An animated model morphs its vertices over time, which a single shared
// instance matrix cannot express. Skins/clips/morphs never take this path.
if (
this._originalModel &&
this._originalModel.animations &&
this._originalModel.animations.length > 0
) {
return false;
}
return !!this._getSingleMesh();
}
@@ -164,6 +178,7 @@ namespace gdjs {
let found: THREE.Mesh | null = null;
let meshCount = 0;
let skippedSkinnedMesh = false;
let hasMorphTargets = false;
// An empty group contributes nothing; a mesh (or a mesh-less leaf) is one
// drawable, so anything but exactly one drawable means no shared geometry.
const visit = (node: THREE.Object3D) => {
@@ -173,9 +188,16 @@ namespace gdjs {
skippedSkinnedMesh = true;
return;
}
if (
maybeMesh.morphTargetInfluences &&
maybeMesh.morphTargetInfluences.length > 0
) {
hasMorphTargets = true;
}
meshCount++;
if (meshCount === 1) found = maybeMesh;
return;
// Fall through: a mesh node with child meshes is more than one
// drawable and must be counted as such (rejected below).
}
if (node.children.length === 0) {
// A leaf that is not a mesh (a light, a camera, an empty).
@@ -184,7 +206,7 @@ namespace gdjs {
for (const child of node.children) visit(child);
};
visit(scene);
if (skippedSkinnedMesh || meshCount !== 1) return null;
if (skippedSkinnedMesh || hasMorphTargets || meshCount !== 1) return null;
return found;
}
@@ -202,7 +224,7 @@ namespace gdjs {
* Returns false when instancing is not applicable, in which case the caller
* keeps the clone path.
*/
private _trySetupInstancing(): boolean {
trySetupInstancing(): boolean {
if (!this._canUseInstancing()) return false;
const mesh = this._getSingleMesh();
if (!mesh) return false;
@@ -241,6 +263,16 @@ namespace gdjs {
});
this._instanceKey = key;
// The clone draws `carrier * threeObject(stretch/centering) * mesh`; bake
// everything after the carrier into one correction matrix so the instance
// matrix (`carrier * correction`) matches the clone exactly.
this._originalModel.scene.updateMatrixWorld(true);
this._threeObject.updateMatrix();
this._correctionMatrix = new THREE.Matrix4().multiplyMatrices(
this._threeObject.matrix,
mesh.matrixWorld
);
// The instanced mesh draws this object now: drop the per-object clone and
// take the carrier out of the scene graph. The carrier is never added to
// the scene while instancing is on - it exists only to carry the transform
@@ -249,6 +281,11 @@ namespace gdjs {
const carrier = this.get3DRendererObject();
carrier.clear();
layerRenderer.remove3DRendererObject(carrier);
// The first write was previously lost because `_updateModel` writes the
// matrix before the slot exists; write it now that the slot is acquired.
this._writeInstanceMatrix();
this._updateShadow();
return true;
}
@@ -263,6 +300,7 @@ namespace gdjs {
const carrier = this.get3DRendererObject();
carrier.updateMatrix();
matrix.copy(carrier.matrix);
if (this._correctionMatrix) matrix.multiply(this._correctionMatrix);
if (this._layerRenderer) {
this._layerRenderer
.getModelInstancePool()
@@ -306,6 +344,29 @@ namespace gdjs {
this._writeInstanceMatrix();
}
override updateRotation() {
super.updateRotation();
this._writeInstanceMatrix();
}
/**
* On the instanced path the real draw is an `InstancedMesh` slot (the carrier
* is a detached transform node), so hide/cull collapses or restores the slot
* instead of setting `visible` on the carrier.
*/
protected override applyRenderVisibility(): void {
if (this._instanceKey !== null && this._instanceSlot !== null) {
const drawn = this._isVisible && !this._isCulled;
if (this._layerRenderer) {
const pool = this._layerRenderer.getModelInstancePool();
if (!drawn) pool.collapseSlot(this._instanceKey, this._instanceSlot);
else this._writeInstanceMatrix();
}
return;
}
super.applyRenderVisibility();
}
getOriginPoint(): FloatPoint3D {
//@ts-ignore
const point: FloatPoint3D = gdjs.staticArray(
@@ -558,7 +619,7 @@ namespace gdjs {
// Last, so the clone is in place if instancing cannot be used: the
// instanced path replaces it, and silently leaves it alone otherwise.
this.releaseInstancing();
this._trySetupInstancing();
this.trySetupInstancing();
}
/**
@@ -587,6 +648,21 @@ namespace gdjs {
}
_updateShadow() {
if (
this._instanceKey !== null &&
this._instanceSlot !== null &&
this._layerRenderer
) {
this._layerRenderer
.getModelInstancePool()
.setSlotShadows(
this._instanceKey,
this._instanceSlot,
this._model3DRuntimeObject._isCastingShadow,
this._model3DRuntimeObject._isReceivingShadow
);
return;
}
this._threeObject.traverse((child) => {
child.castShadow = this._model3DRuntimeObject._isCastingShadow;
child.receiveShadow = this._model3DRuntimeObject._isReceivingShadow;
@@ -8,12 +8,15 @@ namespace gdjs {
* The whole map (`[0, dimX, 0, dimY]`) is returned whenever culling must not
* be applied:
* - the map is small (`dimX + dimY <= 100`): the maths is not worth paying for;
* - the tile map is nested in a custom object: its box is in the container's
* space while the camera corners are scene-space, so it is drawn in full
* rather than culled with a wrong window;
* - the container has no scene or the scene has no such layer;
* - the layer's 3D camera is tilted, because the axis-aligned tile-bounds
* maths is only valid for an unrotated camera.
*
* The camera is resolved from the **scene's** layer, so a tile map nested in
* a custom object is culled too - its own container has no camera.
* The camera is resolved from the **scene's** layer (a nested tile map's own
* container has no camera).
*
* The computed bounds are clamped to `[0, dimX] x [0, dimY]`: a camera outside
* the map must not produce out-of-range bounds that differ every frame and so
@@ -38,6 +41,20 @@ namespace gdjs {
const instanceContainer = object.getInstanceContainer();
const scene = instanceContainer.getScene();
// Nested tile map: scene-space camera corners vs a container-space grid.
// Draw it in full rather than cull with a wrong window.
if (instanceContainer !== scene) return result;
// No loaded tile map yet: the grid lookup would map both corners to [0,0].
if (!object.getTileMap()) return result;
// The axis-aligned tile bounds maths is only valid for an unrotated,
// unmirrored map (the transform uses |scale| and samples only two corners).
if (
object.getAngle() !== 0 ||
object.getScaleX() < 0 ||
object.getScaleY() < 0
) {
return result;
}
const layerName = object.getLayer();
const layer = scene ? scene.getLayer(layerName) : null;
if (!layer) return result;
@@ -71,22 +88,23 @@ namespace gdjs {
cameraY + cameraHalfHeight
);
// Clamp to the map.
// Clamp to the map, expanded one tile on every side so a partially-visible
// tile at the window edge is not cut off.
result[0] = Math.max(
0,
Math.min(dimX, Math.min(cameraLeftTile, cameraRightTile))
Math.min(dimX, Math.min(cameraLeftTile, cameraRightTile) - 1)
);
result[1] = Math.max(
0,
Math.min(dimX, Math.max(cameraLeftTile, cameraRightTile) + 1)
Math.min(dimX, Math.max(cameraLeftTile, cameraRightTile) + 2)
);
result[2] = Math.max(
0,
Math.min(dimY, Math.min(cameraTopTile, cameraBottomTile))
Math.min(dimY, Math.min(cameraTopTile, cameraBottomTile) - 1)
);
result[3] = Math.max(
0,
Math.min(dimY, Math.max(cameraTopTile, cameraBottomTile) + 1)
Math.min(dimY, Math.max(cameraTopTile, cameraBottomTile) + 2)
);
return result;
};
@@ -266,24 +266,18 @@ namespace gdjs {
const object = allInstancesList[i];
// 3D objects report no `getRendererObject()` (see
// `RuntimeObject3D.getRendererObject`), so they need their own culling.
// A custom object has no camera of its own: resolve the scene's, for the
// layer the 3D object is on.
// `RuntimeObject3D.getRendererObject`), so they need their own
// visibility handling.
//
// Both 3D object families are caught by the capability, not by a base
// class: `CustomRuntimeObject3D` does not extend `RuntimeObject3D`.
const isThreeD = gdjs.CullableRuntimeObject3D.isCullable3D(object);
if (isThreeD) {
const scene = this.getScene();
const layer = scene ? scene.getLayer(object.getLayer()) : null;
const layerRenderer = layer ? layer.getRenderer() : null;
const frustum = layerRenderer
? layerRenderer.getThreeFrustum()
: null;
const isVisible =
!object.isHidden() && (!frustum || object.isInFrustum(frustum));
const threeObject = object.get3DRendererObject();
if (threeObject) threeObject.visible = isVisible;
// A 3D child's box is in the custom object's container space, but the
// frustum is in scene space: cull nothing here (see
// AI_GAME_STUDIO_ASSESSMENT follow-up for container-space frustums).
object.setCullingVisible(false);
const isVisible = !object.isHidden();
if (isVisible) {
this.getGame()
+3 -8
View File
@@ -493,14 +493,9 @@ namespace gdjs {
*/
_updateObjectsPreRender() {
const allInstancesList = this.getAdhocListOfAllInstances();
// No 3D culling here: a plain instance container has no camera of its own,
// so 3D culling is only defined where a camera exists - on the scene
// (`RuntimeScene._updateObjectsPreRender`) and inside a custom object,
// which resolves the scene's camera for the object's layer
// (`CustomRuntimeObjectInstanceContainer._updateObjectsPreRender`).
// `RuntimeScene` overrides this method with the culling 2D and 3D objects
// both need; this implementation is the first-frame path for a scene and
// the every-frame path for a container without a camera.
// RuntimeScene overrides this with the culling both 2D and 3D objects
// need; this implementation runs only for containers without their own
// culling (custom object containers override it too).
for (let i = 0, len = allInstancesList.length; i < len; ++i) {
const object = allInstancesList[i];
const rendererObject = object.getRendererObject();
@@ -223,8 +223,25 @@ namespace gdjs {
*/
private _threeFrustum: THREE.Frustum | null = null;
private _threeFrustumMatrix: THREE.Matrix4 | null = null;
/** Everything the frustum depends on, so it is rebuilt only on a change. */
private _threeFrustumSignature: string | null = null;
/**
* Everything the frustum depends on, so it is rebuilt only on a change.
* Compared field by field - never concatenated - to avoid a per-object
* per-frame string allocation. `getCameraZ` is included so a camera depth
* change also invalidates the cached frustum.
*/
private _frustumSigValid: boolean = false;
private _frustumSigCameraX: float = 0;
private _frustumSigCameraY: float = 0;
private _frustumSigCameraZ: float = 0;
private _frustumSigCameraRotation: float = 0;
private _frustumSigCameraZoom: float = 0;
private _frustumSigWidth: float = 0;
private _frustumSigHeight: float = 0;
private _frustumSigRotX: float = 0;
private _frustumSigRotY: float = 0;
private _frustumSigFov: float = 0;
private _frustumSigNear: float = 0;
private _frustumSigFar: float = 0;
// For a 2D+3D layer, the 2D rendering is done on the render texture
// and then must be displayed on a plane in the 3D world:
@@ -344,6 +361,14 @@ namespace gdjs {
return this._modelInstancePool;
}
/** Release the Model3D instance pool (called when the scene is unloaded). */
disposeModelInstancePool(): void {
if (this._modelInstancePool) {
this._modelInstancePool.dispose();
this._modelInstancePool = null;
}
}
/** The 3D group this layer renders its 3D objects into. */
getThreeGroup(): THREE.Group | null {
return this._threeGroup;
@@ -390,33 +415,36 @@ namespace gdjs {
const camera = this._threeCamera;
if (!camera) return null;
// A cheap signature of everything the frustum depends on.
const cameraSignature =
this._layer.getCameraX() +
'|' +
this._layer.getCameraY() +
'|' +
this._layer.getCameraRotation() +
'|' +
this._layer.getCameraZoom() +
'|' +
this._layer.getWidth() +
'|' +
this._layer.getHeight() +
'|' +
camera.rotation.x +
'|' +
camera.rotation.y +
'|' +
camera.fov +
'|' +
camera.near +
'|' +
camera.far;
// Everything the frustum depends on. Compared field by field (never
// concatenated) to avoid a per-object per-frame string allocation.
const cameraX = this._layer.getCameraX();
const cameraY = this._layer.getCameraY();
const cameraZ = this._layer.getCameraZ(null);
const cameraRotation = this._layer.getCameraRotation();
const cameraZoom = this._layer.getCameraZoom();
const width = this._layer.getWidth();
const height = this._layer.getHeight();
const rotX = camera.rotation.x;
const rotY = camera.rotation.y;
const fov = camera instanceof THREE.PerspectiveCamera ? camera.fov : 0;
const near = camera.near;
const far = camera.far;
if (
this._threeFrustum &&
this._threeFrustumSignature === cameraSignature
this._frustumSigValid &&
this._frustumSigCameraX === cameraX &&
this._frustumSigCameraY === cameraY &&
this._frustumSigCameraZ === cameraZ &&
this._frustumSigCameraRotation === cameraRotation &&
this._frustumSigCameraZoom === cameraZoom &&
this._frustumSigWidth === width &&
this._frustumSigHeight === height &&
this._frustumSigRotX === rotX &&
this._frustumSigRotY === rotY &&
this._frustumSigFov === fov &&
this._frustumSigNear === near &&
this._frustumSigFar === far
) {
return this._threeFrustum;
}
@@ -429,13 +457,37 @@ namespace gdjs {
}
this._update3DCameraFromLayer();
if (this._threeCameraDirty) {
camera.updateProjectionMatrix();
this._threeCameraDirty = false;
}
camera.updateMatrixWorld();
if (this._threeScene) this._threeScene.updateMatrixWorld();
this._threeFrustumMatrix.multiplyMatrices(
camera.projectionMatrix,
camera.matrixWorldInverse
);
// Right-multiply by the group's world matrix to map the frustum into the
// space of the boxes built by `gdjs.Object3DCulling.computeAABB` (the
// group's world matrix carries the `_threeScene` Y mirror).
if (this._threeGroup) {
this._threeFrustumMatrix.multiply(this._threeGroup.matrixWorld);
}
this._threeFrustum.setFromProjectionMatrix(this._threeFrustumMatrix);
this._threeFrustumSignature = cameraSignature;
this._frustumSigCameraX = cameraX;
this._frustumSigCameraY = cameraY;
this._frustumSigCameraZ = cameraZ;
this._frustumSigCameraRotation = cameraRotation;
this._frustumSigCameraZoom = cameraZoom;
this._frustumSigWidth = width;
this._frustumSigHeight = height;
this._frustumSigRotX = rotX;
this._frustumSigRotY = rotY;
this._frustumSigFov = fov;
this._frustumSigNear = near;
this._frustumSigFar = far;
this._frustumSigValid = true;
return this._threeFrustum;
}
+70 -21
View File
@@ -72,6 +72,12 @@ namespace gdjs {
getAABB3D(box: THREE.Box3): THREE.Box3;
/** True when the object's box intersects `frustum`. */
isInFrustum(frustum: THREE.Frustum): boolean;
/**
* Hide/show the object because it left/entered the camera frustum. Routed
* through the renderer (not a detached three.js node) so instanced models
* can collapse the hidden instance's slot.
*/
setCullingVisible(culled: boolean): void;
}
/** @category Objects > 3D Objects */
@@ -98,6 +104,10 @@ namespace gdjs {
getRotationX(): float;
getRotationY(): float;
angle: float;
/** Scene-space rotation center (not always the box center). */
getCenterXInScene(): float;
getCenterYInScene(): float;
getCenterZInScene(): float;
};
/**
@@ -106,10 +116,14 @@ namespace gdjs {
* @category Objects > 3D Objects
*/
export namespace Object3DCulling {
/** Scratch objects: never returned to callers, never read across calls. */
const temporaryBox = new THREE.Box3();
const temporaryMatrix = new THREE.Matrix4();
const temporaryEuler = new THREE.Euler();
/**
* Scratch objects: never returned to callers, never read across calls.
* Allocated lazily at first use so that 2D-only games - which never load
* `three.js` - can still boot this namespace (it is included unconditionally).
*/
let temporaryBox: THREE.Box3 | null = null;
let temporaryMatrix: THREE.Matrix4 | null = null;
let temporaryEuler: THREE.Euler | null = null;
/**
* Write the object's conservative axis-aligned box into `box` and return it.
@@ -120,18 +134,26 @@ namespace gdjs {
* conservative, not tight - a non-axis-aligned rotation yields a larger box
* than the object's silhouette, which is the safe direction for culling.
*
* The box is rotated around the object's rotation center
* (`getCenter*InScene`), which is not always the box center (e.g. a
* `Model3D` whose origin is at its box corner). A negative size is
* normalized so it can never invert the box.
*
* Allocates nothing.
*/
export const computeAABB = (
object: Object3DBoxSource,
box: THREE.Box3
): THREE.Box3 => {
const halfWidth = object.getWidth() / 2;
const halfHeight = object.getHeight() / 2;
const halfDepth = object.getDepth() / 2;
const centerX = object.getDrawableX() + halfWidth;
const centerY = object.getDrawableY() + halfHeight;
const centerZ = object.getDrawableZ() + halfDepth;
const width = object.getWidth();
const height = object.getHeight();
const depth = object.getDepth();
const halfWidth = Math.abs(width) / 2;
const halfHeight = Math.abs(height) / 2;
const halfDepth = Math.abs(depth) / 2;
const centerX = object.getDrawableX() + width / 2;
const centerY = object.getDrawableY() + height / 2;
const centerZ = object.getDrawableZ() + depth / 2;
if (
object.getRotationX() === 0 &&
@@ -154,14 +176,16 @@ namespace gdjs {
// Half-extents of the rotated box: for each world axis, the sum of the
// absolute contributions of the three local half-extents.
temporaryEuler.set(
const matrix = temporaryMatrix || (temporaryMatrix = new THREE.Matrix4());
const euler = temporaryEuler || (temporaryEuler = new THREE.Euler());
euler.set(
gdjs.toRad(object.getRotationX()),
gdjs.toRad(object.getRotationY()),
gdjs.toRad(object.angle),
'ZYX'
);
temporaryMatrix.makeRotationFromEuler(temporaryEuler);
const elements = temporaryMatrix.elements;
matrix.makeRotationFromEuler(euler);
const elements = matrix.elements;
const halfExtentX =
Math.abs(elements[0]) * halfWidth +
Math.abs(elements[4]) * halfHeight +
@@ -175,15 +199,39 @@ namespace gdjs {
Math.abs(elements[6]) * halfHeight +
Math.abs(elements[10]) * halfDepth;
// Rotate the box center around the object's rotation center: the box sits
// at C' = P + R*(C - P), where P is the rotation pivot and C the box center.
const pivotX = object.getCenterXInScene();
const pivotY = object.getCenterYInScene();
const pivotZ = object.getCenterZInScene();
const offsetX = centerX - pivotX;
const offsetY = centerY - pivotY;
const offsetZ = centerZ - pivotZ;
const rotatedCenterX =
pivotX +
elements[0] * offsetX +
elements[4] * offsetY +
elements[8] * offsetZ;
const rotatedCenterY =
pivotY +
elements[1] * offsetX +
elements[5] * offsetY +
elements[9] * offsetZ;
const rotatedCenterZ =
pivotZ +
elements[2] * offsetX +
elements[6] * offsetY +
elements[10] * offsetZ;
box.min.set(
centerX - halfExtentX,
centerY - halfExtentY,
centerZ - halfExtentZ
rotatedCenterX - halfExtentX,
rotatedCenterY - halfExtentY,
rotatedCenterZ - halfExtentZ
);
box.max.set(
centerX + halfExtentX,
centerY + halfExtentY,
centerZ + halfExtentZ
rotatedCenterX + halfExtentX,
rotatedCenterY + halfExtentY,
rotatedCenterZ + halfExtentZ
);
return box;
};
@@ -196,8 +244,9 @@ namespace gdjs {
object: Object3DBoxSource,
frustum: THREE.Frustum
): boolean => {
computeAABB(object, temporaryBox);
return frustum.intersectsBox(temporaryBox);
const scratchBox = temporaryBox || (temporaryBox = new THREE.Box3());
computeAABB(object, scratchBox);
return frustum.intersectsBox(scratchBox);
};
}
+11 -4
View File
@@ -312,6 +312,14 @@ namespace gdjs {
object.onDestroyed();
}
// Release each layer's Model3D instance pool so a reloaded scene does not
// leak shared InstancedMeshes.
for (const name in this._layers.items) {
if (this._layers.items.hasOwnProperty(name)) {
this._layers.items[name].getRenderer().disposeModelInstancePool();
}
}
// Notify the renderer
if (this._renderer) {
this._renderer.onSceneUnloaded();
@@ -542,10 +550,9 @@ namespace gdjs {
const frustum = layerRenderer
? layerRenderer.getThreeFrustum()
: null;
const isVisible =
!object.isHidden() && (!frustum || object.isInFrustum(frustum));
const threeObject = object.get3DRendererObject();
if (threeObject) threeObject.visible = isVisible;
const culled = !!frustum && !object.isInFrustum(frustum);
object.setCullingVisible(culled);
const isVisible = !object.isHidden() && !culled;
// Update effects and perform the pre-render update only if the
// object is visible, like the 2D path below.
+2 -2
View File
@@ -172,10 +172,10 @@ module.exports = function (config) {
'./newIDE/app/resources/GDJS/Runtime/Extensions/TextObject/textruntimeobject-pixi-renderer.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/A_RuntimeObject3D.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/A_RuntimeObject3DRenderer.js',
// Loaded before the object that uses them.
// Mirror Extensions/3D/JsExtension.js addIncludeFile order.
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Scene3DTools.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Model3DInstancePool.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Model3DRuntimeObject.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Model3DInstancePool.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Model3DRuntimeObject3DRenderer.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Cube3DRuntimeObject.js',
'./newIDE/app/resources/GDJS/Runtime/Extensions/3D/Cube3DRuntimeObjectPixiRenderer.js',
-1
View File
@@ -1,4 +1,3 @@
// @ts-check
// @ts-nocheck The expect.js assertions take an optional message argument, which
// the installed `expect` typings do not declare.
+131 -61
View File
@@ -1,7 +1,6 @@
// @ts-check
// @ts-nocheck The runtime types do not describe the internals these tests drive
// (the instance pool slot, the resource manager registry), which the built
// runtime does expose.
// (the instance pool, the resource manager registry), which the built runtime
// does expose.
describe('Model3D instancing', function () {
/** The single-mesh model shipped for these tests. */
@@ -29,21 +28,23 @@ describe('Model3D instancing', function () {
return runtimeGame;
};
const make3DLayerData = (name) => ({
name,
renderingType: '3d',
cameraType: 'perspective',
visibility: true,
cameras: [],
effects: [],
ambientLightColorR: 255,
ambientLightColorG: 255,
ambientLightColorB: 255,
isLightingLayer: false,
followBaseLayerCamera: false,
});
const createSceneWith3DLayer = (runtimeGame) => {
const runtimeScene = new gdjs.RuntimeScene(runtimeGame);
runtimeScene.addLayer({
name: '',
renderingType: '3d',
cameraType: 'perspective',
visibility: true,
cameras: [],
effects: [],
ambientLightColorR: 255,
ambientLightColorG: 255,
ambientLightColorB: 255,
isLightingLayer: false,
followBaseLayerCamera: false,
});
runtimeScene.addLayer(make3DLayerData(''));
return runtimeScene;
};
@@ -104,79 +105,148 @@ describe('Model3D instancing', function () {
const countGroups = (runtimeScene) =>
getThreeChildren(runtimeScene).filter((child) => child.isGroup).length;
it('shares one InstancedMesh between instanced models and keeps the clone path for the others', async () => {
const getPoolKey = (runtimeScene) => getLayerPool(runtimeScene).getKeys()[0];
it('shares one InstancedMesh between same-key models and draws no clones', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = createSceneWith3DLayer(runtimeGame);
const first = createModel3D(runtimeScene, 'First', true);
const second = createModel3D(runtimeScene, 'Second', true);
const third = createModel3D(runtimeScene, 'Third', true);
// Not opted in: must keep the per-object clone.
const cloned = createModel3D(runtimeScene, 'Cloned', false);
createModel3D(runtimeScene, 'First', true);
createModel3D(runtimeScene, 'Second', true);
runtimeScene.render();
// the three instanced models share one InstancedMesh
// (i) one InstancedMesh in the layer group and no per-object clones.
expect(countInstancedMeshes(runtimeScene)).to.be(1);
// the fourth, non-instanced model still gets its own clone
expect(countGroups(runtimeScene)).to.be(1);
// moving the first instance rewrites its matrix
expect(getLayerPool(runtimeScene).getKeys().length).to.be(1);
expect(first.getRenderer().getInstanceSlot()).to.be(0);
expect(second.getRenderer().getInstanceSlot()).to.be(1);
expect(third.getRenderer().getInstanceSlot()).to.be(2);
expect(cloned.getRenderer().getInstanceSlot()).to.be(null);
expect(countGroups(runtimeScene)).to.be(0);
});
it('writes the instance matrix of each instance when it moves', async () => {
it('grows past the initial capacity without throwing or losing matrices', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = createSceneWith3DLayer(runtimeGame);
// 16 fill the initial capacity; the 17th grows the mesh.
for (let i = 0; i < 16; i++) {
const object = createModel3D(runtimeScene, 'O' + i, true);
object.setX(i * 1000);
}
runtimeScene.render();
// (ii) the 17th acquire grows the mesh (pre-fix: `TypeError` on `null.add`).
const seventeenth = createModel3D(runtimeScene, 'O16', true);
seventeenth.setX(16000);
// Slots 0..15 keep the matrices written before the grow.
const instancedMesh = getLayerPool(runtimeScene).getInstancedMesh(
getPoolKey(runtimeScene)
);
expect(instancedMesh).to.be.ok();
const matrix = new THREE.Matrix4();
for (let slot = 0; slot < 16; slot++) {
instancedMesh.getMatrixAt(slot, matrix);
// A lost/collapsed slot would have a zero scale; the grow copies it as-is.
expect(matrix.elements[0]).to.not.be(0);
}
});
it('collapses the slot of a deleted model to zero scale', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = createSceneWith3DLayer(runtimeGame);
const first = createModel3D(runtimeScene, 'First', true);
const second = createModel3D(runtimeScene, 'Second', true);
createModel3D(runtimeScene, 'Second', true);
runtimeScene.render();
const key = getLayerPool(runtimeScene).getKeys()[0];
const instancedMesh = getLayerPool(runtimeScene).getInstancedMesh(key);
expect(instancedMesh).to.be.ok();
const instancedMesh = getLayerPool(runtimeScene).getInstancedMesh(
getPoolKey(runtimeScene)
);
const matrix = new THREE.Matrix4();
instancedMesh.getMatrixAt(0, matrix);
const firstX = matrix.elements[12];
expect(matrix.elements[0]).to.not.be(0);
first.setX(1000);
// (iii) deleting the first model collapses its slot (pre-fix: drawn forever).
runtimeScene.markObjectForDeletion(first);
runtimeScene.render();
instancedMesh.getMatrixAt(0, matrix);
expect(matrix.elements[12]).to.be.above(firstX);
instancedMesh.getMatrixAt(1, matrix);
const secondX = matrix.elements[12];
second.setX(2000);
runtimeScene.render();
instancedMesh.getMatrixAt(1, matrix);
// the freed slot 0 is handed to the next instanced model
expect(matrix.elements[12]).to.be.above(secondX);
expect(matrix.elements[0]).to.be(0);
});
it('frees a slot when an instanced model leaves the scene, for reuse', async () => {
it('rewrites the instance matrix on rotation', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = createSceneWith3DLayer(runtimeGame);
const first = createModel3D(runtimeScene, 'First', true);
const second = createModel3D(runtimeScene, 'Second', true);
const object = createModel3D(runtimeScene, 'First', true);
runtimeScene.render();
expect(first.getRenderer().getInstanceSlot()).to.be(0);
expect(second.getRenderer().getInstanceSlot()).to.be(1);
const instancedMesh = getLayerPool(runtimeScene).getInstancedMesh(
getPoolKey(runtimeScene)
);
const matrix = new THREE.Matrix4();
instancedMesh.getMatrixAt(0, matrix);
const before = matrix.elements[0];
// Removing the first object gives slot 0 back.
runtimeScene.markObjectForDeletion(first);
// (iv) a rotation reaches the instance matrix (pre-fix: unchanged).
object.setAngle(Math.PI / 2);
runtimeScene.render();
instancedMesh.getMatrixAt(0, matrix);
expect(matrix.elements[0]).to.not.be(before);
});
it('collapses the slot on hide and restores it on unhide', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = createSceneWith3DLayer(runtimeGame);
const object = createModel3D(runtimeScene, 'First', true);
runtimeScene.render();
const third = createModel3D(runtimeScene, 'Third', true);
const instancedMesh = getLayerPool(runtimeScene).getInstancedMesh(
getPoolKey(runtimeScene)
);
const matrix = new THREE.Matrix4();
// (v) hide collapses the slot; unhide restores the transform matrix.
object.hide(true);
runtimeScene.render();
expect(third.getRenderer().getInstanceSlot()).to.be(0);
instancedMesh.getMatrixAt(0, matrix);
expect(matrix.elements[0]).to.be(0);
object.hide(false);
runtimeScene.render();
instancedMesh.getMatrixAt(0, matrix);
expect(matrix.elements[0]).to.not.be(0);
});
it('moves the instance matrix to the new layer pool on a layer change', async () => {
const runtimeGame = await makeGameWithSingleMeshModel();
const runtimeScene = new gdjs.RuntimeScene(runtimeGame);
runtimeScene.addLayer(make3DLayerData(''));
runtimeScene.addLayer(make3DLayerData('other'));
const object = createModel3D(runtimeScene, 'First', true);
runtimeScene.render();
const sourceMesh = runtimeScene
.getLayer('')
.getRenderer()
.getModelInstancePool()
.getInstancedMesh(
runtimeScene.getLayer('').getRenderer().getModelInstancePool().getKeys()[0]
);
const matrix = new THREE.Matrix4();
sourceMesh.getMatrixAt(0, matrix);
const before = matrix.elements[0];
// (vi) the slot is re-acquired from the destination layer's pool.
object.setLayer('other');
runtimeScene.render();
const targetPool = runtimeScene
.getLayer('other')
.getRenderer()
.getModelInstancePool();
const targetMesh = targetPool.getInstancedMesh(targetPool.getKeys()[0]);
expect(targetMesh).to.be.ok();
targetMesh.getMatrixAt(0, matrix);
expect(matrix.elements[0]).to.be(before);
});
it('silently keeps the clone path for a Basic material', async () => {
@@ -184,10 +254,10 @@ describe('Model3D instancing', function () {
const runtimeScene = createSceneWith3DLayer(runtimeGame);
// "Basic" replaces every material per instance: not shareable.
const object = createModel3D(runtimeScene, 'Basic', true, 'Basic');
createModel3D(runtimeScene, 'Basic', true, 'Basic');
runtimeScene.render();
expect(object.getRenderer().getInstanceSlot()).to.be(null);
// Materials/skins/animation clips never take the instanced path.
expect(countInstancedMeshes(runtimeScene)).to.be(0);
expect(countGroups(runtimeScene)).to.be(1);
});
+52 -4
View File
@@ -1004,7 +1004,7 @@ export const GDEVELOP_OPENAI_TOOLS: Array<{|
description: 'List of task IDs this task depends on.',
},
},
required: ['id', 'title', 'status'],
required: ['id', 'title', 'description', 'status'],
},
},
},
@@ -2260,10 +2260,15 @@ export const customAddMessageToAiRequest = async ({
});
}
// The role a studio sub-agent was spawned with must shape every turn, not
// just the first: keep its prompt and tool subset.
const studioRoleId = existing.studioRoleId || null;
const systemPrompt = buildSystemPrompt({
gameProjectJson,
projectSpecificExtensionsSummaryJson,
mode: mode || existing.mode,
role: studioRoleId,
});
const openAiMessages = transformGDevelopMessagesToOpenAi(
@@ -2273,7 +2278,9 @@ export const customAddMessageToAiRequest = async ({
const assistantResponse = await sendChatCompletion({
messages: openAiMessages,
tools: GDEVELOP_OPENAI_TOOLS,
tools: studioRoleId
? getToolsForRole((studioRoleId: any), GDEVELOP_OPENAI_TOOLS)
: GDEVELOP_OPENAI_TOOLS,
});
const assistantMsgId = `msg-asst-${Date.now()}`;
@@ -2284,6 +2291,23 @@ export const customAddMessageToAiRequest = async ({
output.push(assistantMessage);
const currentCachedRequest = localAiRequestsCache[aiRequestId] || existing;
// A suggestion or suspension write that landed while the model was
// answering must not be discarded (W5): re-insert cache-only messages
// before the turn's own additions.
const turnIds = new Set(output.map(message => message.messageId));
const cacheOnlyMessages = (currentCachedRequest.output || []).filter(
message => message && !turnIds.has(message.messageId)
);
let merged = output;
if (cacheOnlyMessages.length > 0) {
const appendedThisTurn = (userMessage && userMessage.trim() ? 1 : 0) + 1;
const turnStart = Math.max(0, output.length - appendedThisTurn);
merged = [
...output.slice(0, turnStart),
...cacheOnlyMessages,
...output.slice(turnStart),
];
}
const updatedAiRequest: AiRequest = {
// Read the cache again, inside the lock: a suspend or a suggestion write
// that landed while the model was answering must not be discarded.
@@ -2294,7 +2318,7 @@ export const customAddMessageToAiRequest = async ({
// can read what the agent produced before it was stopped.
status:
currentCachedRequest.status === 'suspended' ? 'suspended' : 'ready',
output,
output: merged,
};
localAiRequestsCache[aiRequestId] = updatedAiRequest;
@@ -2384,6 +2408,7 @@ export const customCreateSubAgentAiRequest = async ({
toolsVersion: 'v14',
toolOptions: null,
parentAiRequestId: parentAiRequestId,
studioRoleId: roleId,
error: null,
output,
lastUserMessagePriceInCredits: 0,
@@ -2459,18 +2484,41 @@ export const customSuspendAiRequest = (aiRequestId: string): AiRequest => {
if (existing) {
// Replace the cached request rather than mutating it in place: a model turn
// may be holding a reference to it and will write its own copy back.
const now = new Date().toISOString();
const suspended: AiRequest = {
...existing,
status: 'suspended',
updatedAt: new Date().toISOString(),
updatedAt: now,
};
localAiRequestsCache[aiRequestId] = suspended;
// A parent Stop must not leave BYOK-burning children running: suspend every
// sub-agent of this request too.
for (const key of Object.keys(localAiRequestsCache)) {
const child = localAiRequestsCache[key];
if (child && child.parentAiRequestId === aiRequestId) {
localAiRequestsCache[key] = {
...child,
status: 'suspended',
updatedAt: now,
};
}
}
saveLocalAiRequests();
return suspended;
}
return customGetAiRequest(aiRequestId);
};
/**
* Client-side update write-through: a mutated request is written back to the
* local cache so plan flips and the loop guard's `status: 'error'` are not
* reverted by the next cache fetch. Content-idempotent.
*/
export const customUpdateAiRequest = (aiRequest: AiRequest): void => {
localAiRequestsCache[aiRequest.id] = aiRequest;
saveLocalAiRequests();
};
/**
* Client-side Fork AI Request.
*/
+94
View File
@@ -14,7 +14,9 @@ import {
LOCAL_BYOK_USER_ID,
_resetCustomAiClientForTesting,
customCreateAiRequest,
customCreateSubAgentAiRequest,
customAddMessageToAiRequest,
customUpdateAiRequest,
customGetAiRequest,
customGetAiRequests,
customGetAiRequestStatuses,
@@ -27,6 +29,8 @@ import {
testConnection,
} from './CustomAIClient';
import { getToolsForRole } from '../AiGeneration/Studio/Roles';
jest.mock('axios');
describe('CustomAIClient', () => {
@@ -616,4 +620,94 @@ describe('CustomAIClient', () => {
);
});
});
describe('Studio sub-agent role enforcement', () => {
/** Mock one assistant reply so a turn completes. */
const mockAssistantReply = (content: string) => {
// $FlowFixMe
axios.post.mockResolvedValueOnce({
status: 200,
data: { choices: [{ message: { role: 'assistant', content } }] },
});
};
const spawnTester = async () => {
mockAssistantReply('Spawned.');
return customCreateSubAgentAiRequest({
parentAiRequestId: 'local-ai-parent',
roleId: 'tester',
userRequest: 'Verify the grid.',
gameProjectJson: null,
projectSpecificExtensionsSummaryJson: null,
spawnContextNote: null,
});
};
const sentToolNames = (callIndex: number) => {
// $FlowFixMe
const body = axios.post.mock.calls[callIndex][1];
return {
names: (body.tools || []).map(tool => tool.function.name),
systemPrompt: body.messages[0].content,
};
};
it('offers a spawned sub-agent only its role tools and prompt', async () => {
await spawnTester();
const { names, systemPrompt } = sentToolNames(0);
const testerToolNames = getToolsForRole(
'tester',
GDEVELOP_OPENAI_TOOLS
).map(tool => tool.function.name);
expect(names.length).toBeGreaterThan(0);
names.forEach(name => expect(testerToolNames).toContain(name));
expect(systemPrompt).toContain('QA tester');
});
it('keeps the role tool subset and prompt on later turns', async () => {
const child = await spawnTester();
// A second turn - the critical regression: pre-fix this offered all the
// tools and dropped the role prompt.
mockAssistantReply('Done.');
await customAddMessageToAiRequest({
aiRequestId: child.id,
userMessage: '',
functionCallOutputs: [],
});
const { names, systemPrompt } = sentToolNames(1);
const testerToolNames = getToolsForRole(
'tester',
GDEVELOP_OPENAI_TOOLS
).map(tool => tool.function.name);
expect(names.length).toBeGreaterThan(0);
names.forEach(name => expect(testerToolNames).toContain(name));
expect(systemPrompt).toContain('QA tester');
});
it('writes a mutated request through to the cache', async () => {
const child = await spawnTester();
const before = (child.output || []).length;
const marker: any = {
type: 'message',
role: 'assistant',
status: 'completed',
content: [
{
type: 'output_text',
status: 'completed',
text: 'marker',
annotations: [],
},
],
messageId: 'msg-marker',
};
customUpdateAiRequest({
...child,
output: [...(child.output || []), marker],
});
expect(customGetAiRequest(child.id).output.length).toBe(before + 1);
});
});
});
@@ -33,6 +33,7 @@ import {
getUserRequestText,
} from './AiRequestUtils';
import { type EditApprovalRequest } from './Utils';
import { customUpdateAiRequest } from '../AI/CustomAIClient';
type EditorFunctionCallResultsStorage = {|
getEditorFunctionCallResults: (
@@ -374,6 +375,11 @@ export const useAiRequestsStorage = (): AiRequestStorage => {
const newAiRequest = updateFn(
previousState.aiRequests[aiRequestId] || null
);
// Write a local (BYOK) request through to the cache so plan flips and
// the loop guard's error state are not reverted by the next cache fetch.
if (aiRequestId.startsWith('local-ai-')) {
customUpdateAiRequest(newAiRequest);
}
return {
...previousState,
aiRequests: {
@@ -839,17 +839,17 @@ export const AskAiEditor: React.ComponentType<Props> = React.memo<Props>(
console.info(
'Skipping send for AI request: some function call results are not finished yet.'
);
return;
return false;
}
if (hasFunctionsCallsToProcess) {
console.info(
'Skipping send for AI request: there are still function calls to process.'
);
return;
return false;
}
// If nothing to send, stop there.
if (functionCallOutputs.length === 0 && !userMessage) return;
if (functionCallOutputs.length === 0 && !userMessage) return false;
// Paying with credits is only when a user message is sent (and quota is exhausted).
let payWithCredits = false;
@@ -1004,6 +1004,7 @@ export const AskAiEditor: React.ComponentType<Props> = React.memo<Props>(
onOpenExternalLayout(externalLayoutName);
});
}
return true;
},
[
profile,
@@ -1069,7 +1070,7 @@ export const AskAiEditor: React.ComponentType<Props> = React.memo<Props>(
createdProject?: ?gdProject,
|}
) => {
await onSendMessage({
return onSendMessage({
aiRequestId,
userMessage: '',
createdProject: options.createdProject,
@@ -1112,6 +1113,7 @@ export const AskAiEditor: React.ComponentType<Props> = React.memo<Props>(
editorCallbacks,
aiRequestsToProcess,
onSendEditorFunctionCallResults,
isStudioEnabled: true,
getEditorFunctionCallResults,
addEditorFunctionCallResults,
onSceneEventsModifiedOutsideEditor,
@@ -494,13 +494,13 @@ export const AskAiStandAloneForm = ({
// If anything is not finished yet, stop there (we only send all
// results at once, AI do not support partial results).
if (hasUnfinishedResult) return;
if (hasFunctionsCallsToProcess) return;
if (hasUnfinishedResult) return false;
if (hasFunctionsCallsToProcess) return false;
// If nothing to send, stop there.
// When in a standalone form, this can happen if the agent did not
// decide to create a project, in this case, abort and clear the form.
if (functionCallOutputs.length === 0) return;
if (functionCallOutputs.length === 0) return false;
try {
setSendingAiRequest(aiRequestId, true);
@@ -578,6 +578,7 @@ export const AskAiStandAloneForm = ({
// is up-to-date after an AI request.
await delay(500);
await refreshLimits({ withRetry: true });
return true;
},
[
profile,
@@ -605,7 +606,7 @@ export const AskAiStandAloneForm = ({
createdProject?: ?gdProject,
|}
) => {
await onSendMessage({
return onSendMessage({
aiRequestId,
userMessage: '',
createdSceneNames: options.createdSceneNames,
@@ -631,6 +632,7 @@ export const AskAiStandAloneForm = ({
editorCallbacks,
aiRequestsToProcess,
onSendEditorFunctionCallResults,
isStudioEnabled: false,
getEditorFunctionCallResults,
addEditorFunctionCallResults,
i18n,
@@ -68,11 +68,28 @@ export const isSubAgentAtTurnCap = ({
export const isSubAgentFinished = ({
subAgentRequest,
editorFunctionCallResults,
aiRequests,
}: {|
subAgentRequest: AiRequest,
editorFunctionCallResults: Array<EditorFunctionCallResult> | null,
aiRequests?: { [string]: AiRequest } | null,
|}): boolean => {
if (subAgentRequest.status !== 'ready') return false;
// A sub-agent at its turn cap is finalized with its last message even if it
// still has calls left: the dispatcher has stopped issuing its turns.
if (
aiRequests &&
isSubAgentAtTurnCap({ aiRequest: subAgentRequest, aiRequests })
) {
return true;
}
// The loop guard's terminal state is 'error'; treat it as finished so the
// studio writes its (failed) report back instead of waiting forever.
if (
subAgentRequest.status !== 'ready' &&
subAgentRequest.status !== 'error'
) {
return false;
}
if (
getFunctionCallsToProcess({
aiRequest: subAgentRequest,
@@ -92,21 +109,56 @@ export const isSubAgentFinished = ({
) {
return false;
}
// A result that was executed but whose output has not landed in the child's
// transcript yet: finalizing now would drop the child's real wrap-up forever.
const writtenBackCallIds = new Set(
(subAgentRequest.output || [])
.filter(message => message.type === 'function_call_output')
.map(message => (message: any).call_id)
);
if (
editorFunctionCallResults &&
editorFunctionCallResults.some(
result => !writtenBackCallIds.has(result.call_id)
)
) {
return false;
}
return true;
};
/** Reports are capped so one runaway sub-agent cannot flood the parent. */
export const MAX_SUB_AGENT_REPORT_LENGTH = 4000;
/**
* The text written into the parent's `function_call_output`: the sub-agent's
* last assistant message, prefixed with the role's display name.
*
* The message's text lives in a `content` entry whose type is `'output_text'`
* for a message the server produced, and `'text'` for one the local BYOK path
* parsed (`parseAssistantMessage` emits `'text'`); both are read, and the
* message's own `text` field is the fallback, so a report is never lost to the
* difference.
* last assistant message, prefixed with any failure or turn-limit context and
* suffixed with its failed calls, then truncated. The message's text is read
* from a `content` entry (`'output_text'` from the server, `'text'` from the
* local BYOK parser) with the message's own `text` field as fallback.
*/
export const buildSubAgentReport = (subAgentRequest: AiRequest): string => {
export const buildSubAgentReport = (
subAgentRequest: AiRequest,
aiRequests?: { [string]: AiRequest } | null
): string => {
const output = subAgentRequest.output || [];
let report = '';
if (subAgentRequest.error) {
report += `Failed: ${subAgentRequest.error.message}\n\n`;
}
if (
aiRequests &&
isSubAgentAtTurnCap({ aiRequest: subAgentRequest, aiRequests })
) {
report += 'Reached its turn limit.\n\n';
}
// The sub-agent's last assistant message is the core of the report. Its text
// lives in a `content` entry whose type is `'output_text'` for a message the
// server produced, and `'text'` for one the local BYOK path parsed; both are
// read, and the message's own `text` field is the fallback.
let core = '(no report)';
for (let i = output.length - 1; i >= 0; i--) {
const message = output[i];
if (message.type !== 'message' || message.role !== 'assistant') continue;
@@ -115,13 +167,39 @@ export const buildSubAgentReport = (subAgentRequest: AiRequest): string => {
);
const textFromContent = textContent ? (textContent: any).text : null;
if (typeof textFromContent === 'string' && textFromContent) {
return textFromContent;
core = textFromContent;
} else {
const messageText = (message: any).text;
core =
typeof messageText === 'string' && messageText
? messageText
: '(no report)';
}
const messageText = (message: any).text;
if (typeof messageText === 'string' && messageText) return messageText;
return '(no report)';
break;
}
return '(no report)';
report += core;
// Surface failed tool calls so the parent sees what went wrong.
const failedMessages = output
.filter(
message =>
message.type === 'function_call_output' &&
(message: any).success === false
)
.map(message => {
const messageOutput = (message: any).output;
return (messageOutput && messageOutput.message) || '(failed call)';
});
if (failedMessages.length > 0) {
report += `\n\nFailed calls:\n- ${failedMessages.join('\n- ')}`;
}
if (report.length > MAX_SUB_AGENT_REPORT_LENGTH) {
return (
report.slice(0, MAX_SUB_AGENT_REPORT_LENGTH) + '\n…(report truncated)'
);
}
return report;
};
/**
@@ -38,6 +38,17 @@ const spawnCall = (
...(subAgentId ? { subAgentAiRequestId: subAgentId } : {}),
});
const functionCallOutput = (
callId: string,
success: boolean,
message?: string
): any => ({
type: 'function_call_output',
call_id: callId,
success,
output: message !== undefined ? { message } : {},
});
const makeSubAgent = (overrides: any): any => ({
id: 'child-1',
createdAt: '2026-01-01T00:00:00.000Z',
@@ -125,7 +136,13 @@ describe('FinalizeSubAgents', () => {
).toBe(true);
expect(
isSubAgentFinished({
subAgentRequest: makeSubAgent({}),
subAgentRequest: makeSubAgent({
output: [
assistantMessage('Done.'),
// The result's output has landed in the transcript (written back).
functionCallOutput('call-y', true),
],
}),
editorFunctionCallResults: [
{
status: 'finished',
@@ -137,6 +154,42 @@ describe('FinalizeSubAgents', () => {
})
).toBe(true);
});
it('treats a sub-agent that errored as finished (4i)', () => {
expect(
isSubAgentFinished({
subAgentRequest: makeSubAgent({
status: 'error',
error: { code: 'repeated-tool-call-loop', message: 'stuck' },
}),
editorFunctionCallResults: null,
})
).toBe(true);
});
it('is false for an executed-but-unsent result (4i)', () => {
expect(
isSubAgentFinished({
subAgentRequest: makeSubAgent({}),
editorFunctionCallResults: [
{
status: 'finished',
call_id: 'call-x',
success: false,
output: { message: 'boom' },
},
],
})
).toBe(false);
});
it('is false for an aborted/timeout-shaped request', () => {
expect(
isSubAgentFinished({
subAgentRequest: makeSubAgent({ status: 'aborted' }),
editorFunctionCallResults: null,
})
).toBe(false);
});
});
describe('buildSubAgentReport', () => {
+59 -19
View File
@@ -45,14 +45,14 @@ export type LoopGuardDecision = {|
export const DEFAULT_LOOP_GUARD_CONFIG = {
enabled: true,
/** Consecutive identical tool calls (same name AND same arguments) allowed. */
/** Trips on the Nth consecutive identical call; N-1 are tolerated. */
repeatedToolLimit: 8,
/** Consecutive failed tool calls allowed. */
/** Trips on the Nth consecutive failed call; N-1 are tolerated. */
errorStormLimit: 5,
/**
* Assistant turns allowed without a user message in between. Generous on
* purpose: a legitimate long task (a `run_script` doing hundreds of calls, a
* gameplay test) is not a loop.
* Trips on the Nth assistant turn without a user message; N-1 are tolerated.
* Generous on purpose: a legitimate long task (a `run_script` doing hundreds
* of calls, a gameplay test) is not a loop.
*/
maxTurnsWithoutUserInput: 60,
};
@@ -80,24 +80,53 @@ const actionFor = (level: LoopGuardLevel): LoopGuardAction => {
return 'none';
};
/** Recursively stringify with object keys sorted, so key order is not identity. */
const stableStringify = (value: any): string => {
if (Array.isArray(value)) {
return `[${value.map(stableStringify).join(',')}]`;
}
if (value && typeof value === 'object') {
const keys = Object.keys(value).sort();
return `{${keys
.map(key => `${JSON.stringify(key)}:${stableStringify(value[key])}`)
.join(',')}}`;
}
return JSON.stringify(value);
};
/**
* The identity of a tool call: its name and a hash of its **whole** arguments
* string.
* Parse `argsJson` and re-emit it canonically (keys sorted) so two argument
* objects that differ only in key order hash the same. A non-JSON string is
* hashed as-is.
*/
const canonicalizeArgs = (argsJson: string): string => {
try {
return stableStringify(JSON.parse(argsJson || 'null'));
} catch (error) {
return argsJson || '';
}
};
/**
* The identity of a tool call: its name and a hash of its arguments, taken
* whole and in canonical (key-sorted) form.
*
* Hashing the whole string is the point. munder-difflin's #377 records that its
* old `input.slice(0, 200)` collided on Bash commands sharing a long identical
* preamble - nine *different* measurements in a row read as nine identical calls
* and constrained the agent. Different arguments must produce different keys,
* however long their common prefix.
* Hashing the whole arguments is the point. munder-difflin's #377 records that
* its old `input.slice(0, 200)` collided on Bash commands sharing a long
* identical preamble - nine *different* measurements in a row read as nine
* identical calls and constrained the agent. Different arguments must produce
* different keys, however long their common prefix; canonicalizing means two
* calls that differ only in argument key order are the *same* call.
*/
export const toolCallKey = (name: string, argsJson: string): string => {
const shaObj = new jsSHA('SHA-256', 'TEXT', { encoding: 'UTF8' });
shaObj.update(argsJson || '');
shaObj.update(canonicalizeArgs(argsJson));
return `${name}:${shaObj.getHash('HEX')}`;
};
export type LoopGuard = {|
recordToolCall: (call: {| name: string, arguments: string |}) => void,
recordSuccess: () => void,
recordError: () => void,
recordTurn: () => void,
reset: () => void,
@@ -118,6 +147,7 @@ export const createLoopGuard = (
let repeatCount = 0;
let errorCount = 0;
let turnCount = 0;
let actionEmittedForTrip = false;
const evaluateTrips = (): {| tripping: boolean, reason: string |} => {
if (repeatCount >= effectiveConfig.repeatedToolLimit) {
@@ -151,11 +181,12 @@ export const createLoopGuard = (
} else {
repeatKey = key;
repeatCount = 1;
// A distinct tool call is progress: it clears the error storm
// (munder-difflin `breaker.ts` `recordToolUse`).
errorCount = 0;
}
},
recordSuccess: () => {
// Only a real success clears the error storm: an attempt is not progress.
errorCount = 0;
},
recordError: () => {
errorCount++;
},
@@ -171,6 +202,7 @@ export const createLoopGuard = (
repeatCount = 0;
errorCount = 0;
turnCount = 0;
actionEmittedForTrip = false;
},
evaluate: () => {
if (!effectiveConfig.enabled) {
@@ -200,12 +232,20 @@ export const createLoopGuard = (
level = target;
reason = trip.tripping ? trip.reason : changed ? 'recovering' : reason;
// Emit on an escalation (a new level carries its own action) or on a
// fresh trip after a reset - but never re-send a durable action on every
// tick. Re-armed by `reset` and on recovery.
const shouldEmit = escalated || (trip.tripping && !actionEmittedForTrip);
if (shouldEmit) {
actionEmittedForTrip = true;
} else if (!trip.tripping) {
actionEmittedForTrip = false;
}
return {
level,
reason,
// Only an escalation carries an action, so a durable steer is not
// re-sent on every tick.
action: escalated ? actionFor(target) : 'none',
action: shouldEmit ? actionFor(target) : 'none',
changed,
};
},
@@ -92,15 +92,23 @@ describe('LoopGuard', () => {
expect(decision.reason).toContain('5');
});
it('is reset by a successful (distinct) tool call', () => {
const guard = createLoopGuard({ errorStormLimit: 3 });
guard.recordError();
guard.recordError();
guard.recordToolCall({ name: 'create_scene', arguments: '{}' });
guard.recordError();
guard.recordError();
// Only two consecutive errors since the successful call.
expect(guard.evaluate().level).toBe('healthy');
it('is reset by a success but not by a mere attempt', () => {
// A distinct tool call is an attempt, not progress: the storm continues.
const attempt = createLoopGuard({ errorStormLimit: 3 });
attempt.recordError();
attempt.recordError();
attempt.recordToolCall({ name: 'create_scene', arguments: '{}' });
attempt.recordError();
expect(attempt.evaluate().level).toBe('steering');
// A real success clears the storm.
const success = createLoopGuard({ errorStormLimit: 3 });
success.recordError();
success.recordError();
success.recordSuccess();
success.recordError();
success.recordError();
expect(success.evaluate().level).toBe('healthy');
});
});
@@ -211,6 +219,46 @@ describe('LoopGuard', () => {
});
});
describe('canonicalization and re-arming', () => {
it('treats arguments that differ only in key order as the same call', () => {
expect(toolCallKey('create_scene', '{"a":1,"b":2}')).toBe(
toolCallKey('create_scene', '{"b":2,"a":1}')
);
});
it('is not tripped by a benign A-B-A-B alternation', () => {
const guard = createLoopGuard({ repeatedToolLimit: 3 });
for (const args of [
'{"a":1}',
'{"a":2}',
'{"a":1}',
'{"a":2}',
'{"a":1}',
]) {
guard.recordToolCall({ name: 'create_scene', arguments: args });
}
expect(guard.evaluate().level).toBe('healthy');
});
it('re-emits an action on a re-trip after reset at the ceiling', () => {
const guard = createLoopGuard({
repeatedToolLimit: 1,
maxTurnsWithoutUserInput: 1000,
});
guard.recordToolCall({ name: 'a', arguments: '{}' });
guard.evaluate(); // steering
guard.evaluate(); // constrained
guard.evaluate(); // constrained (ceiling, durable: no action)
expect(guard.evaluate().action).toBe('none');
guard.reset();
guard.recordToolCall({ name: 'a', arguments: '{}' });
const retripped = guard.evaluate();
expect(retripped.level).toBe('constrained');
expect(retripped.action).toBe('constrain');
});
});
it('defaults are the documented ones', () => {
expect(DEFAULT_LOOP_GUARD_CONFIG).toEqual({
enabled: true,
@@ -23,7 +23,7 @@ export type StudioPlanTask = {|
title: string,
description: string,
status: StudioPlanTaskStatus,
dependsOn: Array<string>,
dependsOn?: Array<string>,
/**
* Set by the studio when the task is delegated, so the plan row can be linked
* to the sub-agent's function call (matches
@@ -74,7 +74,9 @@ export const areTaskDependenciesSatisfied = (
.filter(otherTask => otherTask.status === 'done')
.map(otherTask => otherTask.id)
);
return task.dependsOn.every(dependencyId => doneIds.has(dependencyId));
return (task.dependsOn || []).every(dependencyId =>
doneIds.has(dependencyId)
);
};
/** The first pending task whose dependencies are satisfied, or null. */
@@ -118,6 +120,32 @@ export const mergePlanTasks = (
});
};
/**
* Fold the plan inside a finished tool result's `output` over the plan the
* request already has. `output` is the object `EditorFunctionCallRunner`
* builds (`{ success, meta, ...output }`), so it is read as an object - never
* parsed. A result without a plan, or a non-success one, passes through
* untouched.
*/
export const mergePlanResultOutput = (
result: Object,
existingTasks: Array<StudioPlanTask> | null
): Object => {
if (result.status !== 'finished' || !result.success) return result;
const output: any = result.output;
if (!output || !output.plan || !Array.isArray(output.plan.tasks)) {
return result;
}
if (!existingTasks) return result;
return {
...result,
output: {
...output,
plan: { tasks: mergePlanTasks(existingTasks, output.plan.tasks) },
},
};
};
/**
* Apply `patch` to one task, leaving every other task — and every other field of
* the patched task — untouched.
@@ -155,6 +183,22 @@ const describeValue = (value: any): string => {
* sent it, naming the first offending index. A model mistake becomes a normal
* failed tool call, never a thrown error inside the editor.
*/
// Bounds on a plan the model sends, so a runaway tool call cannot flood the
// store (W12).
const MAX_PLAN_TASKS = 32;
const MAX_PLAN_TASK_ID_LENGTH = 64;
const MAX_PLAN_TASK_TITLE_LENGTH = 200;
const MAX_PLAN_TASK_DESCRIPTION_LENGTH = 1000;
const ALLOWED_TASK_FIELDS = [
'id',
'title',
'description',
'status',
'dependsOn',
'agentCallId',
];
export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
if (!Array.isArray(rawTasks)) {
return {
@@ -162,8 +206,17 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
message: 'create_or_update_plan requires a tasks array.',
};
}
if (rawTasks.length > MAX_PLAN_TASKS) {
return {
success: false,
message: `create_or_update_plan accepts at most ${MAX_PLAN_TASKS} tasks, got ${
rawTasks.length
}.`,
};
}
const tasks: Array<StudioPlanTask> = [];
const seenIds = new Set<string>();
for (let index = 0; index < rawTasks.length; index++) {
const rawTask = rawTasks[index];
const invalid = (detail: string): StudioPlanTaskValidation => ({
@@ -174,11 +227,36 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
if (!rawTask || typeof rawTask !== 'object' || Array.isArray(rawTask)) {
return invalid(`expected an object, got ${describeValue(rawTask)}.`);
}
// Fail closed (I11): a field this validator does not model would be
// silently dropped by a rebuild, so reject the whole task instead.
for (const key of Object.keys(rawTask)) {
if (!ALLOWED_TASK_FIELDS.includes(key)) {
return invalid(`unexpected field ${JSON.stringify(key)}.`);
}
}
if ('agentCallId' in rawTask && typeof rawTask.agentCallId !== 'string') {
return invalid(
`\`agentCallId\` must be a string when present, got ${describeValue(
rawTask.agentCallId
)}.`
);
}
if (typeof rawTask.id !== 'string' || !rawTask.id.trim()) {
return invalid(
`\`id\` must be a non-empty string, got ${describeValue(rawTask.id)}.`
);
}
// Validate and store the trimmed id (I12); every later match is on it.
const id = rawTask.id.trim();
if (id.length > MAX_PLAN_TASK_ID_LENGTH) {
return invalid(
`\`id\` must be at most ${MAX_PLAN_TASK_ID_LENGTH} characters.`
);
}
if (seenIds.has(id)) {
return invalid(`duplicate \`id\` ${JSON.stringify(id)}.`);
}
seenIds.add(id);
if (typeof rawTask.title !== 'string' || !rawTask.title.trim()) {
return invalid(
`\`title\` must be a non-empty string, got ${describeValue(
@@ -186,6 +264,11 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
)}.`
);
}
if (rawTask.title.trim().length > MAX_PLAN_TASK_TITLE_LENGTH) {
return invalid(
`\`title\` must be at most ${MAX_PLAN_TASK_TITLE_LENGTH} characters.`
);
}
if (
typeof rawTask.description !== 'string' ||
!rawTask.description.trim()
@@ -196,6 +279,11 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
)}.`
);
}
if (rawTask.description.trim().length > MAX_PLAN_TASK_DESCRIPTION_LENGTH) {
return invalid(
`\`description\` must be at most ${MAX_PLAN_TASK_DESCRIPTION_LENGTH} characters.`
);
}
if (!isStudioPlanTaskStatus(rawTask.status)) {
return invalid(
`\`status\` must be one of ${PLAN_TASK_STATUSES.join(
@@ -211,7 +299,7 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
Array.isArray(rawDependsOn) &&
rawDependsOn.every((id: any) => typeof id === 'string')
) {
dependsOn = rawDependsOn;
dependsOn = rawDependsOn.map((dependencyId: any) => dependencyId.trim());
} else {
return invalid(
`\`dependsOn\` must be an array of strings, got ${describeValue(
@@ -221,12 +309,16 @@ export const validatePlanTasks = (rawTasks: any): StudioPlanTaskValidation => {
}
const task: StudioPlanTask = {
id: rawTask.id,
id,
title: rawTask.title,
description: rawTask.description,
status: (rawTask.status: StudioPlanTaskStatus),
dependsOn,
};
// `dependsOn` is set only when the caller sent it (W11): a re-plan that
// omits it must not clear dependencies another writer set.
if (rawDependsOn != null) {
task.dependsOn = dependsOn;
}
if (typeof rawTask.agentCallId === 'string') {
task.agentCallId = rawTask.agentCallId;
}
@@ -139,7 +139,7 @@ describe('PlanStore', () => {
});
describe('validatePlanTasks', () => {
it('accepts a well-formed plan and defaults dependsOn to an empty array', () => {
it('accepts a well-formed plan and omits dependsOn when not provided', () => {
const result = validatePlanTasks([
{
id: 'a',
@@ -150,7 +150,9 @@ describe('PlanStore', () => {
]);
expect(result.success).toBe(true);
if (result.success) {
expect(result.tasks[0].dependsOn).toEqual([]);
// W11: an omitted `dependsOn` is left off the task so a re-plan cannot
// clear a dependency another writer set.
expect(result.tasks[0].dependsOn).toBeUndefined();
expect(result.tasks[0].status).toBe('pending');
}
});
@@ -298,4 +300,98 @@ describe('PlanStore', () => {
expect(reread).toEqual(patched);
});
});
describe('composition and bounds (W11-W13, I11)', () => {
it('lets an omitted dependsOn survive a re-plan', () => {
const existing: any = [
makeTask({ id: 'a', dependsOn: ['b'], agentCallId: 'call-1' }),
makeTask({ id: 'b', status: 'done' }),
];
const validated = validatePlanTasks([
{ id: 'a', title: 'A2', description: 'D', status: 'pending' },
]);
expect(validated.success).toBe(true);
const merged: any = mergePlanTasks(
existing,
validated.success ? validated.tasks : []
);
// W11 regression: the omitted dependsOn (and agentCallId) survive.
expect(merged[0].dependsOn).toEqual(['b']);
expect(merged[0].agentCallId).toBe('call-1');
expect(merged[0].title).toBe('A2');
});
it('rejects duplicate ids (W13)', () => {
const validated = validatePlanTasks([
{ id: 'a', title: 'A', description: 'D', status: 'pending' },
{ id: 'a', title: 'A2', description: 'D', status: 'pending' },
]);
expect(validated.success).toBe(false);
});
it('rejects an oversized plan and oversized strings (W12)', () => {
const many = Array.from({ length: 33 }, (_, i) => ({
id: `t${i}`,
title: 'T',
description: 'D',
status: 'pending',
}));
expect(validatePlanTasks(many).success).toBe(false);
expect(
validatePlanTasks([
{
id: 'x'.repeat(65),
title: 'T',
description: 'D',
status: 'pending',
},
]).success
).toBe(false);
expect(
validatePlanTasks([
{
id: 'a',
title: 'y'.repeat(201),
description: 'D',
status: 'pending',
},
]).success
).toBe(false);
expect(
validatePlanTasks([
{
id: 'a',
title: 'T',
description: 'z'.repeat(1001),
status: 'pending',
},
]).success
).toBe(false);
});
it('rejects unexpected fields and a null agentCallId (I11)', () => {
expect(
validatePlanTasks([
{
id: 'a',
title: 'T',
description: 'D',
status: 'pending',
extra: 1,
},
]).success
).toBe(false);
expect(
validatePlanTasks([
{
id: 'a',
title: 'T',
description: 'D',
status: 'pending',
agentCallId: null,
},
]).success
).toBe(false);
});
});
});
@@ -16,6 +16,9 @@
* - `createRequestWriteQueue` serializes the writes per request, keeping at most
* one pending write (latest wins) and retrying a blocked one rather than
* dropping it.
*
* A `write` must never `await` an `enqueue` for the same `aiRequestId`: the drain
* loop cannot interleave and that promise would never settle.
*/
export type RequestWriteBlock =
@@ -104,8 +107,16 @@ export const createRequestWriteQueue = (): RequestWriteQueue => {
while (entry.pending) {
const pending = entry.pending;
if (pending.isBlocked()) {
// Do not drop it: the next enqueue retries it. Nothing more to do
// now, because the queue only advances when the caller enqueues.
// Do not drop it, and do not wait for another enqueue (none may ever
// come): schedule a self-recheck. The promise stays pending until the
// write actually runs.
setTimeout(() => {
if (!entry.isRunning && entry.pending) {
runEntry(aiRequestId).catch(error => {
console.error('[studio] request write queue failure:', error);
});
}
}, 50);
break;
}
entry.pending = null;
@@ -132,12 +143,19 @@ export const createRequestWriteQueue = (): RequestWriteQueue => {
): Promise<void> => {
const entry = getEntry(aiRequestId);
// Latest wins: replace whatever was waiting, and settle it as superseded
// (resolved, not rejected - the caller asked for a write that a newer one
// made pointless).
// (rejected, not resolved - a replaced write never ran and must not report
// success).
if (entry.pending) {
const superseded = entry.pending;
entry.pending = null;
superseded.resolve();
superseded.reject(
Object.assign(
new Error('Superseded by a newer write for this request.'),
{
code: 'superseded',
}
)
);
}
return new Promise<void>((resolve, reject) => {
entry.pending = { write, isBlocked, resolve, reject };
@@ -190,7 +190,15 @@ describe('RequestWriteGate', () => {
() => false
);
await Promise.all([first, second, third]);
// The superseded write never ran and must reject, not report success.
let supersededError = null;
try {
await second;
} catch (error) {
supersededError = error;
}
await Promise.all([first, third]);
expect(supersededError && supersededError.code).toBe('superseded');
expect(events).toEqual(['first', 'third']);
});
@@ -199,10 +207,10 @@ describe('RequestWriteGate', () => {
let blocked = true;
const events = [];
const write = queue.enqueue(
const first = queue.enqueue(
'req-1',
async () => {
events.push('ran');
events.push('first-ran');
},
() => blocked
);
@@ -211,17 +219,22 @@ describe('RequestWriteGate', () => {
await new Promise(resolve => setTimeout(resolve, 10));
expect(events).toEqual([]);
// Unblock and enqueue again: the retried write runs.
// Unblock: the queue retries the blocked write itself (no new enqueue).
blocked = false;
await new Promise(resolve => setTimeout(resolve, 80));
await first;
expect(events).toEqual(['first-ran']);
// A later write runs after it: both ran, so the first was retried, not
// dropped.
await queue.enqueue(
'req-1',
async () => {
events.push('ran');
events.push('second-ran');
},
() => blocked
() => false
);
await write;
expect(events).toEqual(['ran']);
expect(events).toEqual(['first-ran', 'second-ran']);
});
it('rejects the caller when its write throws', async () => {
+9 -5
View File
@@ -25,7 +25,8 @@ export type StudioRole = {|
/**
* Exact tool names exposed to this role. Any name not listed here is never
* sent to the model (see `getToolsForRole`), so a role cannot be talked into
* using a tool it was not granted.
* calling a tool it was not granted. (`run_script` excepted: it reaches any
* registry function not in `NonScriptableFunctionNames`.)
*/
allowedToolNames: Array<string>,
/**
@@ -81,6 +82,9 @@ export const MUTATING_TOOL_NAMES: Array<string> = [
'change_custom_behavior',
'create_custom_function',
'change_custom_function',
'run_gameplay_test',
'change_gameplay_tests',
'initialize_project',
];
export const STUDIO_ROLES: { [StudioRoleId]: StudioRole } = {
@@ -202,17 +206,17 @@ export const SPAWNABLE_ROLE_IDS: Array<StudioRoleId> = [
];
export const isStudioRoleId = (value: any): boolean =>
typeof value === 'string' && !!STUDIO_ROLES[(value: any)];
typeof value === 'string' &&
Object.prototype.hasOwnProperty.call(STUDIO_ROLES, (value: any));
export const isSpawnableRoleId = (value: any): boolean =>
isStudioRoleId(value) && SPAWNABLE_ROLE_IDS.includes((value: StudioRoleId));
export const getStudioRole = (roleId: StudioRoleId): StudioRole => {
const role = STUDIO_ROLES[roleId];
if (!role) {
if (!Object.prototype.hasOwnProperty.call(STUDIO_ROLES, (roleId: any))) {
throw new Error(`Unknown studio role: ${String(roleId)}.`);
}
return role;
return STUDIO_ROLES[roleId];
};
/**
@@ -12,9 +12,7 @@ describe('Studio roles', () => {
const allToolNames: Array<string> = GDEVELOP_OPENAI_TOOLS.map(
tool => tool.function.name
);
// `spawn_agent` is part of the studio's own tool schema addition (Phase 4),
// not of the upstream-derived part of `GDEVELOP_OPENAI_TOOLS`.
const availableToolNames = new Set([...allToolNames, 'spawn_agent']);
const availableToolNames = new Set(allToolNames);
const roleIds = Object.keys(STUDIO_ROLES);
@@ -106,11 +104,15 @@ describe('Studio roles', () => {
expect(
designerReadOnlyTools.filter(name => !READ_ONLY_TOOL_NAMES.includes(name))
).toEqual([]);
// And the tester is granted no mutating tool at all.
// And the tester writes no game content: its only mutating tools (per W1)
// are the two gameplay-test tools, which change test definitions, never the
// game itself.
const testerMutations = STUDIO_ROLES.tester.allowedToolNames.filter(name =>
MUTATING_TOOL_NAMES.includes(name)
);
expect(testerMutations).toEqual([]);
expect(testerMutations.sort()).toEqual(
['change_gameplay_tests', 'run_gameplay_test'].sort()
);
});
it('only the manager may delegate, so a sub-agent never spawns a sub-agent', () => {
@@ -2,11 +2,14 @@
import { makeSimplifiedProjectBuilder } from '../../EditorFunctions/SimplifiedProject/SimplifiedProject';
import { customCreateSubAgentAiRequest } from '../../AI/CustomAIClient';
import { getStudioRole, isSpawnableRoleId, type StudioRoleId } from './Roles';
import { isSpawnableRoleId, type StudioRoleId } from './Roles';
import { type AiRequestMessageAssistantFunctionCall } from '../../Utils/GDevelopServices/Generation';
const gd: libGDevelop = global.gd;
/** Cap on live sub-agents one parent may have (W8). */
export const MAX_SUB_AGENTS_PER_PARENT = 8;
/** Whether a function call is the studio's delegation tool. */
export const isSpawnAgentCall = (
functionCall: AiRequestMessageAssistantFunctionCall
@@ -61,12 +64,23 @@ export const parseSpawnAgentArgs = (
const task = rawTask.trim();
if (!task) return null;
// Reject (do not coerce) a present-but-wrongly-typed optional field.
if (rawContext !== undefined && typeof rawContext !== 'string') return null;
if (rawRelatedTaskId !== undefined && typeof rawRelatedTaskId !== 'string')
return null;
const context = typeof rawContext === 'string' ? rawContext.trim() : '';
const relatedTaskId =
typeof rawRelatedTaskId === 'string' && rawRelatedTaskId.trim()
? rawRelatedTaskId.trim()
: null;
// Reject oversized strings (W6): a runaway argument must not become a child
// request the user cannot read.
if (shortTitle.length > 100 || task.length > 4000 || context.length > 4000) {
return null;
}
return { role, shortTitle, task, context, relatedTaskId };
};
@@ -118,9 +132,10 @@ export const buildGddContextNote = (project: ?gdProject): string => {
: JSON.stringify(variable.variableChildren || variable.type)
}`
);
return `The studio's design document, as GDD_ project variables:\n${lines.join(
const note = `The studio's design document, as GDD_ project variables:\n${lines.join(
'\n'
)}`;
return note.length > 4000 ? note.slice(0, 4000) + '\n…(truncated)' : note;
};
export type SpawnSubAgentResult =
@@ -140,6 +155,7 @@ export const spawnSubAgent = async ({
gameProjectJson,
projectSpecificExtensionsSummaryJson,
project,
gddContextNote,
onStamped,
}: {|
parentAiRequestId: string,
@@ -147,6 +163,9 @@ export const spawnSubAgent = async ({
gameProjectJson: string | null,
projectSpecificExtensionsSummaryJson: string | null,
project: ?gdProject,
// The GDD note, computed once per batch and passed in (W7). When provided it
// is used verbatim instead of rebuilding the simplified project per call.
gddContextNote?: string | null,
onStamped: (subAgentAiRequestId: string) => void,
|}): Promise<SpawnSubAgentResult> => {
const parsedArgs = parseSpawnAgentArgs(functionCall);
@@ -154,13 +173,6 @@ export const spawnSubAgent = async ({
return { error: 'Invalid spawn_agent arguments.' };
}
const role = getStudioRole(parsedArgs.role);
if (!role) {
// `isSpawnableRoleId` already gated this, but keep the failure a normal
// result rather than a throw, so a model mistake is a failed tool call.
return { error: `Unknown role: ${String(parsedArgs.role)}.` };
}
let userRequest = `Task: ${parsedArgs.task}`;
if (parsedArgs.context) {
userRequest += `\n\nContext: ${parsedArgs.context}`;
@@ -170,7 +182,9 @@ export const spawnSubAgent = async ({
// it, so handing it back to the designer would be noise.
const spawnContextNote =
parsedArgs.role === 'developer' || parsedArgs.role === 'tester'
? buildGddContextNote(project)
? gddContextNote != null
? gddContextNote
: buildGddContextNote(project)
: null;
const subAgentRequest = await customCreateSubAgentAiRequest({
@@ -11,6 +11,7 @@ import {
isSubAgentFinished,
buildSubAgentReport,
getSubAgentReportLabel,
getSubAgentRoleId,
} from './FinalizeSubAgents';
import {
buildPlanOutput,
@@ -132,7 +133,7 @@ export const useStudioRuntime = ({
aiRequestId: string,
editorFunctionCallResults: Array<EditorFunctionCallResult>,
options: Object
) => Promise<void>,
) => Promise<boolean>,
/**
* Runs a write once the request is free (see `RequestWriteGate`). Optional so
* the hook can be used without the gate; the write then runs immediately.
@@ -172,6 +173,10 @@ export const useStudioRuntime = ({
const subAgentRequest = aiRequests[subAgentId];
if (!subAgentRequest) continue;
// A hosted sub-agent is retired by the server, never finalized here.
if (!getSubAgentRoleId({ aiRequest: subAgentRequest, aiRequests })) {
continue;
}
const parentRequest = aiRequests[subAgentInfo.parentAiRequestId];
if (!parentRequest) continue;
@@ -229,33 +234,43 @@ export const useStudioRuntime = ({
const parentRequest = aiRequests[parentAiRequestId];
if (parentRequest) {
const updatedOutput = buildPlanStatusUpdateOutput(
parentRequest,
callId
);
if (updatedOutput) {
const writePlan = async () => {
updateAiRequest(parentAiRequestId, currentRequest =>
currentRequest
? { ...currentRequest, output: updatedOutput }
: // The request disappeared: hand back the same empty shell
// the mutator expects rather than null.
({
id: parentAiRequestId,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
userId: 'local-byok-user',
status: 'ready',
error: null,
output: updatedOutput,
}: any)
);
};
if (enqueueRequestWrite) {
await enqueueRequestWrite(parentAiRequestId, writePlan);
} else {
await writePlan();
}
const writePlan = async () => {
updateAiRequest(parentAiRequestId, currentRequest => {
const base =
currentRequest ||
({
id: parentAiRequestId,
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
userId: 'local-byok-user',
status: 'ready',
error: null,
output: [],
}: any);
// Recompute inside the updater: two finalizations in one pass
// would otherwise write the first snapshot twice.
const updated = buildPlanStatusUpdateOutput(base, callId);
return updated ? { ...base, output: updated } : base;
});
};
if (enqueueRequestWrite) {
// Fire-and-forget: a blocked gate write no longer settles
// inline, so awaiting it would freeze every later finalization
// behind `isRunningRef`.
enqueueRequestWrite(parentAiRequestId, writePlan).catch(
error => {
if (error.code === 'superseded') {
console.info('[studio] plan write superseded');
} else {
console.error(
'[studio] error while writing the plan update:',
error
);
}
}
);
} else {
await writePlan();
}
}
@@ -263,7 +278,7 @@ export const useStudioRuntime = ({
aiRequest: subAgentRequest,
aiRequests,
});
await onSendEditorFunctionCallResults(
const sent = await onSendEditorFunctionCallResults(
parentAiRequestId,
[
{
@@ -272,13 +287,17 @@ export const useStudioRuntime = ({
success: true,
output: {
message: `Report from the ${label}:\n\n${buildSubAgentReport(
subAgentRequest
subAgentRequest,
aiRequests
)}`,
},
},
],
{}
);
// Report delivery is not droppable: throw so the `catch` retries
// (it deletes `startedCallIds`, so the next pass redoes this).
if (!sent) throw new Error('Sub-agent report was not sent.');
}
} catch (error) {
console.error('[studio] error while finalizing a sub-agent:', error);
+211 -73
View File
@@ -62,9 +62,16 @@ import { makeSimplifiedProjectBuilder } from '../EditorFunctions/SimplifiedProje
import { prepareAiUserContent } from './PrepareAiUserContent';
import { extractGDevelopApiErrorStatusAndCode } from '../Utils/GDevelopServices/Errors';
import { isCustomEndpointEnabled } from '../AI/CustomAIClient';
import { isSpawnAgentCall, spawnSubAgent } from './Studio/SpawnSubAgents';
import {
isSpawnAgentCall,
spawnSubAgent,
buildGddContextNote,
MAX_SUB_AGENTS_PER_PARENT,
} from './Studio/SpawnSubAgents';
import { createLoopGuard } from './Studio/LoopGuard';
import { mergePlanTasks } from './Studio/PlanStore';
import { mergePlanResultOutput } from './Studio/PlanStore';
import { isSubAgentAtTurnCap } from './Studio/FinalizeSubAgents';
import { getStudioRole } from './Studio/Roles';
import {
createRequestWriteQueue,
getRequestWriteBlock,
@@ -253,6 +260,7 @@ export const useProcessFunctionCalls = ({
editorCallbacks,
aiRequestsToProcess,
onSendEditorFunctionCallResults,
isStudioEnabled,
getEditorFunctionCallResults,
addEditorFunctionCallResults,
onSceneEventsModifiedOutsideEditor,
@@ -289,7 +297,7 @@ export const useProcessFunctionCalls = ({
createdExternalLayoutNames?: Array<string>,
createdProject?: ?gdProject,
|}
) => Promise<void>,
) => Promise<boolean>,
getEditorFunctionCallResults: string => Array<EditorFunctionCallResult> | null,
addEditorFunctionCallResults: (
string,
@@ -327,6 +335,9 @@ export const useProcessFunctionCalls = ({
onWillInstallExtension: (extensionNames: Array<string>) => void,
onExtensionInstalled: (extensionNames: Array<string>) => void,
isReadyToProcessFunctionCalls: boolean,
// Whether this view runs the multi-agent studio (the editor does; the
// stand-alone form does not, so it cannot spawn sub-agents).
isStudioEnabled?: boolean,
getIsAutoEditEnabled: () => boolean,
suspendAiRequest: (aiRequestId: string) => Promise<void>,
requestEditApproval: (request: EditApprovalRequest) => Promise<boolean>,
@@ -436,6 +447,9 @@ export const useProcessFunctionCalls = ({
// consecutive identical calls, consecutive failures and turns, so a local
// agent that loops cannot run forever.
const loopGuardsRef = React.useRef<Map<string, any>>(new Map());
const loopGuardLastUserMessageIdRef = React.useRef<Map<string, string>>(
new Map()
);
// Forget all previously-granted edit approvals so the next modifying call
// asks again. Called when the user toggles auto-edit: turning it on then off
@@ -453,6 +467,16 @@ export const useProcessFunctionCalls = ({
if (!isReadyToProcessFunctionCalls) return;
if (aiRequest.status === 'suspended') return;
// A studio sub-agent that has used up its role's turn budget takes no
// further turn (the studio finalizes it with its last message). Return
// before taking any in-flight lock.
if (
isCustomEndpointEnabled() &&
isSubAgentAtTurnCap({ aiRequest, aiRequests: aiRequestsRef.current })
) {
return;
}
const functionCallsToProcess = functionCalls.filter(
functionCall =>
!inFlightFunctionCallIdsRef.current.has(
@@ -473,14 +497,44 @@ export const useProcessFunctionCalls = ({
);
});
// The studio role a sub-agent was spawned with restricts its tool subset
// (defense in depth: the model is offered only these, and anything else it
// still calls is refused here). All denied calls stay in
// `functionCallsToProcess` so their in-flight locks are released below.
const studioRole = aiRequest.studioRoleId
? getStudioRole((aiRequest.studioRoleId: any))
: null;
const studioDeniedCallIds = new Set<string>();
if (studioRole) {
for (const functionCall of functionCallsToProcess) {
if (!studioRole.allowedToolNames.includes(functionCall.name)) {
studioDeniedCallIds.add(functionCall.call_id);
addEditorFunctionCallResults(aiRequest.id, [
{
status: 'finished',
call_id: functionCall.call_id,
success: false,
output: {
message: `The tool ${
functionCall.name
} is not available for the ${studioRole.displayName} role.`,
},
},
]);
}
}
}
// An explorer sub-agent's script is read-only (see below: it is exposed
// only non-mutating functions). Knowing this lets us both skip its edit
// approval and restrict the functions its `run_script` can call.
// approval and restrict the functions its `run_script` can call. A
// read-only studio role is read-only too.
const subAgentKind = getSubAgentKind({
aiRequest,
aiRequests: aiRequestsRef.current,
});
const isReadOnlyScriptContext = subAgentKind === 'explorer';
const isReadOnlyScriptContext =
subAgentKind === 'explorer' || (!!studioRole && !!studioRole.readOnly);
// Gate project-modifying calls behind a user confirmation when auto-edit
// is off. Read-only calls (exploration, inspection) always run. The first
@@ -510,6 +564,9 @@ export const useProcessFunctionCalls = ({
// non-mutating functions), so it never needs an edit approval even
// though `run_script` is declared as project-modifying.
!(isReadOnlyScriptContext && functionCall.name === 'run_script') &&
// A read-only studio role never raises the prompt (its subset was
// fixed at spawn and is enforced by the dispatch filter above).
!(studioRole && studioRole.readOnly) &&
!isCallApproved(functionCall)
);
@@ -567,21 +624,39 @@ export const useProcessFunctionCalls = ({
}
}
addEditorFunctionCallResults(
aiRequest.id,
functionCallsToProcess.map(functionCall => ({
status: 'working',
call_id: functionCall.call_id,
}))
);
// The local loop guard: with no server under BYOK, nothing else stops an
// agent looping on fresh call ids. Only recorded for local requests - the
// hosted backend enforces its own `repeated-tool-call-loop` error, and a
// second client-side trip over the same condition would double-report.
if (isCustomEndpointEnabled()) {
const guard = createLoopGuard();
// One guard per request (persisted across passes) so its counters
// accumulate; reset on a new user message.
const loopGuardHandledCallIds = new Set<string>();
const guard = isCustomEndpointEnabled()
? loopGuardsRef.current.get(aiRequest.id) || createLoopGuard()
: null;
if (guard) {
loopGuardsRef.current.set(aiRequest.id, guard);
const lastUserMessage = (aiRequest.output || [])
.filter(
message => message.type === 'message' && message.role === 'user'
)
.pop();
const currentLastUserMessageId = lastUserMessage
? (lastUserMessage: any).messageId
: null;
const seenLastUserMessageId = loopGuardLastUserMessageIdRef.current.get(
aiRequest.id
);
if (
currentLastUserMessageId &&
currentLastUserMessageId !== seenLastUserMessageId
) {
guard.reset();
loopGuardLastUserMessageIdRef.current.set(
aiRequest.id,
currentLastUserMessageId
);
}
functionCallsToProcess.forEach(functionCall => {
guard.recordToolCall({
name: functionCall.name,
@@ -595,17 +670,19 @@ export const useProcessFunctionCalls = ({
const loopGuardMessage = `The local agent looks stuck (${
decision.reason
}). Stop repeating this action and change approach, or report the problem.`;
// Pair the warning with the batch's first real call: an unpaired
// `function_call_output` (a call the model never made) is rejected by
// OpenAI-compatible APIs.
const guardedCall = functionCallsToProcess[0];
addEditorFunctionCallResults(aiRequest.id, [
{
status: 'finished',
// A synthetic id: the model sees the warning in its transcript
// as the output of a call it did not make, which is enough for
// it to change course.
call_id: `loop-guard-${aiRequest.id}-${Date.now()}`,
call_id: guardedCall.call_id,
success: false,
output: { message: loopGuardMessage },
},
]);
loopGuardHandledCallIds.add(guardedCall.call_id);
}
if (decision.action === 'constrain') {
// Stop issuing further turns for this request. The request is marked
@@ -651,11 +728,54 @@ export const useProcessFunctionCalls = ({
// hosted backend never emits this tool name, so hosted behaviour is
// untouched and this pre-pass is skipped entirely.
const spawnCalls = isCustomEndpointEnabled()
? functionCallsToProcess.filter(isSpawnAgentCall)
? functionCallsToProcess.filter(
functionCall =>
isSpawnAgentCall(functionCall) &&
!studioDeniedCallIds.has(functionCall.call_id)
)
: [];
const spawnHandledCallIds = new Set<string>();
const spawnFailureResults: Array<EditorFunctionCallResult> = [];
if (spawnCalls.length > 0) {
// Computed once per batch (W7), not once per spawn.
const gddContextNote = buildGddContextNote(project);
// Fan-out cap (W8): bound the live children one parent may have.
const existingChildCount = Object.keys(aiRequestsRef.current).filter(
id =>
(aiRequestsRef.current[id] || {}).parentAiRequestId === aiRequest.id
).length;
let spawnedThisBatch = 0;
for (const spawnCall of spawnCalls) {
// The stand-alone view has no studio: sub-agents cannot be spawned.
if (!isStudioEnabled) {
spawnHandledCallIds.add(spawnCall.call_id);
const denial = {
status: 'finished',
call_id: spawnCall.call_id,
success: false,
output: { message: 'Sub-agents are not available in this view.' },
};
spawnFailureResults.push(denial);
addEditorFunctionCallResults(aiRequest.id, [denial]);
continue;
}
// Beyond the fan-out cap, refuse instead of spawning.
if (
existingChildCount + spawnedThisBatch >=
MAX_SUB_AGENTS_PER_PARENT
) {
spawnHandledCallIds.add(spawnCall.call_id);
const capDenial = {
status: 'finished',
call_id: spawnCall.call_id,
success: false,
output: { message: 'Sub-agent limit reached for this request.' },
};
spawnFailureResults.push(capDenial);
addEditorFunctionCallResults(aiRequest.id, [capDenial]);
continue;
}
spawnedThisBatch++;
let spawnResult;
try {
spawnResult = await spawnSubAgent({
@@ -664,6 +784,7 @@ export const useProcessFunctionCalls = ({
gameProjectJson: aiRequest.gameProjectJson || null,
projectSpecificExtensionsSummaryJson: null,
project,
gddContextNote,
onStamped: subAgentAiRequestId => {
// Stamp the call in place so the shared loop skips it from here
// on, then activate the child so AiRequestContext polls it and
@@ -684,26 +805,44 @@ export const useProcessFunctionCalls = ({
spawnHandledCallIds.add(spawnCall.call_id);
if (spawnResult.error) {
// Surface it as a failed call the model sees, rather than a crash.
addEditorFunctionCallResults(aiRequest.id, [
{
status: 'finished',
call_id: spawnCall.call_id,
success: false,
output: { message: spawnResult.error },
},
]);
const failure = {
status: 'finished',
call_id: spawnCall.call_id,
success: false,
output: { message: spawnResult.error },
};
spawnFailureResults.push(failure);
addEditorFunctionCallResults(aiRequest.id, [failure]);
}
}
}
const callsForEditor = spawnHandledCallIds.size
? functionCallsToProcess.filter(
functionCall => !spawnHandledCallIds.has(functionCall.call_id)
)
: functionCallsToProcess;
const callsForEditor = functionCallsToProcess.filter(
functionCall =>
!spawnHandledCallIds.has(functionCall.call_id) &&
!loopGuardHandledCallIds.has(functionCall.call_id) &&
!studioDeniedCallIds.has(functionCall.call_id)
);
// Mark only the editor calls as "working": a spawn (or a loop-guard hit)
// already has a terminal result, and a permanent "working" entry would
// make `hasUnfinishedResult` silently drop sibling sends.
addEditorFunctionCallResults(
aiRequest.id,
callsForEditor.map(functionCall => ({
status: 'working',
call_id: functionCall.call_id,
}))
);
if (callsForEditor.length === 0) {
// Nothing left for the editor: the spawn results were already recorded,
// and the parent's next turn is driven by the sub-agent reporting back.
callsForEditor.forEach(functionCall => {
// Nothing left for the editor: surface any spawn failures to the model
// (they are already recorded but not yet sent), then release the locks.
if (spawnFailureResults.length > 0) {
await onSendEditorFunctionCallResults(
aiRequest.id,
spawnFailureResults,
{}
);
}
functionCallsToProcess.forEach(functionCall => {
inFlightFunctionCallIdsRef.current.delete(
`${aiRequest.id}:${functionCall.call_id}`
);
@@ -899,47 +1038,43 @@ export const useProcessFunctionCalls = ({
return;
}
// Record outcomes for the loop guard's error-storm trip (a real success
// clears it; a failure advances it).
if (guard) {
results.forEach(result => {
if (result.status !== 'finished') return;
if (result.success) guard.recordSuccess();
else guard.recordError();
});
}
const newResults = addEditorFunctionCallResults(aiRequest.id, results);
// A plan written by the plan tool is folded over the plan the request
// already has, matching tasks by id: the tool sends the tasks it knows
// about, so a field another writer set (the studio's `agentCallId` when
// it delegates a task, Phase 5) must survive. Without this, re-planning
// would silently strip it.
const mergedResults = newResults.map(result => {
if (result.status !== 'finished' || !result.success) return result;
let output;
try {
output = JSON.parse(result.output);
} catch (error) {
return result;
}
if (!output || !output.plan || !Array.isArray(output.plan.tasks)) {
return result;
}
const existingPlan = getLatestActivePlan(aiRequest);
const existingTasks = existingPlan ? existingPlan.tasks : null;
if (!existingTasks) return result;
return {
...result,
output: JSON.stringify({
...output,
plan: {
tasks: mergePlanTasks(
// $FlowFixMe[incompatible-type]
existingTasks,
output.plan.tasks
),
},
}),
};
});
// it delegates a task) must survive. `result.output` is the object
// `EditorFunctionCallRunner` builds, so it is read as an object, never
// parsed.
const existingPlan = getLatestActivePlan(aiRequest);
const existingTasks = existingPlan ? existingPlan.tasks : null;
const mergedResults = newResults.map(result =>
// $FlowFixMe[incompatible-type]
mergePlanResultOutput(result, existingTasks)
);
await onSendEditorFunctionCallResults(aiRequest.id, mergedResults, {
createdSceneNames,
createdExternalLayoutNames,
createdProject,
});
const sent = await onSendEditorFunctionCallResults(
aiRequest.id,
mergedResults,
{
createdSceneNames,
createdExternalLayoutNames,
createdProject,
}
);
if (!sent) {
console.error('[studio] function call results were not sent');
}
} finally {
// Flush the coalesced editor notifications for everything modified in
// this batch (one hot reload instead of one per call). In `finally` so
@@ -959,8 +1094,10 @@ export const useProcessFunctionCalls = ({
}
// Release the lock so these calls can be retried if needed
// (e.g. after an error or a suspension).
callsForEditor.forEach(functionCall => {
// (e.g. after an error or a suspension). Release every call that was
// locked (including spawn/loop-guard handled ones), not just the ones
// sent to the editor.
functionCallsToProcess.forEach(functionCall => {
inFlightFunctionCallIdsRef.current.delete(
`${aiRequest.id}:${functionCall.call_id}`
);
@@ -993,6 +1130,7 @@ export const useProcessFunctionCalls = ({
getAssetStoreTagForNewObject,
generateEvents,
onSendEditorFunctionCallResults,
isStudioEnabled,
getIsAutoEditEnabled,
suspendAiRequest,
requestEditApproval,
@@ -164,6 +164,9 @@ export type AiRequest = {
forkedAfterOriginalMessageId?: string | null,
forkedAfterNewMessageId?: string | null,
parentAiRequestId?: string | null,
// The studio role a sub-agent was spawned with (fork-only; set only on local
// sub-agent requests) so its tool subset and prompt survive every turn.
studioRoleId?: string | null,
error: AiRequestError | null,
contextStats?: AiRequestContextStats | null,