Support creating Actors from a shape

[mattiashagstrand]

In the current implementation all Actors are backed by textures, but it would be nice to be able to create Actors that are just shapes.

Since core animation already has a CAShapeLayer, a new class ShapeLayer could be created to provide a cleaner API. Then the Actor class needs to be changed so that it can use different types of layers.

[JohnSundell]

Great idea @mattiashagstrand - I would love if that feature was added!

I've actually done some thinking about this before, so I'll sum up my thoughts and let me know what you think:

I think it would be cool if there was a Shape object, instead of it being a feature of Actor. The reason, is that not all actions that an actor can do make sense for a shape. For example playing an animation or having a scale applied to it (since the shape will be resolution agnostic).

It would also enable us to do some cool stuff with things like hit boxes. Something like this:

actor.hitBox = Shape(circleWithRadius: 50)

I totally agree that the Shape object should be backed by CAShapeLayer 👍

What do you think? 😄

[mattiashagstrand]

Totally agree that some of the actions on actor don't apply for shapes. At the same time, many of the features of an actor also applies for a shape, e.g. shapes have size and position, could collide, move and so on.

I think that a Shape object is a good idea no matter how the rendering is implemented. It could definitely be used in hit testing.

So maybe we should have one Shapeobject and another object that takes a shape as input and uses a CAShapeLayerto display it... 🤔

One problem with a new object for displaying shapes could be that we end up with some code that is a duplication of what is already implemented in Actor. On the other hand I prefer easy to understand code and API over keeping everything super DRY.

Now that we are thinking about supporting new layers, don't forget about CAEmitterLayer. Would be really cool to support particle systems... 😎

[JohnSundell]

Yeah, I totally agree that we need to minimize the amount of code duplication if we were to go the separate object route. But I think this is a goal we need to have in general - I'd love for us to implement lots of specialized objets (including ParticleEmitter, like you mention, etc). There are so many cool Core Animation features we can use 😄

So, I think the goal needs to be to make it possible to define new objects while still avoiding code duplication. I discussed this a bit with @NSMyself in #63. We should be able to enable high reusability of code by using protocols and potentially some form of base class that Actor and Shape can share.

I think we might be able use the same Shape object for both use cases (rendering + hit boxes), if we make the object lazily load its layer whenever its added to a scene 🤔 So when used as a hit box it won't use a layer and instead just provide shape information.

[NSMyself]

A bit off-topic but still: regarding #63, if the SceneObject protocol specified an add function (and we might as well specify a remove one too) we'd have a lot less boilerplate. Very easy and fast to do too.

[mattiashagstrand]

I was also thinking about adding something to SceneObject, but instead of adding add and remove functions, what if we add a new property:

var layer: Layer { get }

Then we could remove the very similar add functions in Scene and just have one function:

func add(_ sceneObject: SceneObject)

Of course, for this to work we need to change the implementation of Layer. Maybe it should be a protocol instead of a subclass of CALayer... 🤔

And it also requires changes in Grid...

[mattiashagstrand]

I get your point @JohnSundell about having a ingle Shapeobject. Lazily loading the layer is a great idea! It would also make it consistent with Labeland Block.

About my other idea of extending SceneObject. Probably shouldn't have a layer property on it since that would mean that Layer would have to be public and we would leak implementation details in the API. But what about something like this:

internal protocol Displayable {
    var layer: Layer { get }
}

extension SceneObject: Displayable { }

[JohnSundell]

@mattiashagstrand Yeah exactly, we don't want to leak the fact that we're using CoreAnimation for rendering as part of the API, since it's quite possible that we'd like to switch rendering backends in the future 👍

I'm afraid that using an extension like you suggested won't really work either, since you can't extend a protocol with another protocol (yet). However, one solution could be to make SceneObject a class instead of a protocol, and be able to have both internal and public properties on it. One challenge though is that different objects will require different CALayer subclasses 😅 But we can probably find some way around that 😄

[mattiashagstrand]

Doh! Swift is so great that sometimes I forget it can't do everything... 🙃

Yes, supporting different CALayer subclasses is a challenge... 🤔

Making SceneObjecta class could work since none of the implementors have a superclass today.

One solution could be to add methods to SceneObjects as suggested by @NSMyself. To avoid making the layer property in Scene internal they could look like this:

protocol SceneObject {
    var scene: Scene? { get set }
    func add(to layer: Layer)
    func remove(from layer: Layer)
}

class Shape: SceneObject {
    func add(to layer: Layer) {
        layer.addSublayer(ShapeLayer())
    }
}

[JohnSundell]

Yeah I really like that idea - it would make the implementation a lot simpler too, and goes inline with how plugins work for example (in that they get a callback when attached to an object). We don't require the layer in remove though, since we can just call layer.removeFromSuperlayer() 👍

[JohnSundell]

@mattiashagstrand @NSMyself Actually I don't think we need either of those methods, come to think about it. Because we already have the Activatable protocol that sends activation & deactivation events to all scene objects. All we'd have to do is to expose the layer property of Scene internally so that actors can add themselves to it 😄

[mattiashagstrand]

Ahh, yes, I like it! 👍 It also has the added benefit that actors are added to the scene lazily. So if a scene is never activated we don't have to create layers for the scene objects.

Should I start a PR and implement this change plus add a first version of a Shapeobject? I guess we implement collisions in the same way as in actors so the PR doesn't get to big...

By the way, should I create PRs in my fork or directly in the main repo?

[JohnSundell]

@mattiashagstrand Great point 👍

Let's start by doing the SceneObject refactor separately. I think before we start adding Shape we should get everything in order to help us avoid code duplication (where it makes sense). So let's start with this one and then take it from there 😄

It would be awesome if you want to work on this. You can work on your fork, and then submit a PR against this repo, that way we can keep the master repo clean of temporary branches 👌

[JohnSundell]

@mattiashagstrand Before you start though, one challenge will this approach will be to preserve the order in which actor layers are added to the scene. Since grid.actors is a Set (for quick insertions & deletions) there's no guaranteed order. So we might need to do something like in Timeline where it keeps track of objects that are pending activation using an Array.

[JohnSundell]

Maybe an intermediate step is to simply do what you and @NSMyself suggested and add an add(to layer: Layer) method to SceneObject for now.

[JohnSundell]

But maybe let's call it addLayer(to superlayer: Layer) to make it clear that it's the object's layer that should be added.

[mattiashagstrand]

Ahh, yes, the ordering is a problem.

Ok, I'll start by just adding the new method to SceneObject. For now we can skip the removefunction I think. Actor already does layer.removeFromSuperlayer() it is deactivated.

Maybe the next logical thing to extract from Actor is collision detection?

[JohnSundell]

Yes, sounds like a plan! Want to make a separate issue for that?

[mattiashagstrand]

I'll create a new issue!