kshootmania / ksm-chart-format

Music game chart format (.ksh/.kson) specifications for K-Shoot MANIA
Creative Commons Zero v1.0 Universal
9 stars 0 forks source link

Better chart formats? (feedback is welcome!) #1

Closed m4saka closed 4 years ago

m4saka commented 5 years ago

Edit: Latest kson spec link: https://github.com/m4saka/ksh2kson/blob/master/kson_format.md


I know KSH format is not so great. It's basically designed referring to the jubeat_analyser format and StepMania (.sm) format. (The parameters m=, o=, t= and bar lines (--) are from the jubeat_analyser format.)

KSH format problems:

KSH Chart Format Specification: ksh_format_spec.md

Any feedback would be appreciated!

m4saka commented 5 years ago

ScrollSpeedEvent for adjusting scroll speed, where the note speed is now given by bpm * scrollSpeed.

I think ScrollSpeedEvent can be regarded as an extension of stop events (speed=0). If negative values can be allowed, you can make notes going backwards like B.B.K.K.B.K.K..

m4saka commented 5 years ago

I was thinking too much. A three-time spin should not necessarily be created by using default camera macros. Then two macros "spin1" and "spin2" are not needed, and just using one macro "spin" is enough for an ordinary 360-degree spin.

Setting the default value of "defaultLength" to BeatInfo.resolution * 4 would be neat.

m4saka commented 5 years ago

The actual duration of a lane spin is a bit longer than one bar, so having the "standard" length rather than the "default" length is better, I think.

Let us suppose defining the "spin" macro below. (Variable names and values are random. Just an example.)

{"name":"spin", "standardLength":960, "body":{
    "lane_rotation_z":[
        {"ry":0, "v":0},
        {"ry":960, "v":400},
        {"ry":1440, "v":360, "vf":0}
    ]
}}

And if you invocate the macro as below (or also can be triggered by laser slams),

{"y":0, "l":1920, "name":"spin"}

The invocation length is 1920 and the standard length of "spin" is 960, then the length will be 1920 / 960 = twice as long as the original "ry" position values. Then the actual camera value changes will be:

"lane_rotation_z":[
    {"y":0, "v":0},
    {"y":1920, "v":400},
    {"y":2880, "v":360, "vf":0}
]
m4saka commented 5 years ago

It is not good that you have to set an array to the macro invocation just for supporting lane swings, and as for editing lane swings, having full repeats of a sine wave is a mess (especially when editing).

Adding three parameters is enough for supporting lane swings.

Implicitly-defined "swing" macro (approximation of half-sine; "l" is the standard length):

{"name":"swing", "l":960, "camera":{
    "lane_shift_x":[
        {"ry":0, "v":0, "a":100, "b":59}
        {"ry":480, "v":100, "a":0, "b":59}
        {"ry":960, "v":0}
    ],
    "jdgline_shift_x":[
        {"ry":0, "v":0, "a":100, "b":59}
        {"ry":480, "v":100, "a":0, "b":59}
        {"ry":960, "v":0}
    ]
}}

(idk the exact value of sine approximation, but 59 is the most similar if it is an integer)

Examples:

I think it will be simpler than having an array of camera macro invocations.

m4saka commented 5 years ago

BTW, since "y" is used for time ticks, we should use the right-handed notation for axes.

// camera event list
dictionary CameraEventInfo {
    CameraValuePoint[]? lane_zoom;           // zoom_bottom
    CameraValuePoint[]? lane_shift_x;        // zoom_side
    CameraValuePoint[]? lane_rotation_x;     // zoom_top
    CameraValuePoint[]? lane_rotation_z;     // lane rotation (degree)
    CameraValuePoint[]? jdgline_shift_x;     // judgment line x
    CameraValuePoint[]? jdgline_rotation_z;  // judgment line rotation (degree)
}

↓ should be changed to

// camera event list
dictionary CameraEventInfo {
    CameraValuePoint[]? lane_zoom;           // zoom_bottom
    CameraValuePoint[]? lane_shift_x;        // zoom_side
    CameraValuePoint[]? lane_rotation_x;     // zoom_top
    CameraValuePoint[]? lane_rotation_y;     // lane rotation (degree)
    CameraValuePoint[]? jdgline_shift_x;     // judgment line x
    CameraValuePoint[]? jdgline_rotation_y;  // judgment line rotation (degree)
}

