align model APIs (#855)

spx-gui/src/models/animation.ts

@@ -22,28 +22,29 @@ type ActionConfig = {
 export const defaultFps = 10
 
 export type AnimationInits = {
-  builder_id?: string
-
+  id?: string
   /** Duration for animation to be played once */
   duration?: number
-  onStart?: ActionConfig
-
-  // not supported by builder:
-  isLoop?: boolean // TODO
-  onPlay?: ActionConfig
+  sound?: string
 }
 
-export type RawAnimationConfig = Omit<AnimationInits, 'duration'> & {
-  anitype?: number
+export type RawAnimationConfig = {
+  builder_id?: string
   frameFrom?: number | string
   frameTo?: number | string
   frameFps?: number
+  onStart?: ActionConfig
 
   // legacy APIs, for compatibility only:
   from?: number | string
   to?: number | string
   fps?: number
+
+  // not supported by builder:
   duration?: number
+  anitype?: number
+  isLoop?: boolean
+  onPlay?: ActionConfig
 }
 
 export class Animation extends Disposable {
@@ -68,21 +69,11 @@ export class Animation extends Disposable {
     return `__animation_${this.name}_`
   }
 
-  withCostumeNamePrefix(name: string) {
-    return this.costumeNamePrefix + name
-  }
-
-  private stripCostumeNamePrefix(name: string) {
-    if (!name.startsWith(this.costumeNamePrefix)) return name
-    return name.slice(this.costumeNamePrefix.length)
-  }
-
   costumes: Costume[]
   // For now, detailed methods to manipulate costumes are not needed, we may implement them later
   setCostumes(costumes: Costume[]) {
     for (const costume of costumes) {
-      let costumeName = this.stripCostumeNamePrefix(costume.name)
-      costumeName = ensureValidCostumeName(costumeName, this)
+      const costumeName = ensureValidCostumeName(costume.name, this)
       costume.setParent(this)
       costume.setName(costumeName)
     }
@@ -107,12 +98,8 @@ export class Animation extends Disposable {
     this.name = name
     this.costumes = []
     this.duration = inits?.duration ?? 0
-    this.sound = inits?.onStart?.play ?? null
-    this.id = inits?.builder_id ?? nanoid()
-
-    for (const field of ['isLoop', 'onPlay'] as const) {
-      if (inits?.[field] != null) console.warn(`unsupported field: ${field} for sprite ${name}`)
-    }
+    this.sound = inits?.sound ?? null
+    this.id = inits?.id ?? nanoid()
 
     return reactive(this) as this
   }
@@ -130,43 +117,56 @@ export class Animation extends Disposable {
   static load(
     name: string,
     {
+      builder_id: id,
       frameFrom,
       frameTo,
       frameFps,
       from,
       to,
       fps,
-      // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      duration: _spxDuration, // drop spx `duration`, which is different from ours
-      ...inits
+      onStart,
+      duration: spxDuration,
+      isLoop,
+      onPlay,
+      anitype
     }: RawAnimationConfig,
-    sprite: Sprite,
+    costumes: Costume[],
     sounds: Sound[]
-  ) {
+  ): [animation: Animation, animationCostumeNames: string[]] {
     frameFrom = frameFrom ?? from
     frameTo = frameTo ?? to
     frameFps = frameFps ?? fps
     if (frameFrom == null || frameTo == null)
       throw new Error(`from and to expected for Animation ${name}`)
-    const fromIndex = getCostumeIndex(sprite.costumes, frameFrom)
-    const toIndex = getCostumeIndex(sprite.costumes, frameTo)
-    const costumes = sprite.costumes.slice(fromIndex, toIndex + 1)
-    const duration = costumes.length / (frameFps ?? defaultFps)
-    const soundId =
-      inits?.onStart?.play == null ? null : sounds.find((s) => s.name === inits?.onStart?.play)?.id
-    if (soundId === undefined) {
-      console.warn(`Sound ${inits?.onStart?.play} not found when creating animation ${name}`)
+    const fromIndex = getCostumeIndex(costumes, frameFrom)
+    const toIndex = getCostumeIndex(costumes, frameTo)
+    const animationCostumes = costumes.slice(fromIndex, toIndex + 1).map((c) => c.clone())
+    const duration = animationCostumes.length / (frameFps ?? defaultFps)
+    // drop spx `duration`, which is different from ours
+    if (spxDuration != null) console.warn(`unsupported field: duration for animation ${name}`)
+    if (isLoop != null) console.warn(`unsupported field: isLoop for animation ${name}`)
+    if (onPlay != null) console.warn(`unsupported field: onPlay for animation ${name}`)
+    if (anitype != null) console.warn(`unsupported field: anitype for animation ${name}`)
+    let soundId: string | undefined = undefined
+    if (onStart?.play != null) {
+      const sound = sounds.find((s) => s.name === onStart.play)
+      if (sound == null)
+        console.warn(`Sound ${onStart.play} not found when creating animation ${name}`)
+      else soundId = sound.id
     }
     const animation = new Animation(name, {
-      ...inits,
+      id,
       duration,
-      onStart: { play: soundId ?? undefined }
+      sound: soundId
     })
-    animation.setCostumes(costumes.map((costume) => costume.clone()))
-    for (const costume of costumes) {
-      sprite.removeCostume(costume.id)
+    const animationCostumeNames = animationCostumes.map((c) => c.name)
+    for (const costume of animationCostumes) {
+      if (costume.name.startsWith(animation.costumeNamePrefix)) {
+        costume.setName(costume.name.slice(animation.costumeNamePrefix.length))
+      }
     }
-    return animation
+    animation.setCostumes(animationCostumes)
+    return [animation, animationCostumeNames]
   }
 
   export(
@@ -180,7 +180,11 @@ export class Animation extends Disposable {
     const costumeConfigs: RawCostumeConfig[] = []
     const files: Files = {}
     for (const costume of this.costumes) {
-      const [costumeConfig, costumeFiles] = costume.export({ basePath, includeId })
+      const [costumeConfig, costumeFiles] = costume.export({
+        basePath,
+        includeId,
+        namePrefix: this.costumeNamePrefix
+      })
       costumeConfigs.push(costumeConfig)
       Object.assign(files, costumeFiles)
     }

spx-gui/src/models/costume.ts

@@ -1,24 +1,25 @@
 import { reactive } from 'vue'
+import { nanoid } from 'nanoid'
 
 import { extname, resolve } from '@/utils/path'
 import { adaptImg } from '@/utils/spx'
 import { Disposable } from '@/utils/disposable'
 import { File, type Files } from './common/file'
 import type { Size } from './common'
-import type { Sprite } from './sprite'
 import { getCostumeName, validateCostumeName } from './common/asset-name'
-import { Animation } from './animation'
-import { nanoid } from 'nanoid'
+import type { Sprite } from './sprite'
+import type { Animation } from './animation'
 
 export type CostumeInits = {
-  builder_id?: string
+  id?: string
   x?: number
   y?: number
   faceRight?: number
   bitmapResolution?: number
 }
 
-export type RawCostumeConfig = CostumeInits & {
+export type RawCostumeConfig = Omit<CostumeInits, 'id'> & {
+  builder_id?: string
   name?: string
   path?: string
 }
@@ -98,7 +99,7 @@ export class Costume {
     this.y = inits?.y ?? 0
     this.faceRight = inits?.faceRight ?? 0
     this.bitmapResolution = inits?.bitmapResolution ?? 1
-    this.id = inits?.builder_id ?? nanoid()
+    this.id = inits?.id ?? nanoid()
     return reactive(this) as this
   }
 
@@ -137,7 +138,7 @@ export class Costume {
   }
 
   static load(
-    { name, path, ...inits }: RawCostumeConfig,
+    { builder_id: id, name, path, ...inits }: RawCostumeConfig,
     files: Files,
     /** Path of directory which contains the sprite's config file */
     basePath: string
@@ -146,19 +147,20 @@ export class Costume {
     if (path == null) throw new Error(`path expected for costume ${name}`)
     const file = files[resolve(basePath, path)]
     if (file == null) throw new Error(`file ${path} for costume ${name} not found`)
-    return new Costume(name, file, inits)
+    return new Costume(name, file, { ...inits, id })
   }
 
   export({
     basePath,
-    includeId = true
+    includeId = true,
+    namePrefix = ''
   }: {
     /** Path of directory which contains the sprite's config file */
     basePath: string
     includeId?: boolean
+    namePrefix?: string
   }): [RawCostumeConfig, Files] {
-    const name =
-      this.parent instanceof Animation ? this.parent.withCostumeNamePrefix(this.name) : this.name
+    const name = namePrefix + this.name
     const filename = name + extname(this.img.name)
     const config: RawCostumeConfig = {
       x: this.x,

spx-gui/src/models/sound.ts

@@ -8,12 +8,13 @@ import type { Project } from './project'
 import { nanoid } from 'nanoid'
 
 export type SoundInits = {
-  builder_id?: string
+  id?: string
   rate?: number
   sampleCount?: number
 }
 
-export type RawSoundConfig = SoundInits & {
+export type RawSoundConfig = Omit<SoundInits, 'id'> & {
+  builder_id?: string
   path?: string
 }
 
@@ -56,7 +57,7 @@ export class Sound extends Disposable {
     this.file = file
     this.rate = inits?.rate ?? 0
     this.sampleCount = inits?.sampleCount ?? 0
-    this.id = inits?.builder_id ?? nanoid()
+    this.id = inits?.id ?? nanoid()
     return reactive(this) as this
   }
 
@@ -73,11 +74,11 @@ export class Sound extends Disposable {
     const pathPrefix = join(soundAssetPath, name)
     const configFile = files[join(pathPrefix, soundConfigFileName)]
     if (configFile == null) return null
-    const { path, ...inits } = (await toConfig(configFile)) as RawSoundConfig
+    const { builder_id: id, path, ...inits } = (await toConfig(configFile)) as RawSoundConfig
     if (path == null) throw new Error(`path expected for sound ${name}`)
     const file = files[resolve(pathPrefix, path)]
     if (file == null) throw new Error(`file ${path} for sound ${name} not found`)
-    return new Sound(name, file, inits)
+    return new Sound(name, file, { ...inits, id })
   }
 
   static async loadAll(files: Files) {

spx-gui/src/models/sprite.test.ts

@@ -4,15 +4,32 @@ import { Animation } from './animation'
 import { Costume } from './costume'
 import { File } from './common/file'
 
+function makeCostume(name: string) {
+  return new Costume(name, new File(`${name}.png`, async () => new ArrayBuffer(0)))
+}
+
 describe('Sprite', () => {
+  it('should export & load animations correctly', async () => {
+    const sprite = new Sprite('Sprite')
+    sprite.addCostume(makeCostume('costume1'))
+    sprite.addCostume(makeCostume('costume2'))
+    const animation1 = new Animation('animation1')
+    animation1.setCostumes([makeCostume('costume3'), makeCostume('costume4')])
+    const animation2 = new Animation('animation2')
+    animation2.setCostumes([makeCostume('costume5'), makeCostume('costume6')])
+    sprite.addAnimation(animation1)
+    sprite.addAnimation(animation2)
+    const exported = sprite.export({ sounds: [] })
+    const [loadedSprite] = await Sprite.loadAll(exported, [])
+    expect(loadedSprite.costumes.map((c) => c.name)).toEqual(['costume1', 'costume2'])
+    expect(loadedSprite.animations.map((c) => c.name)).toEqual(['animation1', 'animation2'])
+    expect(loadedSprite.animations[0].costumes.map((c) => c.name)).toEqual(['costume3', 'costume4'])
+    expect(loadedSprite.animations[1].costumes.map((c) => c.name)).toEqual(['costume5', 'costume6'])
+  })
   it('should handle id-name conversion for animationBindings correctly', async () => {
     const sprite = new Sprite('Sprite')
-    const animation = new Animation('animation', {
-      builder_id: 'animation#'
-    })
-    animation.costumes.push(
-      new Costume('costume', new File('costume', async () => new ArrayBuffer(0)))
-    )
+    const animation = new Animation('animation')
+    animation.setCostumes([makeCostume('costume1')])
     sprite.addAnimation(animation)
     sprite.setAnimationBoundStates(animation.id, [State.turn, State.die])
     expect(sprite.getAnimationBoundStates(animation.id)).toEqual([State.die, State.turn])