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

How to handle parametric components #10

Open
louisponet opened this issue May 1, 2021 · 3 comments
Open

How to handle parametric components #10

louisponet opened this issue May 1, 2021 · 3 comments

Comments

@louisponet
Copy link
Owner

I've cleaned up the @component macro, there was a slight bug.
In doing so I also made it so that now Parametric ComponentData types are allowed.

This brings up the question of how to best handle this, and if it makes sense to begin with.

e.g.

using Parameters
using GeometryTypes
using Overseer

@component @with_kw struct Spatial{T}
    position::Point3{T} = Point3(1.0,1.0,1.0)
    velocity::Vec3{T} = Vec3(1.0,1.0,1.0)
end

@component struct Spring{T}
    center::Point3{T}
    spring_constant::T
end

struct Oscillator <: System end

# Overseer.requested_components(::Oscillator) = (Spatial, Spring) will currently error at the Ledger creation stage
Overseer.requested_components(::Oscillator) = (Spatial{Float64}, Spring{Float64})

function Overseer.update(::Oscillator, m::AbstractLedger)
...
end

stage = Stage(:simulation, [Oscillator()])
m = Ledger(stage)

m[Spatial] # nothing found
m[Spatial{Float64}] # found, empty
Entity(m, Spatial(Point3f0(1.0,1.0,1.0), Vec3f0(1.0,1.0,1.0))) # Component{Spatial{Float32}} will be added to m
m[Spatial{Float32}] # found, containing 1 entity

As you can see from this example, there are some things to consider. First of all, I think I'd like to be able to do m[Spatial], although I'm unsure how exactly this can be performant. Maybe only something like an iterator over all components in m, <: Spatial would be useful.

Basically, do we like the behavior that is present now?

@c42f
Copy link
Contributor

c42f commented May 5, 2021

First of all, I think I'd like to be able to do m[Spatial], although I'm unsure how exactly this can be performant. Maybe only something like an iterator over all components in m, <: Spatial would be useful.

Maybe declare that m[Spatial] returns the single component which is <: Spatial, and make it an error if there's more than one? That would be ok for your example where you don't want more than one Spatial component.

The lookup can't be fast if we've got a Dict of component arrays keyed on the concrete types though — I think you'd need a ledger containing a Tuple of the concrete component types so the lookup can be done at compile time. I think we talked about that on a previous issue somewhere.

@louisponet
Copy link
Owner Author

louisponet commented May 5, 2021

Yes, you're right. I was wondering if a simple

if haskey(dict, ComponentData) 
    dict[ComponentData]
else
    return getindex.((dict,), filter(x->x isa ComponentData, keys(dict)))
end

would be a valid naive implementation with some tradeoff for wanting to use this functionality (i.e. you're not specifically asking for the right things). Again, I guess this would mainly happen outside the innermost loops. But that mentality at some point may bite us in the ass anyway.

I was thinking that actually it would be quite nice to have something like a MergedComponent that is basically a tuple of components with the same non-parametric type, which you could use as a single object in @entities_in so that it would loop over all entities in the first, e.g. Spatial{Float32} continuing with the second Spatial{Float64} etc. Although that might lead to headaches when trying to do @entities_in with multiple of those... hmmm

@c42f
Copy link
Contributor

c42f commented May 6, 2021

I'd potentially write that as

get(dict, ComponentData) do
    for (k,v) in dict # just the first match?
        k isa ComponentData && return v
    end
    error("...")
end

I don't think it's great to return either a vector or a single component, depending on multiple vs single matches. It's pretty hard to reliably use an API where the result might-or-might-not be a collection.


Here's another perhaps bad idea — how about detecting when a concrete version of a parametric type is added, and stripping it down to its parent parametric type (eg, using this hack).

That should let you retain the Dict and do fast lookup (I guess), only with the caveat that the actual concrete DataType can't be used in the lookup for parametric types.

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

No branches or pull requests

2 participants