And I think controlling the lane and the judgment line separately is hard work although it's neat as a spec. We should have general parameters which relatively affects both the lane and the judgment line.

// camera event list
dictionary CameraEventInfo {
    CameraValuePoint[]? zoom;           // zoom_bottom
    CameraValuePoint[]? shift_x;        // zoom_side
    CameraValuePoint[]? rotation_x;     // zoom_top
    CameraValuePoint[]? rotation_y;     // rotation degree
// ↓ just for experts ↓ (can be accompanied by ordinary parameters above)
    CameraValuePoint[]? lane_zoom;           // zoom_bottom
    CameraValuePoint[]? lane_shift_x;        // zoom_side (lane only)
    CameraValuePoint[]? lane_rotation_x;     // zoom_top (lane only)
    CameraValuePoint[]? lane_rotation_y;     // rotation degree (lane only)
    CameraValuePoint[]? jdgline_zoom;        // zoom_bottom (judgment line only)
    CameraValuePoint[]? jdgline_shift_x;     // judgment line x (judgment line only)
    CameraValuePoint[]? jdgline_rotation_y;  // rotation degree (judgment line only)
}
m4saka commented 5 years ago

With a default value of "repeat_scale" in a macro definition, you won't need to set that value every time.

Implicitly-defined "swing" macro (approximation of half-sine; "l" is the standard length):

{"name":"swing", "l":960, "repeat_scale":-100, "camera":{
    "shift_x":[
        {"ry":0, "v":0, "a":100, "b":59}
        {"ry":480, "v":100, "a":0, "b":59}
        {"ry":960, "v":0}
    ]
}}

Examples:

Drewol commented 5 years ago

I do like this in some ways but for spins and quarter/half spins (or whatever they're called) there's more to it than just a linear spin. For example on a normal spin there is some special decay and the background spins separately from the lane and on quarter/half spins the background does a full spin while the lane doesn't.

m4saka commented 5 years ago

Thanks for your feedback. I didn't intended to use these decay values even for spins. These parameters are just designed for simulating the current lane swing specs of KSM.

For example on a normal spin there is some special decay and the background spins separately from the lane and on quarter/half spins the background does a full spin while the lane doesn't.

Then we just need to have a CameraValuePoint[] for rotating background layers. If a bg layer definition (not specified in the spec yet) has a flag whether to reflect these rotation values, you can use animation layers like "snow" and "sakura" in KSM, which do not reflect lane spins.

m4saka commented 5 years ago

Also, lane spins should affect the scale of boss layers. Then having two values like "layer_rotation" and "layer_scale" will be neat. The default animation layer will have a flag that describes reflecting only "layer_rotation".

m4saka commented 5 years ago

Updated spec! https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/d892df965637faa47a9cf191fff1d60f2a5f2686

Changes:

m4saka commented 5 years ago

In the current spec, fx/laser audio effect changes are represented regardless of the notes. This covers the current KSH spec (fx effects can be changed while one fx note), but audio effects also can be changed even at a position where notes do not exist. I think the interface of editor can be complex considering this trivial situation. Also, in the KSM community, changing audio effect types while a note is not considered as a good way.

So how about using a specification similar to the SDVX spec for audio effects? In the SDVX spec, only one audio effect is enabled even with two fx notes at the same time. As for two simultaneous laser notes, only one side which has higher value is activated. This will greatly simplify the implementation of the audio effect system and the editor interface, I think.

Of course, since audio effect parameter values can be modified by charts, the new spec can simulate the audio effects in legacy KSH charts (this cannot completely simulate the states depending on the playing situation, but almost the same when playing normally).

Examples: example1 (Note that audio effects are bypassed when no fx notes are pressed. So "Gate:mix=90%" doesn't mean Gate is activated even without playing.)

The ksh-to-kson conversion would be a bit complex, but I believe it is not a big problem (especially if new kson clients want users to convert KSH files before playing).

123jimin commented 5 years ago

While it would take some time, I'm currently making a ksh ↔ kson converter in JavaScript. Maybe I could find some issues on the spec while making it.

It's still an very early stage so I can abandon it and restart in a different language. You can suggest other languages such as C++ or Java with required interface.

For audio effects I have two questions:

m4saka commented 5 years ago

Updated the spec. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/25d501bb82b4addae2eadafe3d593547bf13f09e

Changes:

m4saka commented 5 years ago

@123jimin, I'm sorry I largely revised the spec considering BGA stuffs...

Where would I specify tick time for AudioEffectEvent?

Now it is specified in AudioEffectInfo.pulse_event.y.

For the example provided in your example, since audio effects are specified per lanes, isn't it sufficient to put "Gate;Flangers" at the very beginning?

No, previously audio effects were specified per lanes but currently it's not. Audio effects are now tied to notes for two reasons. The first is to simplify editor implementation. The second is to use the same Def/Invoke scheme for audio effects.

123jimin commented 5 years ago

EDIT: ah, forgot the // All the other metadata fields comment. Ignore this comment for now 😢

Just began writing the converter logic.

I think that following ksh header values are missing in kson meta.

123jimin commented 5 years ago

Notes while making ksh2kson converter (will edit frequently):

This is the current BeatInfo output for my ksh2kson on one of my short chart Yeeeeee.ksh:

beat: {
  bpm: [ { y: 0, v: 186 }, { y: 972, v: 1.35 } ],
  time_sig:
    [ { idx: 0, v: { n: 6, d: 8 } },
      { idx: 6, v: { n: 9, d: 16 } },
      { idx: 7, v: { n: 1, d: 8 } } ],
  scroll_speed: [], // Can't fill this right now
  resolution: 48 // I guess all ksh charts have fixed resolution value
}

This is the current NoteInfo.button output. This chart only have BT chips (and a laser section).

button:
      [ [ { y: 48, v: { l: 0 } },
          { y: 120, v: { l: 0 } },
          { y: 336, v: { l: 0 } },
          { y: 624, v: { l: 0 } },
          { y: 696, v: { l: 0 } } ],
        [ { y: 72, v: { l: 0 } },
          { y: 288, v: { l: 0 } },
          { y: 408, v: { l: 0 } },
          { y: 648, v: { l: 0 } },
          { y: 912, v: { l: 0 } } ],
        [ { y: 144, v: { l: 0 } },
          { y: 216, v: { l: 0 } },
          { y: 864, v: { l: 0 } } ],
        [ { y: 504, v: { l: 0 } },
          { y: 552, v: { l: 0 } },
          { y: 720, v: { l: 0 } },
          { y: 792, v: { l: 0 } } ],
        [],
        []],

After applying removal of default l value:

button:
      [ [ { y: 48 }, { y: 120 }, { y: 336 }, { y: 624 }, { y: 696 } ],
        [ { y: 72 }, { y: 288 }, { y: 408 }, { y: 648 }, { y: 912 } ],
        [ { y: 144 }, { y: 216 }, { y: 864 } ],
        [ { y: 504 }, { y: 552 }, { y: 720 }, { y: 792 } ],
        [],
        []],
m4saka commented 5 years ago

@123jimin Good work!

ScrollSpeedPoint was present in the previous spec but not in current spec.

Sorry, it's a mistake. The type of BeatInfo.scroll_speed should have been ByPulse<GraphSectionPoint[]>.

What's the indices for FX? (I think FX-L is 4 and FX-R is 5)

I intended 4=FX-L, 5=FX-R, but I realized that we can't integrate BT & FX lanes in definition for some reasons. (Reason: In the current KSM spec, FX audio effects are bypassed when no long FX notes are pressed. If we integrate BT and FX lanes, FX audio effects are to be unbypassed also when only long BT notes are pressed...)

NoteInfo.button and ByNotes<T>.button will be replaced with NoteInfo.bt/NoteInfo.fx and ByNotes<T>.bt/ByNotes<T>.fx.

I think ByPulse<Interval> for notes and tiltInfos is a little bit cumbersome. ({y:0, v:{l:0}})

Let us set the default value of Interval.l to 0. Then we don't need to specify v for chip notes anymore. I think that having v is not so a bad idea because having the same structure will simplify the implementation and also good for future expandability.

m4saka commented 5 years ago

I also realized that you need to write the name from Def<T>.name in every invocation because of Invoke<T>.name. It's better to have a dictionary using Def<T>.name as the key and an array of invocations as the value. (You cannot have the array of invocations in Def<T> because we have implicitly-generated definitions like spin.)

m4saka commented 5 years ago

Updated the spec. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/292ad2af6f68cbcdbdc03740b9724d89b3a7d502

m4saka commented 5 years ago

Please make sure to use quotes ("") for every key in JSON. JSLint can be helpful for validating your JSON. http://jslint.com/

m4saka commented 5 years ago

Updated the spec. Almost all the meta data in KSH spec can be represented in this spec. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/1e0178f46a5bc799fb977e5ccb2ece875c36d2fc

("pfilterdelay" in KSH should be supported as a parameter of audio effects, but the type of audio effects is not fully specified in kson yet.)

123jimin commented 5 years ago

Thank you for adding meta values!

(The text I posted was JS literal objects and I'll use JSON.stringify to convert it to proper JSON. Thank you for pointing out.)

P.S. this is the first KSON I have ever converted from KSH (excluding slam rotations). The size is 1/5 of original ksh.

This chart does not have any speedchange (implemented), camera pos change (not implemented), or audio sound effects (not implemented).

https://raw.githubusercontent.com/123jimin/kson-js/master/test/OneJuicyStep.kson

I think that just a single integer (or an array of single integer) for chip notes and [y, l] for long notes are better in terms of chart size (roughly reduces the NoteInfo.bt and NoteInfo.fx by half, at the cost of having irregular structure.

123jimin commented 5 years ago

This is my current implementation of laser spec, w/o bezier curves.

image

m4saka commented 5 years ago

It is not described in the current specification text, but the value (v and vf) of laser is between 0 and 100 (instead of 0-50). The laser resolution will be twice as large as KSH.

I think that just a single integer (or an array of single integer) for chip notes and [y, l] for long notes are better in terms of chart size (roughly reduces the NoteInfo.bt and NoteInfo.fx by half, at the cost of having irregular structure.

I had the same idea, but using different type will cause complex implementation (especially for C++), and this cannot be applied to the standard length of camera/layer patterns. Using ByPulse for objects other than events (which is defined by Def) is not so meaningful, so using "v" does not make much sense. Stopping to use "v" for notes and tilt keep would be a simple solution.

m4saka commented 5 years ago

Small changes to the spec. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/a618832e608786a9c3437efa8d97ce9ab88261da

alexwh commented 5 years ago

Is there going to be any consideration for complex chart visual effects? For example, the backgrounds in some SDVX boss songs, side lane images, or the "screen off" effect in "I".

m4saka commented 5 years ago

Is there going to be any consideration for complex chart visual effects?

Have you checked out the latest kson specification? Boss backgrounds/layers would be done by kson.bg.layer. and you can set kson.bg.layer.def.v.z to a negative value (i.e., in front of the lane) for the "screen off" effect.

alexwh commented 5 years ago

My mistake, I looked through this thread but did not check the full specificiation.

TsFreddie commented 5 years ago

I read through the spec but I can't understand how invocation works. The spec of InvokeList is a little bit confusing for me.

But I'd imagine events are invoked by name?

{ "audio": { "key_sound": { "note_event": { "fx": [
  { "lane": 0, "idx": 20, "name": "clap", "v": { "vol": 20 } }
]}}}

Or like this?

{ "audio": { "key_sound": { "note_event": { "fx": [
  { "lane": 0, "idx": 20, "v": { "name": "clap", "v": { "vol": 20 } } }
]}}}

Or I'm I missing something?

m4saka commented 5 years ago

@TsFreddie, I'm sorry I didn't clarify that in the specification text.

As I stated below, the name will be the key of InvokeList. https://github.com/m4saka/kshootmania-v2/issues/1#issuecomment-468901639 (By this, you won't need to write the name so many times in InvokeList.)

Then your example would be like this.

{ "audio": { "key_sound": { "note_event": { "clap": { "fx": [
  { "lane": 0, "idx": 20, "v": { "vol": 20 } }
]}}}}}
TsFreddie commented 5 years ago

Thanks for the clarification. That make so much sense now.

m4saka commented 5 years ago

But then Def also should have the name as its key...?

dictionary Def<T> {
    DOMString name;          // self-explanatory
    T v;                     // changeable parameters (default values)

    ...                      // unchangeable parameters (e.g. filename, audio effect type)
}

dictionary KeySoundInfo {
    Def<KeySound>[]? def;                           // key sound definitions
    InvokeList<ByPulse<KeySound>[]>? pulse_event;   // key sound invocation by pulse
    InvokeList<ByNotes<KeySound>>? note_event;      // key sound invocation by notes
}

should be

dictionary Def<T> {
    T v;                     // changeable parameters (default values)

    ...                      // unchangeable parameters (e.g. filename, audio effect type)
}

dictionary DefList<T> {
    Def<T>? ...(name is the key);
    ...
}

dictionary KeySoundInfo {
    DefList<KeySound>? def;                         // key sound definitions
    InvokeList<ByPulse<KeySound>[]>? pulse_event;   // key sound invocation by pulse
    InvokeList<ByNotes<KeySound>>? note_event;      // key sound invocation by notes
}
TsFreddie commented 5 years ago

@m4saka That would be preferable for some situation.

For example in C#, by using Json.NET, that could be parsed straight into a dictionary, which could be indexed by key more easily without processing the parsed object first.

I'm not sure about C++ tho

m4saka commented 5 years ago

OK, I revised the spec. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/76eb95256dfbdf2779069d300ca708094a929a5c

Changes:

m4saka commented 5 years ago

Now I'm working on ksh2kson converter (https://github.com/m4saka/ksh2kson) and found several mistakes.

In addition, some parameters are now in the floating-point style (0.0-1.0) instead of the percentage style (0-100). In the bg layer section ("bg"), the relative value modifications between definition and invocation are very confusing because of the value is set in percentage (0-100). Actually I argued the percentage style is better in respect of hashing purposes for internet rankings. However, since some canonicalization processes are needed for hashing in any case, the percentage style has less advantages.

I have revised the specification. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/9e033f2c02ca0d1dd95c9e949993b5f06bf335f3

Changes:

Although this specification has unclear points (for example, the scale of animation layer x/y values, the parameter list of audio effects), I'd like to define this version as "1.0.0" if this version doesn't have a critical issue.

m4saka commented 5 years ago

I realized I missed the curve control point of the laser section (i.e. GraphPoint.a/b and GraphSectionPoint.a/b) in percentage style. But I think these values shouldn't be floating-point because these value can be slightly changed by just opening and saving depending on an environment. The percentage style (0-100) or 0-255 would be better for them.

m4saka commented 5 years ago

But GraphPoint and GraphSectionPoint are also used for parameters other than laser sections, and these are floating-point values. Then we should set GraphPoint.v/vf to double, but then it would be very unnatural that the type of GraphPoint.a/b is long and its range is 0-100.

Maybe it is better to use double for all the parameters used for GraphPoint/GraphSectionPoint including laser sections and change all these to the floating-point style without considering its accuracy problem. (I'm sorry I change my mind a lot...)

m4saka commented 5 years ago

Those revisions are applied to the specification. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/30b72a903478701541943354115555401cf18a6c

Changes:

m4saka commented 4 years ago

The specification of BG stuffs are to be determined after a kson client is released because it is fully depending on implementation and it needs more discussion.

To finalize kson specification, I suggest simply using legacy KSM background for now, instead of using current complicated kson BG specification here.

Here is an example:

{
    ...
    "bg": {
        "legacy":{
            "bg_filename": "desert",
            "layer_filename": "arrow"
        }
    }
    ...
}

Another example with custom images by a charter:

{
    ...
    "bg": {
        "legacy":{
            "bg_filename": "bg.jpg",
            "layer_filename": "layer.gif"
        }
    }
    ...
}

The advantage of adding "legacy" key is you can specify both legacy and (undetermined) new BG option in future if needed.

m4saka commented 4 years ago

In kson specification, it is supposed that only single FX effect is activated even when two long FX notes with a different effect are held at the same time. This nicely simulates the long FX behavior of Sound Voltex, but converting KSH to KSON would be hard work. (Note: Even with this specification, it is possible to convert KSH files without changes. See this post.)

I suggest adding a "continuation" flag to audio effect event (ByNotes<AudioEffect>) like "c" flags in bmson keysound. In this case, if "c" is true, the current KSM behavior is simulated (two FX effects is activated at the same time with two different long FX notes), and if "c" is false, the SDVX behavior is simulated (only one FX effect is activated even with two different long FX notes). The advantage is you can just set "c"=true for every FX note to use the current method if you dislike the new method, and converting KSH to KSON would be much easier.

m4saka commented 4 years ago

As for the "legacy" BG layer of KSH, we also need options to set the play speed and rotation option (whether to be affected by lasers or lane spins).

The following is a quotation from the KSH specification document.

  • "layer" (default:"arrow")
    • The animation layer name (string)
    • There are 7 preset images:
      • "arrow"
      • "smoke"
      • "wave"
      • "techno"
      • "snow"
      • "sakura"
      • "hidden"
    • This option can have a filename (e.g. "layer.png").
    • The speed and the rotation behavior are changed by setting additional values separated by ";" (in v1.67 or later) or "/" (before v1.67).
      • Example: "layer=snow;1100;1"
      • The second value (int) denotes the length of time for one loop in milliseconds.
        • Negative values can be used for playing in reverse.
      • The third value (int, 0-3) denotes:
        • 0: No rotation
        • +1: the layer follows lane tilts
        • +2: the layer follows lane spins

To consider these options, we need a notation like:

{
    ...
    "bg": {
        "legacy": {
            "bg": {
                "filename": "desert"
            }
            "layer": {
                "filename": "snow",
                "duration": "1100",
                "rotation": {
                    "tilt": true,
                    "spin": false
                }
            }
        }
    }
    ...
}
m4saka commented 4 years ago

To cover KSH legacy BG specification, we also need to consider second image files for the BG for 70% or higher gauge.

Then the previous case would be:

{
    ...
    "bg": {
        "legacy": {
            "bg": [
                {
                    "filename": "desert"
                }
            ],
            "layer": [
                {
                    "filename": "snow",
                    "duration": "1100",
                    "rotation": {
                        "tilt": true,
                        "spin": false
                    }
                }
            ]
        }
    }
    ...
}

Another case with two BG images would be:

{
    ...
    "bg": {
        "legacy": {
            "bg": [
                {
                    "filename": "bg.jpg"
                },
                {
                    "filename": "bg_clear.jpg"
                }
            ],
            "layer": [
                {
                    "filename": "snow",
                    "duration": "1100",
                    "rotation": {
                        "tilt": true,
                        "spin": false
                    }
                }
            ]
        }
    }
    ...
}

As for layers, that's done by making two rows in one GIF image, but for consistency I think it's better to use an array also for layers.

Example (just for considering trivial cases):

And I think we should not choose not to implement the "rotation" option for BGs just because that's not possible in KSH.

m4saka commented 4 years ago

I have realized we also consider movies (videos)...

A movie never has two versions for a gauge condition like BG/layer, so using an array is completely meaningless. (Bye bye, consistency...)

{
    ...
    "bg": {
        "legacy": {
            "bg": [
                {
                    "filename": "desert"
                }
            ],
            "layer": [
                {
                    "filename": "snow",
                    "duration": "1100",
                    "rotation": {
                        "tilt": true,
                        "spin": false
                    }
                }
            ]
            "movie": {
                "filename": "movie.avi",
                "offset": 0
            }
        }
    }
    ...
}
m4saka commented 4 years ago

I share a rough draft of the full parameter list of custom audio effects.

Although they are not in the list now, but I think adding BandPassFilter/LowShelfFilter/HighShelfFilter would also be preferred.

Currently, the document of custom effects (user-defined audio effects) of KSH format is still only available in Japanese. I think I need to translate it into English to clarify the value format for each parameter type.

m4saka commented 4 years ago

I have found a specification defect on audio effect parameter change events triggered by pulse (a.k.a. ByPulse<AudioEffect>) used in AudioEffectInfo.pulse_event.

In the current specification, three audio effect instances (BT/FX/laser) are to be made by one audio effect definition (AudioEffectInfo.def), but you cannot specify a certain audio effect instance in invocations by pulse (AudioEffectInfo.pulse_event) because it does not specify a certain note type (BT/FX/laser).

We can avoid this issue by stopping to create three instances per one audio effect definition. I'd like to remove this note from the specification.

// audio effect definition
dictionary Def<AudioEffect> {
    DOMString type;      // name of the audio effect to inherit (e.g. "Flanger")
    AudioEffect? v {
        // Custom fields per FX
    };
    DOMString? filename; // can be specified only if type="SwitchAudio"
}
- // Note: Preset audio effects ("", "Retrigger", "Gate", "Flanger"...) are implicitly defined in default.
- //       If an audio effect with the same name as a preset effect, implicitly defined presets will be overwritten.
- //
- //       BT & FX & laser share the audio effect definition itself,
- // but the instances of audio effects are created for each, and they work in parallel.

The reason why I also eliminated the implicitly-defined audio effects (e.g. "Retrigger", "Flanger", "HighPassFilter"...) is that two different audio effects with the same name cannot exist after this change. Especially, you cannot use two BitCrusher instances (one for FX, and another for laser) at the same time without making another instance in addition to the default implicitly-defined ones.

I would suggest you define all audio effects explicitly, then you don't need to bother sharing one implicitly-defined BitCrusher instance for both FX and laser or making another BitCrusher instance for laser. For example, you can use KSM-style audio effect names, which uses Pascal Case for FX effect names and CAPITAL LETTER for laser effect names, then you can use BitC for FX, and BITC for laser.

An example (I suppose this will be written in every kson file exported from KSM editor):

"def": {
    "Re": {"type": "retrigger"},
    "Ga": {"type": "gate"},
    "Flan": {"type": "flanger"},
    "Pitc": {"type": "pitch_shift"},
    "BitC": {"type": "bitcrusher"}, // for FX
    "Phas": {"type": "phaser"},
    "Wo": {"type": "wobble"},
    "TStp": {"type": "tape_stop"},
    "Echo": {"type": "echo"},
    "SiCh": {"type": "sidechain"},
    "PEAK": {"type": "peaking_filter"},
    "HPF": {"type": "high_pass_filter"},
    "LPF": {"type": "low_pass_filter"},
    "BITC": {"type": "bitcrusher"} // for laser
}

(I know it is not important but I changed them to snake_case for consistency because we do not need to use PascalCase for audio effect types anymore. And I'd prefer bitcrusher & sidechain to bit_crusher & side_chain.)

Of course, even in this case, you can use the audio effects "Re", "Ga", "Flan", ... for lasers, but you need to define another new audio effect only for laser if you use the same audio effect from FX & laser at the same time.

m4saka commented 4 years ago

I have revised the specification. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/465f00113a7fdefb9e20e89f08d44a6d046ffa64

Changes:

m4saka commented 4 years ago

I missed the "c" flag of FX invocation. I've revised the specification again. https://gist.github.com/m4saka/a89594a17dc9422d75e01998bcfd2722/dd595aed137afa465ac404a003eef8131027d8eb

Changes:

m4saka commented 4 years ago

The remaining problem is that it is unclear how to invoke an audio effect type of lasers (e.g. PEAK, HPF, LPF).

While KSH uses time-based invocation, KSON currently uses note-based invocation for lasers. I chose "note-based" for consistency with long FX notes, but the note-based method is a little unnatural because the cutoff frequency of a filter should be determined by both left and right lasers (especially, the maximum value of left and right lasers would be used) although audio effects can be set for one laser note.

However, if you simply use audio.audio_effect.pulse_event in the current kson specification for time-based invocation, you cannot just change a parameter value without invocation. In addition, since audio.audio_effect has both FX & laser audio effects (actually there's no distinction between them in kson spec.), you cannot simply invoke an audio effect only for lasers by using pulse_event.

The third solution would be adding audio parameter assignment feature for lasers using the same idea of tilt_assign. In this way, the third parameter of custom effect (e.g. "10000Hz" in "0Hz>100Hz-10000Hz") would be deleted and the cutoff frequency width would be set by a relative value from the second parameter (e.g. "100Hz" in "0Hz>100Hz-10000Hz"). For example, if you want to use a peaking filter whose cutoff frequency width is 100Hz -- 10000Hz for the laser, you can set 100Hz to audio effect definition of the filter and set +9900Hz to the parameter assignment. The disadvantage is that you need to set this +9900Hz every time you switch to a filter audio effect.

The third solution is natural and consistent with tilt/camera specification, but I think it would be too much...

m4saka commented 4 years ago

If we use relative values, it would be better to use floating-point values for all audio effect parameters. Then we wouldn't have to bother implementing a complex parameter-type system in KSH (for example, "length" type could have both tempo-sync values like "1/16" and millisecond values like "10ms"). If not, you will need to handle irregular cases like the parameter have "1/16" as an absolute value and have "+10ms" as a relative value.

Instead, we can have a sub-parameter like "(parameter name).tempo_sync" which determines whether tempo sync is enabled. Then we wouldn't need the type system.

Drewol commented 4 years ago

For effect definitions, the sample parameter type doesn't specify a sample rate so if it is interpreted as the same number for different sample rates it would result in different sounding effects, so I think it needs to be specified that 1 sample is something like 1/44100 seconds or whatever the base sample rate in ksh is defined as.