Exception thrown in Asset Registry tutorial project

[willeastcott]

To repro:

1. Open this project: https://playcanvas.com/project/406036/overview/tutorial-asset-registry
2. Launch
3. Press space bar
4. Release space bar
5. Exception is thrown

This change was introduce with this commit from @Maksims:

912958c

[Maksims]

Looked into an issue. There is a question:
When ModelComponent.model is set to anything, shall we clone model with its meshInstances when setting, or just set? Reason I ask, is that if we have this scenario:

1. Two Entities, with different parameters on casting shadows or lightmapping.
2. One model Asset.
3. We set this model asset resource to both of those entities, it will share same meshInstances objects, leading to wrong casting shadows and lightmapping parameters as those parameters belong to ModelComponent, but set on shared mesh instances from the asset. So basically, we modify assets resource without intent here.

So, should mesh instances and model object be cloned in this case, to prevent shared objects with unintended parameters changes?

When we use same asset on model component during loading, they are cloned, when we set explicit model parameter (not asset), then it is not cloned, leading to shared instances.
Even worse, try in referenced example do this: this.entity.model.model = this.asset.resouce on two different entities - it will throw an error: GraphNode is already parented. Test project: https://launch.playcanvas.com/871188?debug=true

So, asset.resource should never be reused by ModelComponent, but should be cloned.

[wturber]

My apologies if I'm speaking out of turn here, but as a user, it seems to me that if an attribute belongs to ModelComponent then that attribute should not be set on the Editor Model Instance interface, but should be set on the ModelComponent interface. It seems very odd to me that if shadow attributes are a property of a model, that a user must associate an object to an entity before these shadow attributes can be set. It should be presented to the user as a category of attributes in the same way that MeshInstances are presented to the user as a category of attributes for the ModelComponent. Not doing it this way seems to lead to the problem of sharing attributes between two models that could have been avoided to begin with.

Alternatively, maybe the shadow attributes shouldn't be a property of the ModelComponent at all but should be a property of the Entity.

I realize that this may all be beside the point of your issue of whether to always clone or not. But it seems to be that the root of the problem may be in the way that the shadow (and some other) attributes are being assigned and controlled.

And while I'm poking my nose into this subject, I will add that I found it odd that when I change model components that the Materials for the Mesh Instance did not follow. I have to take special steps to copy those materials over. Otherwise the model inherits the Materials from the previous Entity's model Mesh Instances. Getting only the geometry and not the materials is very non-intuitive. But if we aren't going to concern ourselves about carrying a Model's materials to the new entity, why should we care about carrying a Model's shadow attributes?

Again, I'm not sure if these observations are appropriate here or not. But I'm hoping it will provide some insight into how users who aren't primarily coders may be looking at this and perhaps provide some guidance in choosing a solution path.

[slimbuck]

@Maksims How do transforms work in the case two model components share the same model? Model has graph member so presumably both components will use the same transform?

It seems assigning a model instance to more than one component is simply invalid and is probably something we can easily check in code?

[slimbuck]

You probably don't want to always clone Model instance as in many cases it's probably not necessary.

Perhaps we can clone the model in cases where it is already assigned to a different component? That might be too much under-the-covers-magic though...

[Maksims]

@Maksims How do transforms work in the case two model components share the same model? Model has graph member so presumably both components will use the same transform?

This fails now. But IMHO, for user it is not obvious why, and he is not doing anything wrong. So by cloning, we solve this problem too. We actually do clone always if you set entity.model.asset = asset, but if you do this: entity.model.model = asset.resource then we do not clone (issue).

You probably don't want to always clone Model instance as in many cases it's probably not necessary.

Perhaps we can clone the model in cases where it is already assigned to a different component? That might be too much under-the-covers-magic though...

I have more simple solution: if model is created by resource handler and belongs to asset.resource then this model will be marked as immutable, so model component will always clone set model. So it will match behaviour with setting asset property.