Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions lib/modules-lib/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,7 @@ export interface ModuleSideContent {
* on Source Academy frontend.
*/
body: (context: DebuggerContext) => JSX.Element;

serialize: (context: DebuggerContext) => DebuggerContext;
deserialize: (context: DebuggerContext) => DebuggerContext;
};
8 changes: 8 additions & 0 deletions src/bundles/rune/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,11 @@
hollusion_magnitude,
show
} from './display';


Check warning on line 65 in src/bundles/rune/src/index.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

More than 1 blank line not allowed
export {
serializeRune,
serializeDrawnRune,
deserializeRune,
deserializeDrawnRune
} from './rune'

Check warning on line 71 in src/bundles/rune/src/index.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Missing semicolon

Check warning on line 71 in src/bundles/rune/src/index.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Newline required at end of file but not found
132 changes: 132 additions & 0 deletions src/bundles/rune/src/rune.ts
Original file line number Diff line number Diff line change
Expand Up @@ -419,3 +419,135 @@

public toReplString = () => '<AnimatedRune>';
}

export type SerializedRune = {
vertices: Float32Array;
colors: Float32Array | null;
transformMatrix: mat4;
hollusionDistance: number;
texture?: null;
};

export function serializeRune(rune: Rune): SerializedRune {
return {
vertices: rune.vertices,
colors: rune.colors,
transformMatrix: rune.transformMatrix,
hollusionDistance: rune.hollusionDistance,
texture: null
} as SerializedRune;
}

export function deserializeRune(serializedRune: SerializedRune): Rune {
const vertices = serializedRune.vertices;
const colors = serializedRune.colors;
const transformMatrix = serializedRune.transformMatrix;
const hollusionDistance = serializedRune.hollusionDistance;
const texture = serializedRune.texture || null;

return Rune.of({
vertices,
colors,
transformMatrix,
hollusionDistance,
texture
});
}

export type SerializedDrawnRune =
| {
kind: 'normal';

Check warning on line 459 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
rune: SerializedRune;

Check warning on line 460 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
isHollusion: boolean;

Check warning on line 461 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
}

Check warning on line 462 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 2 spaces but found 4
| {
kind: 'animated';

Check warning on line 464 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
duration: number;

Check warning on line 465 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
fps: number;

Check warning on line 466 in src/bundles/rune/src/rune.ts

View workflow job for this annotation

GitHub Actions / Repo Wide Tasks

Expected indentation of 4 spaces but found 6
frames: SerializedDrawnRune[];
};

/**
* Serialize a DrawnRune (NormalRune) or AnimatedRune to a plain data structure.
* - Normal: serialize the contained Rune
* - Animated: precompute each frame using the animation function and serialize each frame
*/
export function serializeDrawnRune(drawn: DrawnRune | AnimatedRune): SerializedDrawnRune {
// AnimatedRune is a subclass of glAnimation and defined below in this file
if (drawn instanceof AnimatedRune) {
const duration = drawn.duration;
const fps = drawn.fps;
const totalFrames = Math.max(1, Math.round(duration * fps));

// Access the private func stored on AnimatedRune instance. Use any to bypass TS visibility.
const func = (drawn as any).func as (frame: number) => DrawnRune;

const frames: SerializedDrawnRune[] = [];
for (let i = 0; i < totalFrames; i++) {
const frameDrawn = func(i);
frames.push(serializeDrawnRune(frameDrawn));
}

return {
kind: 'animated',
duration,
fps,
frames
};
}

// Normal case: any DrawnRune that is not AnimatedRune - serialize its underlying Rune
const innerRune: Rune = (drawn as any).rune as Rune;
const isHollusion = (drawn as any).isHollusion as boolean;

return {
kind: 'normal',
rune: serializeRune(innerRune),
isHollusion
};
}

/**
* Deserialize a SerializedDrawnRune back into a DrawnRune (NormalRune or AnimatedRune).
*/
export function deserializeDrawnRune(serialized: SerializedDrawnRune): DrawnRune | AnimatedRune {
if (serialized.kind === 'normal') {
const rune = deserializeRune(serialized.rune);

// Create a small subclass instance to preserve isHollusion flag and draw behaviour
Comment on lines +513 to +517
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The deserializeDrawnRune function rehydrates HollusionRune objects as a generic class, causing the instanceof HollusionRune check to fail and breaking the animated hollusion effect.
Severity: HIGH

Suggested Fix

Modify deserializeDrawnRune to correctly reconstruct HollusionRune instances. When serialized.isHollusion is true, the function should return a new HollusionRune object instead of a RehydratedNormalRune. This will ensure the instanceof HollusionRune check passes, allowing the correct animated rendering component to be used.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/bundles/rune/src/rune.ts#L513-L517

Potential issue: The `deserializeDrawnRune` function incorrectly reconstructs
`HollusionRune` instances as a generic `RehydratedNormalRune` class. This causes the
type check `rune instanceof HollusionRune` to fail in the rendering component. As a
result, the special animated rendering for hollusion runes is skipped, and they are
displayed as static images instead. This functional regression breaks the hollusion
animation feature after a serialization and deserialization cycle, such as when
restoring a tab's state.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This issue is resolved by #640 for runes

class RehydratedNormalRune extends DrawnRune {
constructor(r: Rune, isH: boolean) {
super(r, isH);
}

public draw = (canvas: HTMLCanvasElement) => {
const gl = getWebGlFromCanvas(canvas);
const cameraMatrix = mat4.create();
drawRunesToFrameBuffer(
gl,
this.rune.flatten(),
cameraMatrix,
new Float32Array([1, 1, 1, 1]),
null,
true
);
};
}

return new RehydratedNormalRune(rune, serialized.isHollusion);
}

// animated
const { duration, fps, frames } = serialized;

const func = (frame: number) => {
const frameIndex = frame % frames.length;
const des = deserializeDrawnRune(frames[frameIndex]);
if (des instanceof AnimatedRune) {
throw new Error('Nested animated frames are not supported when deserializing an AnimatedRune');
}
return des as DrawnRune;
};

return new AnimatedRune(duration, fps, func);
}
25 changes: 20 additions & 5 deletions src/tabs/Rune/src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,15 @@ import WebGLCanvas from '@sourceacademy/modules-lib/tabs/WebGLCanvas';
import { defineTab, getModuleState } from '@sourceacademy/modules-lib/tabs/utils';
import { glAnimation, type ModuleTab } from '@sourceacademy/modules-lib/types';
import HollusionCanvas from './hollusion_canvas';
import { serializeDrawnRune, deserializeDrawnRune } from '@sourceacademy/bundle-rune/rune';

export const RuneTab: ModuleTab = ({ context }) => {
const { drawnRunes } = getModuleState<RuneModuleState>(context, 'rune');
const runeCanvases = drawnRunes.map((rune, i) => {
// const { drawnRunes } = getModuleState<RuneModuleState>(context, 'rune');
const deserializedDrawnRunes = context.context.moduleContexts.rune.state.deserializedDrawnRunes
console.log("deserializedDrawnRune inside Tab Render");
console.log(deserializedDrawnRunes);
Comment on lines +13 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Avoid leaving debug console logs in production code.


const runeCanvases = deserializedDrawnRunes.map((rune, i) => {
const elemKey = i.toString();

if (glAnimation.isAnimation(rune)) {
Expand All @@ -33,9 +38,19 @@ export const RuneTab: ModuleTab = ({ context }) => {
};

export default defineTab({
toSpawn(context) {
const drawnRunes = context.context?.moduleContexts?.rune?.state?.drawnRunes;
return drawnRunes.length > 0;
serialize(debuggerContext) {
const drawnRunes = debuggerContext.context?.moduleContexts?.rune?.state?.drawnRunes ?? [];
debuggerContext.context.moduleContexts.rune.state.serializedDrawnRunes = drawnRunes.map((r: any) => serializeDrawnRune(r));
return debuggerContext;
},
deserialize(debuggerContext) {
const serializedDrawnRunes = debuggerContext.context?.moduleContexts?.rune?.state?.serializedDrawnRunes ?? [];
debuggerContext.context.moduleContexts.rune.state.deserializedDrawnRunes = serializedDrawnRunes.map((s: any) => deserializeDrawnRune(s));
return debuggerContext;
},
toSpawn(debuggerContext) {
const serializedDrawnRunes = debuggerContext.context?.moduleContexts?.rune?.state?.serializedDrawnRunes ?? [];
return serializedDrawnRunes.length > 0;
},
body(context) {
return <RuneTab context={context} />;
Expand Down
Loading