ruffle-rs / ruffle

A Flash Player emulator written in Rust
https://ruffle.rs
Other
15.72k stars 816 forks source link

RFC: Web API versioning #17202

Open Dinnerbone opened 4 months ago

Dinnerbone commented 4 months ago

I've discussed this a bit here and there on Discord, about how to make our Web API better going forwards. This is my proposal (or proposals, rather) and I'd like input on it before implementing it.

This is the final blocker that I've been waiting for, before introducing stable releases.

Although I'd love any input at all, there are two questions I specifically want answers to:

  1. The open question in "Accessing a version: Polyfilled elements"
  2. Which JS-created API approach to use, "Proposal 1: The element is the API" vs "Proposal 2: The element has the API"

The problems

There's a few problems with the Ruffle web API as it is today

1. It (kinda) conflicts with the Flash API

We didn't really have a grasp on what the Flash web API was - in fact, I don't think we truly knew it existed. It's barely documented - there's snippets here and there, mentions of some of the methods, but mostly we've discovered it by running into sites that used it and wondering why they didn't work.

There's a tracking issue for the Flash web API - some of the methods we can add no problem, but some of them kinda conflict with our existing methods. For example, the Flash Play() function is not the same as Ruffle's play() function.

Technically they're different methods, because Flash seems to capitalize everything UpperCamelCase, whereas we use camelCase. That said... want to explain to users when they should use Play vs play?

2. It definitely conflicts with ExternalInterface

The way that ExternalInterface works is that a movie can ExternalInterface.addCallback("isPlaying"), and then JavaScript can call the newly added isPlaying() method on the movie element.

This was a pretty bad design decision on Macromedia/Adobe's part - these methods could conflict with anything added to the public API of a Flash movie over time. However, since the Flash API used UpperCamelCase, and ActionScript convention was camelCase, it was unlikely to do so.

Ruffle, however... If a movie registers isPlaying, our own isPlaying method is used instead. Whichever way we do it, it's a bug - the API just can't support "anything a Flash movie adds" + "anything Ruffle adds". This is a real world issue.

3. We made mistakes, we can't fix them.

For better or worse, Ruffle's web API strives to be backwards compatible. If someone visits a website that uses an old Ruffle, whilst having a newer Ruffle extension installed, that extension takes over but the website should not break if the website used the API.

That's great, we can live with that. But it kinda sucks. Take for example the pause() and play() methods from an earlier point; they do not pause and play the Movie in the context of Flash, they suspend and resume the player itself.

It'd be nice to have a system in place that lets us change the API without breaking the API, years down the road.

4. We have no concept of what's public vs internal

For documentation purposes (of which we don't link anywhere), some things are marked private or protected internally. That's a TypeScript thing though, and completely 100% doesn't exist in the real world. Everything is public.

When people asked us "how do I do X", we've often told them "use this hidden internal method". That sucks. We should not need to keep backwards compatibility for our internal code. Please no.

The solution

So, the above problems suck, but I think they all share the same solution: We kill the batman.

Alternatively, we version the API. It makes sense, to me at least - it lets us change things pretty much at will, and we get a concrete "this is our API" that we can document and visualize without needing to crawl through literal thousands of lines of code and think "how can I access this?".

The idea is pretty simple: We make a new TypeScript interface per API version, and the user has a way of requesting which API they want. Whenever we make a new version, the existing versions are untouched - they're separate code.

There's just a few small problems...

Versioned player interfaces

Here's two theoretical versions of the API, showcasing how we'd fix the "pause isn't Flash's pause" issue. This is not a proposal of a V2 API, and merely used as an example. Please don't bikeshed the V2 API shown here.

export interface PlayerV1 {
  // Suspend the movie. Nothing will be executed until you call `play()`.
  pause(): void;
  // Resume the movie if it was previously `pause()`d.
  play(): void;
  // Call a method added by the movie via `ExternalInterface.addCallback`
  [callback: string]: (...args) => any;
}

export interface PlayerV2 {
  // Pause the movie's playhead. ActionScript will continue to execute. This is equivalent to unchecking the "Play" button in the context menu, or ActionScript `stop()`.
  stop(): void;
  // Allow the movie's playhead to keep playing. This is equivalent to checking the "Play" button in the context menu, or ActionScript `play()`.
  play(): void;
  // Suspend the movie. Nothing will be executed until you call `resume()`.
  suspend(): void;
  // Resume the movie. Resume the movie if it was previously `suspend()`ed.
  resume(): void;
  // Call a method added by the movie via `ExternalInterface.addCallback`
  callExternalInterface(name: string, ...args): any;
}

Accessing a version: Polyfilled elements

We'd add a new ruffle(version: number = 1) method to the RuffleEmbed and RuffleObject HTML elements, which constructs an object that conforms to the API version given. If the given API isn't available, it'll throw an exception.

Polyfilled elements will gain the Flash JS API (though likely over time, as we get to it).

It's possible for the ruffle method to still conflict with ExternalInterface as described by problem 3 above. I'm willing to take that risk though; it's just one method, instead of many.

Open Question: What do we do about the existing methods/properties of the current API?

  1. Remove them: Only Flash API and ruffle will exist. This solves problem 3, but breaks backwards compatibility.
  2. Keep them: All existing things will continue to exist, there'll be a Play and play method, backwards compatibility is preserved. Problem 3 will unsolvable. We never add or change these methods, they exist only for backwards compatibility prior to versioned APIs.
  3. Keep them, but ExternalInterface can replace them. Unsure if this is actually possible, but it's the above solution but if something registers isPlaying, it replaces Ruffle's isPlaying. This just changes the problem to a different one though...

Example:

const player = getElementById("player"); // assuming this was an `<embed>`/`<object>`
player.id = "game"; // it's a regular HTML element
player.Pause(); // Flash JS API exists
player.ruffle(2).suspend(); // Ruffle is accessed indirectly (it can be stored, though)

player.foo(); // calls the ExternalInterface method 'foo', via Flash JS API
player.ruffle(1).foo(); // calls the ExternalInterface method 'foo', via Ruffle V1 (which mimics Flash JS API)
player.ruffle(2).callExternalInterface("foo"); // calls the ExternalInterface method 'foo', via Ruffle V2 (just an example, not a proposal)

Accessing a version: Created through the API

There's two different ways to do this, with their own pros and cons - so I've separated this into two proposals.

Proposal 1: The element is the API

For players created with window.RufflePlayer.createPlayer(), we'll change the signature to createPlayer(version: number = 1). The RufflePlayer HTML element will gain the properties/methods described by the given version. If the given API isn't available, it'll throw an exception.

JS created elements will not gain the Flash JS API. This is backwards incompatible, if something happened to be doing that today. (I don't know if they do... reminder that we don't implement the majority of the API today anyway).

Whenever anything gets a hold of the RufflePlayer element, they have immediate access to the API.

For example:

const player = document.createElement(RufflePlayer.createPlayer(2));
player.id = "game"; // it's a regular HTML element
player.suspend(); // but it's also the v2 API

player.callExternalInterface("foo"); // calls the ExternalInterface method 'foo', via Ruffle V2 (just an example, not a proposal)
// player.foo(); // if it were v1, calls 'foo' via Ruffle V1 (which mimics Flash JS API)

appendChild(player);
// and later
getElementById("game").resume(); // The API just exists on the player, always.

As the default is version 1, and version 1 should (in theory) be the current API as of writing this document, no backwards incompatibilities lie with things using our JS API.

Pros:

Cons:

Proposal 2: The element has the API

We'll make the RufflePlayer act the same as a polyfilled player, so you call .ruffle(version: number = 1) on it to get the API.

JS created elements will gain the Flash JS API, just like polyfilled elements.

Whenever anything gets a hold of the RufflePlayer element, they have to request API access from it.

This essentially means there's no distinction between a JS-created player and a polyfilled player, they're the same code and used the exact same way. The open question from "Accessing a version: Polyfilled elements" applies here too.

For example:

const player = document.createElement(RufflePlayer.createPlayer());
player.id = "game"; // it's a regular HTML element
player.Pause(); // it has the Flash JS API
player.ruffle(2).suspend(); // you have access to any API version at will (this can be stored and reused).

player.foo(); // calls the ExternalInterface method 'foo', via Flash JS API
player.ruffle(1).foo(); // calls the ExternalInterface method 'foo', via Ruffle V1 (which mimics Flash JS API)
player.ruffle(2).callExternalInterface("foo"); // calls the ExternalInterface method 'foo', via Ruffle V2 (just an example, not a proposal)

appendChild(player);
// and later
getElementById("game").ruffle(2).resume(); // The API always has to be extracted from the player.

Pros:

Cons:

danielhjacobs commented 4 months ago

Copying part of my message from Discord:

My assumption with option 2 and 3 is we'll keep those methods for backwards compatibility but still define them as deprecated, also add them to .ruffle(version: number = 1), and advise people to use that. If it's possible, which I think it should be knowing how flexible JavaScript can be for better or worse, I support option 3. I also support proposal 2 because we should avoid breaking backwards compatibility and the only downside, slight added complexity, is easily solved by good documentation.

n0samu commented 4 months ago

Open Question: What do we do about the existing methods/properties of the current API?

I'm not happy with the situation that we're in nor any of our options, but I support option 3 because that way we are at least creating an uncommon problem, so we can feasibly support users when they run into it and ask us for help. In an ideal world we would be able to choose option 1, but we would have to somehow alert all of our users across the internet that our API is changing explain how to deal with it. The only idea I can think of for that would be to show an overlay like the hardware acceleration warning whenever a Ruffle JS API function is called on a polyfilled element, which would alert anyone interacting with the content that the site owner needs to update their JavaScript for Ruffle's new API. This would still be very disruptive and annoying, and we'd have to delay removal of the relevant JS API functions for a long time to give site owners time to update their websites, so I don't think it's worth it.

As for proposals 1 and 2, I don't have a strong preference, but judging from what's written here proposal 2 definitely sounds better.

n0samu commented 3 months ago

If we're going with Proposal 2, we should start with just one version of the ruffle() API (the new improved version, designated as version 1). This would be selected by default and users would only need to worry about API versions if we decide to make a v2 API in the future. If a user doesn't want to put in the effort to update their code, they still won't need to update it, so offering a v1 API that duplicates the current system is just extra complexity and potential confusion.