Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vector2 compound signal operations #1030

Merged
merged 8 commits into from
May 16, 2024

Conversation

mancopp
Copy link
Contributor

@mancopp mancopp commented Apr 13, 2024

This pull request is related to issue #967

Implementations of mathematical operations in this pull request concern Vector2s based on CompoundSignal exclusively - meaning node fields with decorator @vector2Signal().
Example code to test the functionality:

  const rect = createRef<Rect>();

  view.add(
    <Rect
      ref={rect}
      size={320}
      radius={80}
      x={18}
      y={121}
      smoothCorners
      fill={'#f3303f'}
    />,
  );

  // before
  yield* rect().position(node.position().add([200, 400]), 0.6);
  // after
  yield* rect().position.add([200, 400], 0.6);

  // set
  rect().position.add([200, 400]);
  // tween
  yield* rect().position.add([200, 400], 0.6);
  
  // this:
  rect().position.add([200, 400]);
  // is the same as this:
  rect().position.edit(current => current.add([200, 400]));

  // define a reusable method:
  function snap(grid: PossibleVector2) {
    return (current: Vector2) => current.div(grid).floored.mul(grid);
  }
  
  // use it:
  rect().position.edit(snap(20));

@@ -27,3 +38,42 @@ export function vector2Signal(
wrapper(Vector2)(target, key);
};
}

function compoundVector2Signal(
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of copying this whole function, you could modify the original compound to accept a custom signal class as its argument:

export function compound<
  TSetterValue,
  TValue extends TSetterValue,
  TKeys extends keyof TValue = keyof TValue,
  TOwner = void,
>(
  entries: Record<string, string>,
  klass: typeof CompoundSignalContext<TSetterValue, TValue, TKeys, TOwner> = CompoundSignalContext,
): PropertyDecorator {
  return (target, key) => {
    const meta = getPropertyMetaOrCreate<any>(target, key);
    meta.compound = true;
    meta.compoundEntries = Object.entries(entries);

    addInitializer(target, (instance: any) => {
      if (!meta.parser) {
        useLogger().error(`Missing parser decorator for "${key.toString()}"`);
        return;
      }

      const initial = meta.default;
      const parser = meta.parser.bind(instance);
      const signalContext = new klass(
        meta.compoundEntries.map(([key, property]) => {
          const signal = new SignalContext(
            modify(initial, value => parser(value)[key]),
            <any>map,
            instance,
            undefined,
            makeSignalExtensions(undefined, instance, property),
          ).toSignal();
          return [key as TKeys, signal];
        }),
        parser,
        initial,
        meta.interpolationFunction ?? deepLerp,
        instance,
        makeSignalExtensions(meta, instance, <string>key),
      );

      instance[key] = signalContext.toSignal();
    });
  };
}

vector2Signal could then pass Vector2Context as the second argument to compound

Comment on lines 11 to 65
edit(
callback: (current: Vector2) => SignalValue<PossibleVector2>,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
scale(
value: number,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
mul(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
div(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
add(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
sub(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
dot(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
cross(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
mod(
possibleVector: PossibleVector2,
duration?: number,
timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

You cannot declare these methods using single signatures as this doesn't contain all the necessary information. Depending on the passed parameters, a different type will be returned. This requires method overloading.
For example, this code won't work right now:

rect().position.add(20).rotation(45);

We know that calling add with only the value will return the owner of the signal, allowing us to chain other operations. But the type system doesn't know this.

Instead, you can take inspiration from the CodeSignal

First, declare interfaces for your operations using overloading:

export interface Vector2Edit<TOwner> {
  (callback: (current: Vector2) => SignalValue<PossibleVector2>): TOwner;
  (
    callback: (current: Vector2) => SignalValue<PossibleVector2>,
    duration: number,
    timingFunction?: TimingFunction,
    interpolationFunction?: InterpolationFunction<Vector2>,
  ): SignalGenerator<PossibleVector2, Vector2>;
}

export interface Vector2Operation<TOwner> {
  (possibleVector: PossibleVector2): TOwner;
  (
    value: PossibleVector2,
    duration: number,
    timingFunction?: TimingFunction,
    interpolationFunction?: InterpolationFunction<Vector2>,
  ): SignalGenerator<PossibleVector2, Vector2>;
}

Then define the interface with all the helper methods:

export interface Vector2SignalHelpers<TOwner> {
  edit: Vector2Edit<TOwner>;
  mul: Vector2Operation<TOwner>;
  div: Vector2Operation<TOwner>;
  add: Vector2Operation<TOwner>;
  sub: Vector2Operation<TOwner>;
  dot: Vector2Operation<TOwner>;
  cross: Vector2Operation<TOwner>;
  mod: Vector2Operation<TOwner>;
}

Use it to both, declare the Vector2Signal type:

export type Vector2Signal<
  TOwner = void,
  TContext = Vector2CompoundSignalContext<TOwner>,
> = CompoundSignal<PossibleVector2, Vector2, 'x' | 'y', TOwner, TContext> &
  Vector2SignalHelpers<TOwner>;

And as an interface implemented by the context:

export class Vector2CompoundSignalContext<TOwner = void>
  extends CompoundSignalContext<PossibleVector2, Vector2, 'x' | 'y', TOwner>
  implements Vector2SignalHelpers<TOwner>
{

(This will make sure the context implements all the necessary methods)

Methods on the context should now also feature overloading:

class ... {
  public edit(
    callback: (current: Vector2) => SignalValue<PossibleVector2>,
  ): TOwner;
  public edit(
    callback: (current: Vector2) => SignalValue<PossibleVector2>,
    duration: number,
    timingFunction?: TimingFunction,
    interpolationFunction?: InterpolationFunction<Vector2>,
  ): SignalGenerator<PossibleVector2, Vector2>;
  public edit(
    callback: (current: Vector2) => SignalValue<PossibleVector2>,
    duration?: number,
    timingFunction?: TimingFunction,
    interpolationFunction?: InterpolationFunction<Vector2>,
  ): SignalGenerator<PossibleVector2, Vector2> | TOwner {
    const newValue = callback(this.get());
    return this.invoke(
      newValue,
      duration,
      timingFunction,
      interpolationFunction,
    ) as SignalGenerator<PossibleVector2, Vector2> | TOwner;
  }
}

timingFunction?: TimingFunction,
interpolationFunction?: InterpolationFunction<Vector2>,
) {
const newValue = callback(this.invokable());
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just use get:

Suggested change
const newValue = callback(this.invokable());
const newValue = callback(this.get());

): Vector2 | TOwner | SignalGenerator<PossibleVector2, Vector2>;
};

export class Vector2CompoundSignalContext<
Copy link
Contributor

Choose a reason for hiding this comment

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

The class should be called just Vector2SignalContext

Suggested change
export class Vector2CompoundSignalContext<
export class Vector2SignalContext<

@aarthificial aarthificial merged commit c5b02d1 into motion-canvas:main May 16, 2024
9 checks passed
@aarthificial
Copy link
Contributor

Thanks for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants